Fix primop callsite parameter, const primop args

src/libexpr-c/nix_api_value.cc

@@ -41,8 +41,8 @@ static void nix_c_primop_wrapper(
     void * userdata,
     int arity,
     nix::EvalState & state,
-    const nix::PosIdx pos,
-    nix::Value ** args,
+    nix::CallSite callSite,
+    nix::Value * const * args,
     nix::Value & v)
 {
     nix_c_context ctx;
@@ -73,16 +73,16 @@ static void nix_c_primop_wrapper(
     if (ctx.last_err_code != NIX_OK) {
         if (ctx.last_err_code == NIX_ERR_RECOVERABLE) {
             state.error<nix::RecoverableEvalError>("Recoverable error from custom function: %s", *ctx.last_err)
-                .atPos(pos)
+                .atPos(nix::noPos)
                 .debugThrow();
         } else {
-            state.error<nix::EvalError>("Error from custom function: %s", *ctx.last_err).atPos(pos).debugThrow();
+            state.error<nix::EvalError>("Error from custom function: %s", *ctx.last_err).atPos(nix::noPos).debugThrow();
         }
     }
 
     if (!vTmp.isValid()) {
         state.error<nix::EvalError>("Implementation error in custom function: return value was not initialized")
-            .atPos(pos)
+            .atPos(nix::noPos)
             .debugThrow();
     }
 
@@ -91,7 +91,7 @@ static void nix_c_primop_wrapper(
         // e.g. implementing tail recursion by returning a thunk to the next
         // "iteration". Until then, this is most likely a mistake or misunderstanding.
         state.error<nix::EvalError>("Implementation error in custom function: return value must not be a thunk")
-            .atPos(pos)
+            .atPos(nix::noPos)
             .debugThrow();
     }
 

src/libexpr-tests/lazy-fetcher-attr.cc

@@ -24,7 +24,7 @@ TEST_F(LazyFetcherAttrTest, nonLazyAttrProducesImmediateValue)
     input.attrs.insert_or_assign("revCount", uint64_t(5));
 
     Value v;
-    emitTreeAttrs(state, dummyPath(), input, v, false, false);
+    emitTreeAttrs(state, noPos, dummyPath(), input, v, false, false);
     state.forceValue(v, noPos);
 
     auto * rcAttr = v.attrs()->get(state.symbols.create("revCount"));
@@ -48,7 +48,7 @@ TEST_F(LazyFetcherAttrTest, lazyAttrProducesThunk)
                 }})));
 
     Value v;
-    emitTreeAttrs(state, dummyPath(), input, v, false, false);
+    emitTreeAttrs(state, noPos, dummyPath(), input, v, false, false);
     state.forceValue(v, noPos);
 
     auto * rcAttr = v.attrs()->get(state.symbols.create("revCount"));
@@ -79,7 +79,7 @@ TEST_F(LazyFetcherAttrTest, lazyFunctionOnlyCalledOnAccess)
                 }})));
 
     Value v;
-    emitTreeAttrs(state, dummyPath(), input, v, false, false);
+    emitTreeAttrs(state, noPos, dummyPath(), input, v, false, false);
     state.forceValue(v, noPos);
 
     // Access lastModified, so should not trigger lazy revCount

src/libexpr-tests/value/print.cc

@@ -125,15 +125,15 @@ TEST_F(ValuePrintingTests, vLambda)
 TEST_F(ValuePrintingTests, vPrimOp)
 {
     Value vPrimOp;
-    PrimOp primOp{.name = "puppy", .impl = [](EvalState &, const PosIdx, Value **, Value &) {}};
+    PrimOp primOp{.name = "puppy", .impl = [](EvalState &, CallSite, Value * const *, Value &) {}};
     vPrimOp.mkPrimOp(&primOp);
 
     test(vPrimOp, "«primop puppy»");
 }
 
 TEST_F(ValuePrintingTests, vPrimOpApp)
 {
-    PrimOp primOp{.name = "puppy", .impl = [](EvalState &, const PosIdx, Value **, Value &) {}};
+    PrimOp primOp{.name = "puppy", .impl = [](EvalState &, CallSite, Value * const *, Value &) {}};
     Value vPrimOp;
     vPrimOp.mkPrimOp(&primOp);
 
@@ -529,7 +529,7 @@ TEST_F(ValuePrintingTests, ansiColorsLambda)
 
 TEST_F(ValuePrintingTests, ansiColorsPrimOp)
 {
-    PrimOp primOp{.name = "puppy", .impl = [](EvalState &, const PosIdx, Value **, Value &) {}};
+    PrimOp primOp{.name = "puppy", .impl = [](EvalState &, CallSite, Value * const *, Value &) {}};
     Value v;
     v.mkPrimOp(&primOp);
 
@@ -538,7 +538,7 @@ TEST_F(ValuePrintingTests, ansiColorsPrimOp)
 
 TEST_F(ValuePrintingTests, ansiColorsPrimOpApp)
 {
-    PrimOp primOp{.name = "puppy", .impl = [](EvalState &, const PosIdx, Value **, Value &) {}};
+    PrimOp primOp{.name = "puppy", .impl = [](EvalState &, CallSite, Value * const *, Value &) {}};
     Value vPrimOp;
     vPrimOp.mkPrimOp(&primOp);
 

src/libexpr/eval.cc

@@ -1698,7 +1698,7 @@ void EvalState::callFunction(Value & fun, std::span<Value * const> args, Value &
                     primOpCalls[fn->name]++;
 
                 try {
-                    fn->impl(*this, vCur.determinePos(noPos), const_cast<Value **>(args.data()), vCur);
+                    fn->impl(*this, CallSite{pos}, args.data(), vCur);
                 } catch (Error & e) {
                     if (fn->addTrace)
                         addErrorTrace(e, pos, "while calling the '%1%' builtin", fn->name);
@@ -1748,7 +1748,7 @@ void EvalState::callFunction(Value & fun, std::span<Value * const> args, Value &
                     // 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->impl(*this, vCur.determinePos(noPos), vArgs, vCur);
+                    fn->impl(*this, CallSite{pos}, vArgs, vCur);
                 } catch (Error & e) {
                     if (fn->addTrace)
                         addErrorTrace(e, pos, "while calling the '%1%' builtin", fn->name);

src/libexpr/include/nix/expr/eval.hh

@@ -77,11 +77,32 @@ public:
     }
 };
 
+/**
+ * A position at which a primop is invoked (`noPos` or presumably an `ExprCall` location).
+ *
+ * If you're about to pass your primop call position to a different function that
+ * runs in your primop implementation, then stop, because that position has
+ * already been printed in the trace. Just don't pass a pos, or pass `noPos`.
+ *
+ * Nonetheless, we pass the call site to the primop so that it can be used
+ * in other contexts than the usual `try`/`catch`/`addTrace` flow, and
+ * specifically as extra context in delayed computations that may fail.
+ *
+ * Alternatively, a primop registration can set `PrimOp::addTrace = false`, so
+ * that the otherwise redundant trace item is suppressed, and the primop becomes
+ * responsible for printing its call site location, allowing for some
+ * customization of the trace.
+ */
+struct CallSite
+{
+    /** An already printed pos! Don't make it noisy. Read the `CallSite` comment. */
+    PosIdx pos;
+};
+
 /**
  * Function that implements a primop.
- * FIXME: `args` should be `Value * const *` instead of `Value **`, but that would be a big tedious diff.
  */
-using PrimOpFun = void(EvalState & state, const PosIdx pos, Value ** args, Value & v);
+using PrimOpFun = void(EvalState & state, CallSite callSite, Value * const * args, Value & v);
 
 /**
  * Info about a primitive operation, and its implementation
@@ -1134,9 +1155,9 @@ private:
     friend struct ExprFloat;
     friend struct ExprPath;
     friend struct ExprSelect;
-    friend void prim_getAttr(EvalState & state, const PosIdx pos, Value ** args, Value & v);
-    friend void prim_match(EvalState & state, const PosIdx pos, Value ** args, Value & v);
-    friend void prim_split(EvalState & state, const PosIdx pos, Value ** args, Value & v);
+    friend void prim_getAttr(EvalState & state, CallSite callSite, Value * const * args, Value & v);
+    friend void prim_match(EvalState & state, CallSite callSite, Value * const * args, Value & v);
+    friend void prim_split(EvalState & state, CallSite callSite, Value * const * args, Value & v);
 
     friend struct Value;
     friend class ListBuilder;

src/libexpr/include/nix/expr/fetch-tree.hh

@@ -6,9 +6,14 @@ namespace nix {
 
 /**
  * Convert a libfetchers `Input` to libexpr `Value`.
+ *
+ * @param `callPos` optional position of the `fetchTree` / `fetchGit` / ... call
+ *   that produces this attrset, added to every attribute of the returned
+ *   attrset as its `Attr::pos` for diagnostics and `unsafeGetAttrPos`.
  */
 void emitTreeAttrs(
     EvalState & state,
+    PosIdx callPos,
     const StorePath & storePath,
     const fetchers::Input & input,
     Value & v,

src/libexpr/include/nix/expr/primops.hh

@@ -29,12 +29,12 @@ struct RegisterPrimOp
 /**
  * Load a ValueInitializer from a DSO and return whatever it initializes
  */
-void prim_importNative(EvalState & state, const PosIdx pos, Value ** args, Value & v);
+void prim_importNative(EvalState & state, CallSite callSite, Value * const * args, Value & v);
 
 /**
  * Execute a program and parse its output
  */
-void prim_exec(EvalState & state, const PosIdx pos, Value ** args, Value & v);
+void prim_exec(EvalState & state, CallSite callSite, Value * const * args, Value & v);
 
 void makePositionThunks(EvalState & state, const PosIdx pos, Value & line, Value & column);