libexpr/primops: Migrate primops to return Values

A significant part of #1136.
This is a rather large cl, because all of the primops need to be changed
together.

Change-Id: I7d92698d5344bd6186ee4fa47f5c21966a6a6964

lix/libexpr/builtins/scopedImport.md

@@ -1,6 +1,6 @@
 ---
 name: scopedImport
-implementation: "[](EvalState & state, Value ** args, Value & v) { v = import(state, *args[1], args[0]); }"
+implementation: "[](EvalState & state, Value ** args) -> Value { return import(state, *args[1], args[0]); }"
 args: [scope, path]
 renameInGlobalScope: false
 ---

lix/libexpr/eval.cc

@@ -1202,7 +1202,7 @@ Value EvalState::callFunction(Value & fun, std::span<Value> args, const PosIdx p
                     for (unsigned i = 0; i < argsLeft; i++) {
                         pargs[i] = &args[i];
                     }
-                    fn->fun(*this, pargs.data(), vCur);
+                    vCur = fn->fun(*this, pargs.data());
                 } catch (ThrownError & e) {
                     // Distinguish between an error that simply happened while "throw"
                     // was being evaluated and an explicit thrown error.
@@ -1257,7 +1257,7 @@ Value EvalState::callFunction(Value & fun, std::span<Value> args, const PosIdx p
                     // 1. Unify this and above code. Heavily redundant.
                     // 2. Create a fake env (arg1, arg2, etc.) and a fake expr (arg1: arg2: etc: builtins.name arg1 arg2 etc)
                     //    so the debugger allows to inspect the wrong parameters passed to the builtin.
-                    fn->fun(*this, vArgs.data(), vCur);
+                    vCur = fn->fun(*this, vArgs.data());
                 } catch (Error & e) {
                     e.addTrace(ctx.positions[pos], "while calling the '%1%' builtin", fn->name);
                     throw;

lix/libexpr/extra-primops.hh

@@ -8,25 +8,24 @@ namespace nix {
 class EvalState;
 struct Value;
 
-void prim_addDrvOutputDependencies(EvalState & state, Value * * args, Value & v);
-void prim_fetchTree(EvalState & state, Value * * args, Value & v);
-void prim_fetchGit(EvalState & state, Value * * args, Value & v);
-void prim_fetchMercurial(EvalState & state, Value ** args, Value & v);
-void prim_fetchTarball(EvalState & state, Value * * args, Value & v);
-void prim_fetchurl(EvalState & state, Value * * args, Value & v);
-void prim_fromTOML(EvalState & state, Value * * args, Value & v);
-void prim_appendContext(EvalState & state, Value ** args, Value & v);
-void prim_getContext(EvalState & state, Value * * args, Value & v);
-void prim_hasContext(EvalState & state, Value * * args, Value & v);
-void prim_unsafeDiscardOutputDependency(EvalState & state, Value * * args, Value & v);
-void prim_unsafeDiscardStringContext(EvalState & state, Value ** args, Value & v);
+Value prim_addDrvOutputDependencies(EvalState & state, Value ** args);
+Value prim_fetchTree(EvalState & state, Value ** args);
+Value prim_fetchGit(EvalState & state, Value ** args);
+Value prim_fetchMercurial(EvalState & state, Value ** args);
+Value prim_fetchTarball(EvalState & state, Value ** args);
+Value prim_fetchurl(EvalState & state, Value ** args);
+Value prim_fromTOML(EvalState & state, Value ** args);
+Value prim_appendContext(EvalState & state, Value ** args);
+Value prim_getContext(EvalState & state, Value ** args);
+Value prim_hasContext(EvalState & state, Value ** args);
+Value prim_unsafeDiscardOutputDependency(EvalState & state, Value ** args);
+Value prim_unsafeDiscardStringContext(EvalState & state, Value ** args);
 
 namespace flake {
 
-void prim_flakeRefToString(EvalState & state, Value * * args, Value & v);
-void prim_getFlake(EvalState & state, Value * * args, Value & v);
-void prim_parseFlakeRef(EvalState & state, Value * * args, Value & v);
-
+Value prim_flakeRefToString(EvalState & state, Value ** args);
+Value prim_getFlake(EvalState & state, Value ** args);
+Value prim_parseFlakeRef(EvalState & state, Value ** args);
 }
 
 }

lix/libexpr/flake/flake.cc

@@ -966,14 +966,14 @@ Value callFlake(EvalState & state, const LockedFlake & lockedFlake)
     return state.callFunction(vTmp2, vRootSubdir, noPos);
 }
 
-void prim_getFlake(EvalState & state, Value * * args, Value & v)
+Value prim_getFlake(EvalState & state, Value ** args)
 {
     std::string flakeRefS(state.forceStringNoCtx(*args[0], noPos, "while evaluating the argument passed to builtins.getFlake"));
     auto flakeRef = parseFlakeRef(flakeRefS, {}, true);
     if (evalSettings.pureEval && !flakeRef.input.isLocked())
         throw Error("cannot call 'getFlake' on unlocked flake reference '%s' (use --impure to override)", flakeRefS);
 
-    v = callFlake(
+    return callFlake(
         state,
         lockFlake(
             state,
@@ -988,10 +988,7 @@ void prim_getFlake(EvalState & state, Value * * args, Value & v)
     );
 }
 
-void prim_parseFlakeRef(
-    EvalState & state,
-    Value * * args,
-    Value & v)
+Value prim_parseFlakeRef(EvalState & state, Value ** args)
 {
     std::string flakeRefS(state.forceStringNoCtx(*args[0], noPos,
         "while evaluating the argument passed to builtins.parseFlakeRef"));
@@ -1009,13 +1006,10 @@ void prim_parseFlakeRef(
         );
         binds.insert(s, vv);
     }
-    v = {NewValueAs::attrs, binds};
+    return {NewValueAs::attrs, binds};
 }
 
-void prim_flakeRefToString(
-    EvalState & state,
-    Value * * args,
-    Value & v)
+Value prim_flakeRefToString(EvalState & state, Value ** args)
 {
     state.forceAttrs(*args[0], noPos,
         "while evaluating the argument passed to builtins.flakeRefToString");
@@ -1047,7 +1041,7 @@ void prim_flakeRefToString(
         }
     }
     auto flakeRef = FlakeRef::fromAttrs(attrs);
-    v = {NewValueAs::string, flakeRef.to_string()};
+    return {NewValueAs::string, flakeRef.to_string()};
 }
 
 }

lix/libexpr/primops.hh

@@ -48,12 +48,12 @@ public:
 /**
  * Load a ValueInitializer from a DSO and return whatever it initializes
  */
-void prim_importNative(EvalState & state, Value * * args, Value & v);
+Value prim_importNative(EvalState & state, Value ** args);
 
 /**
  * Execute a program and parse its output
  */
-void prim_exec(EvalState & state, Value * * args, Value & v);
+Value prim_exec(EvalState & state, Value ** args);
 
 std::tuple<Value, Value> makePositionThunks(EvalState & state, const PosIdx pos);
 }

lix/libexpr/primops/context.cc

@@ -7,22 +7,21 @@
 
 namespace nix {
 
-void prim_unsafeDiscardStringContext(EvalState & state, Value ** args, Value & v)
+Value prim_unsafeDiscardStringContext(EvalState & state, Value ** args)
 {
     NixStringContext context;
     auto s = state.coerceToString(noPos, *args[0], context, "while evaluating the argument passed to builtins.unsafeDiscardStringContext");
-    v = {NewValueAs::string, *s};
+    return {NewValueAs::string, *s};
 }
 
-void prim_hasContext(EvalState & state, Value * * args, Value & v)
+Value prim_hasContext(EvalState & state, Value ** args)
 {
     NixStringContext context;
     state.forceString(*args[0], context, noPos, "while evaluating the argument passed to builtins.hasContext");
-    v = {NewValueAs::boolean, !context.empty()};
+    return {NewValueAs::boolean, !context.empty()};
 }
 
-
-void prim_unsafeDiscardOutputDependency(EvalState & state, Value * * args, Value & v)
+Value prim_unsafeDiscardOutputDependency(EvalState & state, Value ** args)
 {
     NixStringContext context;
     auto s = state.coerceToString(noPos, *args[0], context, "while evaluating the argument passed to builtins.unsafeDiscardOutputDependency");
@@ -39,11 +38,10 @@ void prim_unsafeDiscardOutputDependency(EvalState & state, Value * * args, Value
         }
     }
 
-    v = {NewValueAs::string, *s, context2};
+    return {NewValueAs::string, *s, context2};
 }
 
-
-void prim_addDrvOutputDependencies(EvalState & state, Value * * args, Value & v)
+Value prim_addDrvOutputDependencies(EvalState & state, Value ** args)
 {
     NixStringContext context;
     auto s = state.coerceToString(noPos, *args[0], context, "while evaluating the argument passed to builtins.addDrvOutputDependencies");
@@ -82,7 +80,7 @@ void prim_addDrvOutputDependencies(EvalState & state, Value * * args, Value & v)
         }, context.begin()->raw) }),
     };
 
-    v = {NewValueAs::string, *s, context2};
+    return {NewValueAs::string, *s, context2};
 }
 
 
@@ -105,7 +103,7 @@ void prim_addDrvOutputDependencies(EvalState & state, Value * * args, Value & v)
    Note that for a given path any combination of the above attributes
    may be present.
 */
-void prim_getContext(EvalState & state, Value * * args, Value & v)
+Value prim_getContext(EvalState & state, Value ** args)
 {
     struct ContextInfo {
         bool path = false;
@@ -148,7 +146,7 @@ void prim_getContext(EvalState & state, Value * * args, Value & v)
         attrs.insert(state.ctx.store->printStorePath(info.first), {NewValueAs::attrs, infoAttrs});
     }
 
-    v = {NewValueAs::attrs, attrs};
+    return {NewValueAs::attrs, attrs};
 }
 
 
@@ -157,7 +155,7 @@ void prim_getContext(EvalState & state, Value * * args, Value & v)
    See the commentary above unsafeGetContext for details of the
    context representation.
 */
-void prim_appendContext(EvalState & state, Value ** args, Value & v)
+Value prim_appendContext(EvalState & state, Value ** args)
 {
     NixStringContext context;
     auto orig = state.forceString(*args[0], context, noPos, "while evaluating the first argument passed to builtins.appendContext");
@@ -231,6 +229,6 @@ void prim_appendContext(EvalState & state, Value ** args, Value & v)
         }
     }
 
-    v = {NewValueAs::string, orig, context};
+    return {NewValueAs::string, orig, context};
 }
 }

lix/libexpr/primops/fetchMercurial.cc

@@ -5,7 +5,7 @@
 
 namespace nix {
 
-void prim_fetchMercurial(EvalState & state, Value ** args, Value & v)
+Value prim_fetchMercurial(EvalState & state, Value ** args)
 {
     std::string url;
     std::optional<Hash> rev;
@@ -98,8 +98,8 @@ void prim_fetchMercurial(EvalState & state, Value ** args, Value & v)
     attrs2.insert("shortRev", {NewValueAs::string, rev2.gitRev().substr(0, 12)});
     if (auto revCount = input2.getRevCount())
         attrs2.insert("revCount", {NewValueAs::integer, NixInt::Inner(*revCount)});
-    v = {NewValueAs::attrs, attrs2};
 
     state.ctx.paths.allowPath(tree.storePath);
+    return {NewValueAs::attrs, attrs2};
 }
 }

lix/libexpr/primops/fetchTree.cc

@@ -228,9 +228,9 @@ static Value fetchTree(
     return emitTreeAttrs(state.ctx, tree, input2, params.emptyRevFallback, false);
 }
 
-void prim_fetchTree(EvalState & state, Value * * args, Value & v)
+Value prim_fetchTree(EvalState & state, Value ** args)
 {
-    v = fetchTree(state, noPos, args, std::nullopt, FetchTreeParams{.allowNameArgument = false});
+    return fetchTree(state, noPos, args, std::nullopt, FetchTreeParams{.allowNameArgument = false});
 }
 
 static Value fetch(
@@ -334,19 +334,19 @@ static Value fetch(
     return state.ctx.paths.allowAndSetStorePathString(storePath);
 }
 
-void prim_fetchurl(EvalState & state, Value * * args, Value & v)
+Value prim_fetchurl(EvalState & state, Value ** args)
 {
-    v = fetch(state, noPos, args, "fetchurl", false, "");
+    return fetch(state, noPos, args, "fetchurl", false, "");
 }
 
-void prim_fetchTarball(EvalState & state, Value * * args, Value & v)
+Value prim_fetchTarball(EvalState & state, Value ** args)
 {
-    v = fetch(state, noPos, args, "fetchTarball", true, "source");
+    return fetch(state, noPos, args, "fetchTarball", true, "source");
 }
 
-void prim_fetchGit(EvalState & state, Value * * args, Value & v)
+Value prim_fetchGit(EvalState & state, Value ** args)
 {
-    v = fetchTree(
+    return fetchTree(
         state, noPos, args, "git", FetchTreeParams{.emptyRevFallback = true, .allowNameArgument = true}
     );
 }

lix/libexpr/primops/fromTOML.cc

@@ -7,7 +7,7 @@
 
 namespace nix {
 
-void prim_fromTOML(EvalState & state, Value ** args, Value & val)
+Value prim_fromTOML(EvalState & state, Value ** args)
 {
     auto toml = state.forceStringNoCtx(
         *args[0], noPos, "while evaluating the argument passed to builtins.fromTOML"
@@ -57,7 +57,7 @@ void prim_fromTOML(EvalState & state, Value ** args, Value & val)
     };
 
     try {
-        val = visit(
+        return visit(
             toml::parse(
                 tomlStream,
                 "fromTOML", /* the "filename" */

lix/libexpr/value.hh

@@ -32,7 +32,7 @@ struct Value;
 /**
  * Function that implements a primop.
  */
-using PrimOpImpl = void(EvalState & state, Value ** args, Value & v);
+using PrimOpImpl = Value(EvalState & state, Value ** args);
 
 /**
  * Info about a primitive operation, and its implementation

tests/functional/plugins/plugintest.cc

@@ -25,13 +25,13 @@ static void maybeRequireMeowForDlopen() {
     meow();
 }
 
-static void prim_anotherNull (EvalState & state, Value ** args, Value & v)
+static Value prim_anotherNull(EvalState & state, Value ** args)
 {
     assert(entryCalled);
     if (mySettings.settingSet)
-        v = Value::VNULL;
+        return Value::VNULL;
     else
-        v = {NewValueAs::boolean, false};
+        return {NewValueAs::boolean, false};
 }
 
 extern "C" void nix_plugin_entry()