diff --git a/src/libexpr/eval-inline.hh b/src/libexpr/eval-inline.hh index d48871628..52aa75b5f 100644 --- a/src/libexpr/eval-inline.hh +++ b/src/libexpr/eval-inline.hh @@ -93,20 +93,12 @@ void EvalState::forceValue(Value & v, const PosIdx pos) expr->eval(*this, *env, v); } catch (...) { v.mkThunk(env, expr); + tryFixupBlackHolePos(v, pos); throw; } } - else if (v.isApp()) { - try { - callFunction(*v.app.left, *v.app.right, v, noPos); - } catch (InfiniteRecursionError & e) { - // only one black hole can *throw* in any given eval stack so we need not - // check whether the position is set already. - if (v.isBlackhole()) - e.err.errPos = positions[pos]; - throw; - } - } + else if (v.isApp()) + callFunction(*v.app.left, *v.app.right, v, pos); } diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 4dc5af97a..0c35b3713 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -256,8 +256,8 @@ std::string showType(const Value & v) case tPrimOpApp: return fmt("the partially applied built-in function '%s'", std::string(getPrimOp(v)->primOp->name)); case tExternal: return v.external->showType(); - case tThunk: return "a thunk"; - case tApp: return v.isBlackhole() ? "a black hole" : "a function application"; + case tThunk: return v.isBlackhole() ? "a black hole" : "a thunk"; + case tApp: return "a function application"; default: return std::string(showType(v.type())); } @@ -1624,14 +1624,12 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & auto * fn = vCur.primOp; nrPrimOpCalls++; - // This will count black holes, but that's ok, because unrecoverable errors are rare. if (countCalls) primOpCalls[fn->name]++; try { fn->fun(*this, vCur.determinePos(noPos), args, vCur); } catch (Error & e) { - if (!fn->hideInDiagnostics) - addErrorTrace(e, pos, "while calling the '%1%' builtin", fn->name); + addErrorTrace(e, pos, "while calling the '%1%' builtin", fn->name); throw; } @@ -1670,7 +1668,6 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & auto fn = primOp->primOp; nrPrimOpCalls++; - // This will count black holes, but that's ok, because unrecoverable errors are rare. if (countCalls) primOpCalls[fn->name]++; try { @@ -1680,8 +1677,7 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & // so the debugger allows to inspect the wrong parameters passed to the builtin. fn->fun(*this, vCur.determinePos(noPos), vArgs, vCur); } catch (Error & e) { - if (!fn->hideInDiagnostics) - addErrorTrace(e, pos, "while calling the '%1%' builtin", fn->name); + addErrorTrace(e, pos, "while calling the '%1%' builtin", fn->name); throw; } @@ -2035,6 +2031,29 @@ void ExprPos::eval(EvalState & state, Env & env, Value & v) } +void ExprBlackHole::eval(EvalState & state, Env & env, Value & v) +{ + state.error("infinite recursion encountered") + .debugThrow(); +} + +// always force this to be separate, otherwise forceValue may inline it and take +// a massive perf hit +[[gnu::noinline]] +void EvalState::tryFixupBlackHolePos(Value & v, PosIdx pos) +{ + if (!v.isBlackhole()) + return; + auto e = std::current_exception(); + try { + std::rethrow_exception(e); + } catch (InfiniteRecursionError & e) { + e.err.errPos = positions[pos]; + } catch (...) { + } +} + + void EvalState::forceValueDeep(Value & v) { std::set seen; diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 4c7ea1d98..56bc5e48f 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -77,14 +77,6 @@ struct PrimOp */ std::optional experimentalFeature; - /** - * Whether to hide this primop in diagnostics. - * - * Used to hide the fact that black holes are primop applications from - * stack traces. - */ - bool hideInDiagnostics; - /** * Validity check to be performed by functions that introduce primops, * such as RegisterPrimOp() and Value::mkPrimOp(). @@ -473,6 +465,8 @@ public: */ inline void forceValue(Value & v, const PosIdx pos); + void tryFixupBlackHolePos(Value & v, PosIdx pos); + /** * Force a value, then recursively force list elements and * attributes. diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 22be8e68c..84860b30f 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -9,6 +9,8 @@ namespace nix { +ExprBlackHole eBlackHole; + struct PosAdapter : AbstractPos { Pos::Origin origin; diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index cf6fd1a8d..1e57fec7a 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -462,6 +462,16 @@ struct ExprPos : Expr COMMON_METHODS }; +/* only used to mark thunks as black holes. */ +struct ExprBlackHole : Expr +{ + void show(const SymbolTable & symbols, std::ostream & str) const override {} + void eval(EvalState & state, Env & env, Value & v) override; + void bindVars(EvalState & es, const std::shared_ptr & env) override {} +}; + +extern ExprBlackHole eBlackHole; + /* Static environments are used to map variable names onto (level, displacement) pairs used to obtain the value of the variable at diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index b7e903667..2a71747a0 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -4263,29 +4263,6 @@ static RegisterPrimOp primop_splitVersion({ }); -static void prim_blackHoleFn(EvalState & state, const PosIdx pos, Value * * args, Value & v) -{ - state.error("infinite recursion encountered") - .debugThrow(); -} - -static PrimOp primop_blackHole = { - .name = "«blackHole»", - .args = {}, - .fun = prim_blackHoleFn, - .hideInDiagnostics = true, -}; - -static Value makeBlackHole() -{ - Value v; - v.mkPrimOp(&primop_blackHole); - return v; -} - -Value prim_blackHole = makeBlackHole(); - - /************************************************************* * Primop registration *************************************************************/ diff --git a/src/libexpr/primops.hh b/src/libexpr/primops.hh index 244eada86..45486608f 100644 --- a/src/libexpr/primops.hh +++ b/src/libexpr/primops.hh @@ -51,10 +51,4 @@ void prim_importNative(EvalState & state, const PosIdx pos, Value * * args, Valu */ void prim_exec(EvalState & state, const PosIdx pos, Value * * args, Value & v); -/** - * Placeholder value for black holes, used to represent black holes as - * applications of this value to the evaluated thunks. - */ -extern Value prim_blackHole; - } diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 52cd0f901..d9860e921 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -61,6 +61,7 @@ class Bindings; struct Env; struct Expr; struct ExprLambda; +struct ExprBlackHole; struct PrimOp; class Symbol; class PosIdx; @@ -442,16 +443,17 @@ public: }; -extern Value prim_blackHole; +extern ExprBlackHole eBlackHole; -inline bool Value::isBlackhole() const +bool Value::isBlackhole() const { - return internalType == tApp && app.left == &prim_blackHole; + return internalType == tThunk && thunk.expr == (Expr*) &eBlackHole; } -inline void Value::mkBlackhole() +void Value::mkBlackhole() { - mkApp(&prim_blackHole, &prim_blackHole); + internalType = tThunk; + thunk.expr = (Expr*) &eBlackHole; }