Stop discarding serious errors, reörder toPreserves args

6 files changed, 91 insertions, 80 deletions

diff --git a/sbom.json b/sbom.json
index 8c1fdb9fa2e2..4082821204a3 100644
--- a/sbom.json
+++ b/sbom.json
@@ -7,7 +7,7 @@
       "bom-ref": "pkg:nim/nix_actor",
       "name": "nix_actor",
       "description": "Syndicated Nix Actor",
-      "version": "20240824",
+      "version": "20240825",
       "authors": [
         {
           "name": "Emery Hemingway"
diff --git a/src/nix_actor/nix_api.nim b/src/nix_actor/nix_api.nim
index 656217461ac0..e382602ca8c1 100644
--- a/src/nix_actor/nix_api.nim
+++ b/src/nix_actor/nix_api.nim
@@ -26,11 +26,11 @@ proc receiveString(start: cstring; n: cuint; state: pointer) {.cdecl.} =
 
 proc initLibstore*() =
   mitNix:
-    discard nix.libstore_init()
+    checkError nix.libstore_init()
 
 proc initLibexpr*() =
   mitNix:
-    discard nix.libexpr_init()
+    checkError nix.libexpr_init()
 
 proc openStore*(uri = "auto", params: openarray[string] = []): Store =
   mitNix:
@@ -48,13 +48,13 @@ proc getUri*(store: Store; cb: StringCallback) =
   mitNix:
     let state = new StringCallbackState
     state.callback = cb
-    discard nix.store_get_uri(store, receiveString, state[].addr)
+    checkError nix.store_get_uri(store, receiveString, state[].addr)
 
 proc getVersion*(store: Store; cb: StringCallback) =
   mitNix:
     let state = new StringCallbackState
     state.callback = cb
-    discard nix.store_get_version(store, receiveString, state[].addr)
+    checkError nix.store_get_version(store, receiveString, state[].addr)
 
 proc isValidPath*(store: Store; path: string): bool =
   assert not store.isNil
@@ -74,9 +74,9 @@ proc copyClosure*(src, dst: Store; path: string) =
     if sp.isNil:
       raise newException(CatchableError, "store_parse_path failed")
     defer: store_path_free(sp)
-    nix.store_copy_closure(src, dst, sp)
+    checkError nix.store_copy_closure(src, dst, sp)
 
-proc newState*(store: Store; lookupPath: openarray[string]): EvalState =
+proc newState*(store: Store; lookupPath: openarray[string] = []): EvalState =
   mitNix:
     var path = allocCStringArray(lookupPath)
     defer: deallocCStringArray(path)
@@ -87,23 +87,19 @@ proc close*(state: EvalState) = state_free(state)
 
 proc close*(value: Value) =
   mitNix:
-    discard nix.gc_decref(cast[pointer](value))
+    checkError nix.gc_decref(cast[pointer](value))
 
 proc evalFromString*(nix: NixContext; state: EvalState; expr, path: string; result: Value)  =
-  discard nix.expr_eval_from_string(state, expr, path, result)
+  checkError nix.expr_eval_from_string(state, expr, path, result)
 
-proc evalFromString*(state: EvalState; expr: string; path = ""): Value =
+proc evalFromString*(state: EvalState; expr: string; path = "."): Value =
   mitNix:
-    try:
-      result = nix.alloc_value(state)
-      nix.evalFromString(state, expr, path, result)
-    except CatchableError as err:
-      result.close()
-      raise err
+    result = nix.alloc_value(state)
+    nix.evalFromString(state, expr, path, result)
 
 proc force*(state: EvalState; value: Value) =
   mitNix:
-    discard nix.value_force(state, value)
+    checkError nix.value_force(state, value)
 
 proc get_attr_byidx*(value: Value; state: EvalState; i: cuint): (cstring, Value) =
   mitNix:
@@ -111,7 +107,7 @@ proc get_attr_byidx*(value: Value; state: EvalState; i: cuint): (cstring, Value)
 
 proc apply(nix: NixContext; state: EvalState; fn, arg: Value): Value =
   result = nix.alloc_value(state)
-  discard nix.init_apply(result, fn, arg)
+  checkError nix.init_apply(result, fn, arg)
 
 proc apply*(state: EvalState; fn, arg: Value): Value =
   mitNix:
@@ -121,4 +117,4 @@ proc call*(state: EvalState; fn: Value; args: varargs[Value]): Value =
   mitNix:
     result = nix.alloc_value(state)
     var array = cast[ptr UncheckedArray[Value]](args)
-    discard nix.value_call_multi(state, fn, args.len.csize_t, array, result)
+    checkError nix.value_call_multi(state, fn, args.len.csize_t, array, result)
diff --git a/src/nix_actor/nix_api_store.nim b/src/nix_actor/nix_api_store.nim
index 0e1719062248..2df3f891ede4 100644
--- a/src/nix_actor/nix_api_store.nim
+++ b/src/nix_actor/nix_api_store.nim
@@ -7,17 +7,17 @@ import ./nix_api_types
 
 {.pragma: nix_api_store, header: "nix_api_store.h", importc: "nix_$1".}
 
-proc libstore_init*(context: NixContext): nix_err {.nix_api_store, discardable.}
+proc libstore_init*(context: NixContext): nix_err {.nix_api_store.}
 
-proc libstore_init_no_load_config*(context: NixContext): nix_err {.nix_api_store, discardable.}
+proc libstore_init_no_load_config*(context: NixContext): nix_err {.nix_api_store.}
 
-proc init_plugins*(context: NixContext): nix_err {.nix_api_store, discardable.}
+proc init_plugins*(context: NixContext): nix_err {.nix_api_store.}
 
 proc store_open*(context: NixContext; uri: cstring; params: ptr cstringArray): Store {.nix_api_store.}
 
 proc store_free*(store: Store) {.nix_api_store.}
 
-proc store_get_uri*(context: NixContext; store: Store; callback: GetStringCallback; user_data: pointer): nix_err {.nix_api_store, discardable.}
+proc store_get_uri*(context: NixContext; store: Store; callback: GetStringCallback; user_data: pointer): nix_err {.nix_api_store.}
 
 proc store_parse_path*(context: NixContext; store: Store; path: cstring): StorePath {.nix_api_store.}
 
@@ -31,8 +31,8 @@ proc store_is_valid_path*(context: NixContext; store: Store; path: StorePath): b
 
 type RealiseCallback* = proc (userdata: pointer; outname: cstring; `out`: cstring) {.cdecl.}
 
-proc store_realise*(context: NixContext; store: Store; path: StorePath; userdata: pointer; callback: RealiseCallback): nix_err {.nix_api_store, discardable.}
+proc store_realise*(context: NixContext; store: Store; path: StorePath; userdata: pointer; callback: RealiseCallback): nix_err {.nix_api_store.}
 
-proc store_get_version*(context: NixContext; store: Store; callback: GetStringCallback; user_data: pointer): nix_err {.nix_api_store, discardable.}
+proc store_get_version*(context: NixContext; store: Store; callback: GetStringCallback; user_data: pointer): nix_err {.nix_api_store.}
 
-proc store_copy_closure*(context: NixContext; src, dst: Store; path: StorePath): nix_err {.nix_api_store, discardable.}
+proc store_copy_closure*(context: NixContext; src, dst: Store; path: StorePath): nix_err {.nix_api_store.}
diff --git a/src/nix_actor/nix_api_value.nim b/src/nix_actor/nix_api_value.nim
index 5fba2af56a58..d4d5edc5f4c3 100644
--- a/src/nix_actor/nix_api_value.nim
+++ b/src/nix_actor/nix_api_value.nim
@@ -18,7 +18,7 @@ proc get_typename*(context: NixContext; value: Value): cstring {.nix_api_value.}
 
 proc get_bool*(context: NixContext; value: Value): bool {.nix_api_value.}
 
-proc get_string*(context: NixContext; value: Value; callback: GetStringCallback; user_data: pointer): nix_err {.nix_api_value, discardable.}
+proc get_string*(context: NixContext; value: Value; callback: GetStringCallback; user_data: pointer): nix_err {.nix_api_value.}
 
 proc get_path_string*(context: NixContext; value: Value): cstring {.nix_api_value.}
 
@@ -42,39 +42,39 @@ proc get_attr_byidx*(context: NixContext; value: Value; state: EvalState; i: cui
 
 # proc get_attr_name_byidx*(context: NixContext; value: Value; state: EvalState; i: cuint): cstring {.nix_api_value.}
 
-proc init_bool*(context: NixContext; value: Value; b: bool): nix_err {.nix_api_value, discardable.}
+proc init_bool*(context: NixContext; value: Value; b: bool): nix_err {.nix_api_value.}
 
-proc init_string*(context: NixContext; value: Value; str: cstring): nix_err {.nix_api_value, discardable.}
+proc init_string*(context: NixContext; value: Value; str: cstring): nix_err {.nix_api_value.}
 
-proc init_path_string*(context: NixContext; s: EvalState; value: Value; str: cstring): nix_err {.nix_api_value, discardable.}
+proc init_path_string*(context: NixContext; s: EvalState; value: Value; str: cstring): nix_err {.nix_api_value.}
 
-proc init_float*(context: NixContext; value: Value; d: cdouble): nix_err {.nix_api_value, discardable.}
+proc init_float*(context: NixContext; value: Value; d: cdouble): nix_err {.nix_api_value.}
 
-proc init_int*(context: NixContext; value: Value; i: int64): nix_err {.nix_api_value, discardable.}
+proc init_int*(context: NixContext; value: Value; i: int64): nix_err {.nix_api_value.}
 
-proc init_null*(context: NixContext; value: Value): nix_err {.nix_api_value, discardable.}
+proc init_null*(context: NixContext; value: Value): nix_err {.nix_api_value.}
 
-proc init_apply*(context: NixContext; value: Value; fn: Value; arg: Value): nix_err {.nix_api_value, discardable.}
+proc init_apply*(context: NixContext; value: Value; fn: Value; arg: Value): nix_err {.nix_api_value.}
 
-proc init_external*(context: NixContext; value: Value; val: ExternalValue): nix_err {.nix_api_value, discardable.}
+proc init_external*(context: NixContext; value: Value; val: ExternalValue): nix_err {.nix_api_value.}
 
-proc make_list*(context: NixContext; list_builder: ListBuilder; value: Value): nix_err {.nix_api_value, discardable.}
+proc make_list*(context: NixContext; list_builder: ListBuilder; value: Value): nix_err {.nix_api_value.}
 
 proc make_list_builder*(context: NixContext; state: EvalState; capacity: csize_t): ListBuilder {.nix_api_value.}
 
-proc list_builder_insert*(context: NixContext; list_builder: ListBuilder; index: cuint; value: Value): nix_err {.nix_api_value, discardable.}
+proc list_builder_insert*(context: NixContext; list_builder: ListBuilder; index: cuint; value: Value): nix_err {.nix_api_value.}
 
 proc list_builder_free*(list_builder: ListBuilder) {.nix_api_value.}
 
-proc make_attrs*(context: NixContext; value: Value; b: BindingsBuilder): nix_err {.nix_api_value, discardable.}
+proc make_attrs*(context: NixContext; value: Value; b: BindingsBuilder): nix_err {.nix_api_value.}
 
-# proc init_primop*(context: NixContext; value: Value; op: PrimOp): nix_err {.nix_api_value, discardable.}
+# proc init_primop*(context: NixContext; value: Value; op: PrimOp): nix_err {.nix_api_value.}
 
-proc copy_value*(context: NixContext; value: Value; source: Value): nix_err {.nix_api_value, discardable.}
+proc copy_value*(context: NixContext; value: Value; source: Value): nix_err {.nix_api_value.}
 
 proc make_bindings_builder*(context: NixContext; state: EvalState; capacity: csize_t): BindingsBuilder {.nix_api_value.}
 
-proc bindings_builder_insert*(context: NixContext; builder: BindingsBuilder; name: cstring; value: Value): nix_err {.nix_api_value, discardable.}
+proc bindings_builder_insert*(context: NixContext; builder: BindingsBuilder; name: cstring; value: Value): nix_err {.nix_api_value.}
 
 proc bindings_builder_free*(builder: BindingsBuilder) {.nix_api_value.}
 
diff --git a/src/nix_actor/nix_values.nim b/src/nix_actor/nix_values.nim
index e97d19974daa..83b896be7595 100644
--- a/src/nix_actor/nix_values.nim
+++ b/src/nix_actor/nix_values.nim
@@ -51,27 +51,26 @@ proc exportNix*(facet: Facet; v: Value): Value =
           v
   v.mapEmbeds(op)
 
-proc callThru(state: EvalState; nv: NixValue): NixValue =
+proc callThru(nix: NixContext; state: EvalState; nv: NixValue): NixValue =
   result = nv
-  mitNix:
-    while true:
-      case nix.get_type(result)
-      of NIX_TYPE_THUNK:
-        state.force(result)
-      of NIX_TYPE_FUNCTION:
-        # Call functions with empty attrsets.
-        var
-          args = nix.alloc_value(state)
-          bb = nix.make_bindings_builder(state, 0)
-        discard nix.gc_decref(args)
-        doAssert nix.make_attrs(args, bb) == NIX_OK
-        bindings_builder_free(bb)
-        result = state.apply(result, args)
-      else:
-        return
+  while true:
+    case nix.get_type(result)
+    of NIX_TYPE_THUNK:
+      state.force(result)
+    of NIX_TYPE_FUNCTION:
+      # Call functions with empty attrsets.
+      var
+        args = nix.alloc_value(state)
+        bb = nix.make_bindings_builder(state, 0)
+      checkError nix.gc_decref(args)
+      doAssert nix.make_attrs(args, bb) == NIX_OK
+      bindings_builder_free(bb)
+      result = state.apply(result, args)
+    else:
+      return
 
-proc toPreserves*(state: EvalState; value: NixValue; nix: NixContext): Value {.gcsafe.} =
-  var value = callThru(state, value)
+proc toPreserves*(value: NixValue; state: EvalState; nix: NixContext): Value {.gcsafe.} =
+  var value = nix.callThru(state, value)
 
   let kind = nix.get_type(value)
   case kind
@@ -95,11 +94,11 @@ proc toPreserves*(state: EvalState; value: NixValue; nix: NixContext): Value {.g
       var str = nix.get_attr_byname(value, state, "__toString")
       if nix.get_type(str) == NIX_TYPE_FUNCTION:
         str = state.apply(str, value)
-      result = state.toPreserves(str, nix)
+      result = str.toPreserves(state, nix)
     elif nix.has_attr_byname(value, state, "outPath"):
       var outPath = nix.get_attr_byname(value, state, "outPath")
       result = Derivation(
-          value: state.toPreserves(outPath, nix),
+          value: outPath.toPreserves(state, nix),
           context: NixValueRef(value: value).embed,
         ).toPreserves
     else:
@@ -108,7 +107,7 @@ proc toPreserves*(state: EvalState; value: NixValue; nix: NixContext): Value {.g
       var i: cuint
       while i < n:
         let (key, val) = get_attr_byidx(value, state, i)
-        result[($key).toSymbol] = state.toPreserves(val, nix)
+        result[($key).toSymbol] = val.toPreserves(state, nix)
         inc(i)
   of NIX_TYPE_LIST:
     let n = nix.getListSize(value)
@@ -116,37 +115,37 @@ proc toPreserves*(state: EvalState; value: NixValue; nix: NixContext): Value {.g
     var i: cuint
     while i < n:
       var val = nix.getListByIdx(value, state, i)
-      result[i] = state.toPreserves(val, nix)
+      result[i] = val.toPreserves(state, nix)
       inc(i)
   of NIX_TYPE_THUNK, NIX_TYPE_FUNCTION:
     raiseAssert "cannot preserve thunk or function"
   of NIX_TYPE_EXTERNAL:
     result = "«external»".toPreserves
 
-proc toPreserves*(state: EvalState; value: NixValue): Value {.gcsafe.} =
-  mitNix: result = toPreserves(state, value, nix)
+proc toPreserves*(value: NixValue; state: EvalState): Value {.gcsafe.} =
+  mitNix: result = toPreserves(value, state, nix)
 
 proc translate*(nix: NixContext; state: EvalState; pr: preserves.Value): NixValue =
   try:
     result = nix.alloc_value(state)
     case pr.kind
     of pkBoolean:
-      nix.init_bool(result, pr.bool)
+      checkError nix.init_bool(result, pr.bool)
     of pkFloat:
-      nix.init_float(result, pr.float.cdouble)
+      checkError nix.init_float(result, pr.float.cdouble)
     of pkRegister:
-      nix.init_int(result, pr.register.int64)
+      checkError nix.init_int(result, pr.register.int64)
     of pkBigInt:
-      nix.init_int(result, pr.register.int64)
+      checkError nix.init_int(result, pr.register.int64)
     of pkString:
-      nix.init_string(result, pr.string)
+      checkError nix.init_string(result, pr.string)
     of pkByteString:
       raise newException(ValueError, "cannot convert large Preserves integer to Nix: " & $pr)
     of pkSymbol:
       nix.evalFromString(state, cast[string](pr.symbol), "", result)
     of pkRecord:
       if pr.isRecord("null", 0):
-        nix.init_null(result)
+        checkError nix.init_null(result)
       elif pr.isRecord("drv", 2):
         var drv: Derivation
         if not drv.fromPreserves(pr):
@@ -161,17 +160,17 @@ proc translate*(nix: NixContext; state: EvalState; pr: preserves.Value): NixValu
       let b = nix.make_list_builder(state, pr.len.csize_t)
       defer: list_builder_free(b)
       for i, e in pr:
-        discard nix.list_builder_insert(b, i.register.cuint, nix.translate(state, e))
-      nix.make_list(b, result)
+        checkError nix.list_builder_insert(b, i.register.cuint, nix.translate(state, e))
+      checkError nix.make_list(b, result)
     of pkDictionary:
       let b = nix.make_bindings_builder(state, pr.dict.len.csize_t)
       defer: bindings_builder_free(b)
       for (name, value) in pr.dict:
         if name.isSymbol:
-          nix.bindings_builder_insert(b, name.symbol.string, nix.translate(state, value))
+          checkError nix.bindings_builder_insert(b, name.symbol.string, nix.translate(state, value))
         else:
-          nix.bindings_builder_insert(b, $name, nix.translate(state, value))
-      nix.make_attrs(result, b)
+          checkError nix.bindings_builder_insert(b, $name, nix.translate(state, value))
+      checkError nix.make_attrs(result, b)
     of pkEmbedded:
       raise newException(ValueError, "cannot convert Preserves embedded value to Nix")
   except CatchableError as err:
@@ -183,9 +182,9 @@ proc toNix*(pr: preserves.Value; state: EvalState): NixValue =
     result = nix.translate(state, pr)
 
 proc step*(state: EvalState; nv: NixValue; path: openarray[preserves.Value]): Option[preserves.Value] =
-  var nv = callThru(state, nv)
   mitNix:
     var
+      nv = nix.callThru(state, nv)
       i = 0
     while i < path.len:
       if nv.isNil: return
@@ -223,3 +222,17 @@ proc realise*(nix: NixContext; state: EvalState; val: NixValue): Value =
 
 proc realise*(state: EvalState; val: NixValue): Value =
   mitNix: result = nix.realise(state, val)
+
+proc initNull*(state: EvalState): NixValue =
+  mitNix:
+    result = nix.alloc_value(state)
+    checkError nix.init_null(result)
+
+proc isFunc*(value: NixValue): bool =
+  mitNix: result = nix.get_type(value) == NIX_TYPE_FUNCTION
+
+proc isNull*(value: NixValue): bool =
+  mitNix: result = nix.get_type(value) == NIX_TYPE_NULL
+
+proc isThunk*(value: NixValue): bool =
+  mitNix: result = nix.get_type(value) == NIX_TYPE_THUNK
diff --git a/src/nix_actor/utils.nim b/src/nix_actor/utils.nim
index fd28ba1e7b6a..84b3e0ea65fb 100644
--- a/src/nix_actor/utils.nim
+++ b/src/nix_actor/utils.nim
@@ -14,11 +14,13 @@ proc newException(ctx: NixContext): ref NixException =
   if n > 0:
     copyMem(result.msg[0].addr, p, result.msg.len)
 
+template checkError*(code: nix_err) =
+  if code != NIX_OK: raise newException(nix)
+
 template mitNix*(body: untyped): untyped =
   ## Mit nix machen.
   block:
     var nix {.inject.} = c_context_create()
     defer: c_context_free(nix)
     body
-    if err_code(nix) != NIX_OK:
-      let err = newException(nix)
+    checkError err_code(nix)