From 3f6bfcc930c40ba2791a09deadcec04d25aaea96 Mon Sep 17 00:00:00 2001 From: Qyriad Date: Tue, 2 Jul 2024 17:29:19 -0600 Subject: [PATCH 1/7] WIP: give ExprInheritFrom a handle to what its standing in for --- src/libexpr/nixexpr.hh | 7 +++++-- src/libexpr/parser/parser.cc | 15 ++++++++++----- src/libexpr/parser/state.hh | 2 +- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index 418f888b3..981bf201b 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -144,7 +144,10 @@ struct ExprVar : Expr */ struct ExprInheritFrom : ExprVar { - ExprInheritFrom(PosIdx pos, Displacement displ): ExprVar(pos, {}) + std::shared_ptr fromExpr; + + ExprInheritFrom(PosIdx pos, Displacement displ, std::shared_ptr fromExpr) + : ExprVar(pos, {}), fromExpr(fromExpr) { this->level = 0; this->displ = displ; @@ -222,7 +225,7 @@ struct ExprAttrs : Expr }; typedef std::map AttrDefs; AttrDefs attrs; - std::unique_ptr>> inheritFromExprs; + std::unique_ptr>> inheritFromExprs; struct DynamicAttrDef { std::unique_ptr nameExpr, valueExpr; PosIdx pos; diff --git a/src/libexpr/parser/parser.cc b/src/libexpr/parser/parser.cc index 850f1276e..a5a6c0f85 100644 --- a/src/libexpr/parser/parser.cc +++ b/src/libexpr/parser/parser.cc @@ -317,18 +317,23 @@ template<> struct BuildAST : change_head { }); } } - if (auto fromE = std::move(s.from)) { + if (s.from != nullptr) { if (!b.attrs.inheritFromExprs) - b.attrs.inheritFromExprs = std::make_unique>>(); - b.attrs.inheritFromExprs->push_back(std::move(fromE)); + b.attrs.inheritFromExprs = std::make_unique>>(); + std::shared_ptr fromExpr = std::move(s.from); + b.attrs.inheritFromExprs->push_back(fromExpr); for (auto & [i, iPos] : s.attrs) { if (attrs.find(i.symbol) != attrs.end()) ps.dupAttr(i.symbol, iPos, attrs[i.symbol].pos); - auto from = std::make_unique(s.fromPos, b.attrs.inheritFromExprs->size() - 1); + auto inheritFrom = std::make_unique( + s.fromPos, + b.attrs.inheritFromExprs->size() - 1, + fromExpr + ); attrs.emplace( i.symbol, ExprAttrs::AttrDef( - std::make_unique(iPos, std::move(from), i.symbol), + std::make_unique(iPos, std::move(inheritFrom), i.symbol), iPos, ExprAttrs::AttrDef::Kind::InheritedFrom)); } diff --git a/src/libexpr/parser/state.hh b/src/libexpr/parser/state.hh index f5a0428d7..b26f01fb6 100644 --- a/src/libexpr/parser/state.hh +++ b/src/libexpr/parser/state.hh @@ -103,7 +103,7 @@ inline void State::addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_ auto * jAttrs = dynamic_cast(j->second.e.get()); if (jAttrs && ae) { if (ae->inheritFromExprs && !jAttrs->inheritFromExprs) - jAttrs->inheritFromExprs = std::make_unique>>(); + jAttrs->inheritFromExprs = std::make_unique>>(); for (auto & ad : ae->attrs) { auto j2 = jAttrs->attrs.find(ad.first); if (j2 != jAttrs->attrs.end()) // Attr already defined in iAttrs, error. From 3705b31ed51e09dbe6d894c5f4d238b9d1cbcb5e Mon Sep 17 00:00:00 2001 From: Qyriad Date: Mon, 24 Jun 2024 14:33:12 -0600 Subject: [PATCH 2/7] add an impl of Expr::show for ExprInheritFrom that doesn't crash ExprVar::show() assumes it has a name. dynamic inherits do not necessarily (ever?) have a name. Change-Id: If10893188e307431da17f0c1bd0787adc74f7141 --- src/libexpr/nixexpr.cc | 10 ++++++++++ src/libexpr/nixexpr.hh | 3 ++- tests/unit/libexpr/expr-print.cc | 32 ++++++++++++++++++++++++++++++++ tests/unit/meson.build | 1 + 4 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 tests/unit/libexpr/expr-print.cc diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index bc53ca053..2d2fe1fcc 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -50,6 +50,16 @@ void ExprVar::show(const SymbolTable & symbols, std::ostream & str) const str << symbols[name]; } +void ExprInheritFrom::show(SymbolTable const & symbols, std::ostream & str) const +{ + if (name) { + str << symbols[name]; + } else { + // We can't get at the actual dynamic expression from here. + str << "(dynamic inherit)"; + } +} + void ExprSelect::show(const SymbolTable & symbols, std::ostream & str) const { str << "("; diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index 981bf201b..34b25e3b4 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -154,7 +154,8 @@ struct ExprInheritFrom : ExprVar this->fromWith = nullptr; } - void bindVars(EvalState & es, const std::shared_ptr & env); + void show(SymbolTable const & symbols, std::ostream & str) const override; + void bindVars(EvalState & es, const std::shared_ptr & env) override; }; struct ExprSelect : Expr diff --git a/tests/unit/libexpr/expr-print.cc b/tests/unit/libexpr/expr-print.cc new file mode 100644 index 000000000..dc0ee27ed --- /dev/null +++ b/tests/unit/libexpr/expr-print.cc @@ -0,0 +1,32 @@ +#include +#include + +#include + +#include "tests/libexpr.hh" + +#include "nixexpr.hh" + +namespace nix +{ + +using namespace testing; +struct ExprPrintingTests : LibExprTest +{ + void test(Expr const & expr, std::string_view expected) + { + std::stringstream out; + expr.show(state.symbols, out); + ASSERT_EQ(out.str(), expected); + } +}; + +TEST_F(ExprPrintingTests, ExprInheritFrom) +{ + // ExprInheritFrom has its own show() impl. + // If it uses its parent class's impl it will crash. + ExprInheritFrom const eInheritFrom(noPos, 0); + test(eInheritFrom, "(dynamic inherit)"); +} + +} diff --git a/tests/unit/meson.build b/tests/unit/meson.build index 2b5471526..e2ca95629 100644 --- a/tests/unit/meson.build +++ b/tests/unit/meson.build @@ -185,6 +185,7 @@ libexpr_tests_sources = files( 'libexpr/primops.cc', 'libexpr/search-path.cc', 'libexpr/trivial.cc', + 'libexpr/expr-print.cc', 'libexpr/value/context.cc', 'libexpr/value/print.cc', ) From 5cf353692a99607802a3977dd0928002583c3e67 Mon Sep 17 00:00:00 2001 From: Qyriad Date: Sat, 22 Jun 2024 21:17:41 -0600 Subject: [PATCH 3/7] add an ExprPrinter class, like ValuePrinter To be used Shortly Change-Id: I9def7975aa55f251eb8486391677771f7352d7ce --- src/libexpr/print.cc | 6 ++++++ src/libexpr/print.hh | 30 ++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index e387a09fb..87db004b2 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -574,4 +574,10 @@ fmt_internal::HintFmt & fmt_internal::HintFmt::operator%(const ValuePrinter & va return *this; } +std::ostream & operator<<(std::ostream & output, ExprPrinter const & printer) +{ + printer.expr.show(printer.state.symbols, output); + return output; +} + } diff --git a/src/libexpr/print.hh b/src/libexpr/print.hh index 42826d94d..3deaa33d4 100644 --- a/src/libexpr/print.hh +++ b/src/libexpr/print.hh @@ -15,6 +15,8 @@ namespace nix { +struct Expr; + class EvalState; struct Value; @@ -50,6 +52,12 @@ void printValue(EvalState & state, std::ostream & str, Value & v, PrintOptions o /** * A partially-applied form of `printValue` which can be formatted using `<<` * without allocating an intermediate string. + * This class should not outlive the eval state or it will UAF. + * FIXME: This should take `nix::ref`s, to avoid that, but our eval methods all have + * EvalState &, not ref, and constructing a new shared_ptr to data that + * already has a shared_ptr is a much bigger footgun. In the current architecture of + * libexpr, using a ValuePrinter after an EvalState has been destroyed would be + * pretty hard. */ class ValuePrinter { friend std::ostream & operator << (std::ostream & output, const ValuePrinter & printer); @@ -73,4 +81,26 @@ std::ostream & operator<<(std::ostream & output, const ValuePrinter & printer); template<> fmt_internal::HintFmt & fmt_internal::HintFmt::operator%(const ValuePrinter & value); +/** + * A partially-applied form of Expr::show(), which can be formatted using `<<` + * without allocating an intermediate string. + * This class should not outlive the eval state or it will UAF. + * FIXME: This should take `nix::ref`s, to avoid that, but our eval methods all have + * EvalState &, not ref, and constructing a new shared_ptr to data that + * already has a shared_ptr is a much bigger footgun. In the current architecture of + * libexpr, using an ExprPrinter after an EvalState has been destroyed would be + * pretty hard. + */ +class ExprPrinter +{ + /** The eval state used to get symbols. */ + EvalState const & state; + /** The expression to print. */ + Expr const & expr; + +public: + ExprPrinter(EvalState const & state, Expr const & expr) : state(state), expr(expr) { } + friend std::ostream & operator << (std::ostream & output, ExprPrinter const & printer); +}; + } From 738ec33ff3cfd94ea549ee6bfe7d3208f92a91c0 Mon Sep 17 00:00:00 2001 From: Qyriad Date: Sat, 22 Jun 2024 21:22:29 -0600 Subject: [PATCH 4/7] trace when the `foo` part of `foo.bar.baz` errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Turns errors like: let errpkg = throw "invalid foobar"; in errpkg.meta error: … while calling the 'throw' builtin at «string»:2:12: 1| let 2| errpkg = throw "invalid foobar"; | ^ 3| in errpkg.meta error: invalid foobar into errors like: let errpkg = throw "invalid foobar"; in errpkg.meta error: … while evaluating 'errpkg' to select 'meta' on it at «string»:3:4: 2| errpkg = throw "invalid foobar"; 3| in errpkg.meta | ^ … while calling the 'throw' builtin at «string»:2:12: 1| let 2| errpkg = throw "invalid foobar"; | ^ 3| in errpkg.meta error: invalid foobar For the low price of one try/catch, you too can have the incorrect line of code actually show up in the trace! Change-Id: If8d6200ec1567706669d405c34adcd7e2d2cd29d --- src/libexpr/eval.cc | 15 ++++++++++++++- tests/functional/lang/eval-fail-recursion.err.exp | 6 ++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index a6a64a43c..b01867a09 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1432,7 +1432,20 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) Value * vCurrent = &vFirst; // Position for the current attrset Value in this select chain. PosIdx posCurrent; - e->eval(state, env, vFirst); + + try { + e->eval(state, env, vFirst); + } catch (Error & e) { + assert(this->e != nullptr); + state.addErrorTrace( + e, + getPos(), + "while evaluating '%s' to select '%s' on it", + ExprPrinter(state, *this->e), + showAttrPath(state.symbols, this->attrPath) + ); + throw; + } try { auto dts = state.debugRepl diff --git a/tests/functional/lang/eval-fail-recursion.err.exp b/tests/functional/lang/eval-fail-recursion.err.exp index 19380dc65..f0057b2d5 100644 --- a/tests/functional/lang/eval-fail-recursion.err.exp +++ b/tests/functional/lang/eval-fail-recursion.err.exp @@ -1,4 +1,10 @@ error: + … while evaluating 'a' to select 'foo' on it + at /pwd/lang/eval-fail-recursion.nix:1:21: + 1| let a = {} // a; in a.foo + | ^ + 2| + … in the right operand of the update (//) operator at /pwd/lang/eval-fail-recursion.nix:1:12: 1| let a = {} // a; in a.foo From 11ee13f06e25d7b44279040446762f7b13a8cad7 Mon Sep 17 00:00:00 2001 From: Qyriad Date: Sat, 22 Jun 2024 21:59:47 -0600 Subject: [PATCH 5/7] trace which part of `foo.bar.baz` errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Turns errors like: let somepkg.src = throw "invalid foobar"; in somepkg.src.meta error: … while evaluating the attribute 'src.meta' at «string»:2:3: 1| let 2| somepkg.src = throw "invalid foobar"; | ^ 3| in somepkg.src.meta … while calling the 'throw' builtin at «string»:2:17: 1| let 2| somepkg.src = throw "invalid foobar"; | ^ 3| in somepkg.src.meta error: invalid foobar into errors like: let somepkg.src = throw "invalid foobar"; in somepkg.src.meta error: … while evaluating the attribute 'src.meta' at «string»:2:3: 1| let 2| somepkg.src = throw "invalid foobar"; | ^ 3| in somepkg.src.meta … while evaluating 'somepkg.src' to select 'meta' on it at «string»:3:4: 2| somepkg.src = throw "invalid foobar"; 3| in somepkg.src.meta | ^ … while calling the 'throw' builtin at «string»:2:17: 1| let 2| somepkg.src = throw "invalid foobar"; | ^ 3| in somepkg.src.meta error: invalid foobar And for type errors, from: let somepkg.src = "I'm not an attrset"; in somepkg.src.meta error: … while evaluating the attribute 'src.meta' at «string»:2:3: 1| let 2| somepkg.src = "I'm not an attrset"; | ^ 3| in somepkg.src.meta … while selecting an attribute at «string»:3:4: 2| somepkg.src = "I'm not an attrset"; 3| in somepkg.src.meta | ^ error: expected a set but found a string: "I'm not an attrset" into: let somepkg.src = "I'm not an attrset"; in somepkg.src.meta error: … while evaluating the attribute 'src.meta' at «string»:2:3: 1| let 2| somepkg.src = "I'm not an attrset"; | ^ 3| in somepkg.src.meta … while selecting 'meta' on 'somepkg.src' at «string»:3:4: 2| somepkg.src = "I'm not an attrset"; 3| in somepkg.src.meta | ^ error: expected a set but found a string: "I'm not an attrset" For the low price of an enumerate() and a lambda you too can have the incorrect line of code actually show up in the trace! Change-Id: Ic1491c86e33c167891bdac9adad6224784760bd6 --- src/libexpr/eval.cc | 43 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index b01867a09..56581cc19 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1458,12 +1458,46 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) showAttrPath(state, env, attrPath)) : nullptr; - for (auto const & currentAttrName : attrPath) { + for (auto const & [partIdx, currentAttrName] : enumerate(attrPath)) { state.nrLookups++; Symbol const name = getName(currentAttrName, state, env); - state.forceValue(*vCurrent, pos); + // For formatting errors, which should be done only when needed. + auto partsSoFar = [&]() -> std::string { + std::stringstream ss; + // We start with the base thing this ExprSelect is selecting on. + assert(this->e != nullptr); + this->e->show(state.symbols, ss); + + // Then grab each part of the attr path up to this one. + assert(partIdx < attrPath.size()); + std::span const parts( + attrPath.begin(), + attrPath.begin() + partIdx + ); + + // And convert them to strings and join them. + for (auto const & part : parts) { + auto const partName = getName(part, state, env); + ss << "." << state.symbols[partName]; + } + + return ss.str(); + }; + + try { + state.forceValue(*vCurrent, pos); + } catch (Error & e) { + state.addErrorTrace( + e, + getPos(), + "while evaluating '%s' to select '%s' on it", + partsSoFar(), + state.symbols[name] + ); + throw; + } if (vCurrent->type() != nAttrs) { @@ -1479,7 +1513,10 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) "expected a set but found %s: %s", showType(*vCurrent), ValuePrinter(state, *vCurrent, errorPrintOptions) - ).withTrace(pos, "while selecting an attribute").debugThrow(); + ).addTrace( + pos, + HintFmt("while selecting '%s' on '%s'", state.symbols[name], partsSoFar()) + ).debugThrow(); } // Now that we know this is actually an attrset, try to find an attr From dccd84f2f9cf84580f0212c2cff86eb9dc0a11e0 Mon Sep 17 00:00:00 2001 From: Qyriad Date: Mon, 24 Jun 2024 17:26:21 -0600 Subject: [PATCH 6/7] distinguish between throws & errors during throw MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Turns errors like this: let throwMsg = a: throw (a + " invalid bar"); in throwMsg "bullshit" error: … from call site at «string»:3:4: 2| throwMsg = a: throw (a + " invalid bar"); 3| in throwMsg "bullshit" | ^ … while calling 'throwMsg' at «string»:2:14: 1| let 2| throwMsg = a: throw (a + " invalid bar"); | ^ 3| in throwMsg "bullshit" … while calling the 'throw' builtin at «string»:2:17: 1| let 2| throwMsg = a: throw (a + " invalid bar"); | ^ 3| in throwMsg "bullshit" error: bullshit invalid bar into errors like this: let throwMsg = a: throw (a + " invalid bar"); in throwMsg "bullshit" error: … from call site at «string»:3:4: 2| throwMsg = a: throw (a + " invalid bar"); 3| in throwMsg "bullshit" | ^ … while calling 'throwMsg' at «string»:2:14: 1| let 2| throwMsg = a: throw (a + " invalid bar"); | ^ 3| in throwMsg "bullshit" … caused by explicit throw at «string»:2:17: 1| let 2| throwMsg = a: throw (a + " invalid bar"); | ^ 3| in throwMsg "bullshit" error: bullshit invalid bar Change-Id: I593688928ece20f97999d1bf03b2b46d9ac338cb --- src/libexpr/eval.cc | 9 +++++++++ tests/functional/eval.sh | 2 +- tests/functional/lang/eval-fail-duplicate-traces.err.exp | 2 +- .../eval-fail-foldlStrict-strict-op-application.err.exp | 2 +- tests/functional/lang/eval-fail-mutual-recursion.err.exp | 2 +- tests/functional/lang/eval-fail-not-throws.err.exp | 2 +- tests/functional/lang/eval-fail-toJSON.err.exp | 2 +- 7 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 56581cc19..d9bdb0d2c 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1807,6 +1807,15 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & try { fn->fun(*this, vCur.determinePos(noPos), args, vCur); + } catch (ThrownError & e) { + // Distinguish between an error that simply happened while "throw" + // was being evaluated and an explicit thrown error. + if (fn->name == "throw") { + addErrorTrace(e, pos, "caused by explicit %s", "throw"); + } else { + addErrorTrace(e, pos, "while calling the '%s' builtin", fn->name); + } + throw; } catch (Error & e) { addErrorTrace(e, pos, "while calling the '%1%' builtin", fn->name); throw; diff --git a/tests/functional/eval.sh b/tests/functional/eval.sh index 9c125b569..ae6fcec63 100644 --- a/tests/functional/eval.sh +++ b/tests/functional/eval.sh @@ -25,7 +25,7 @@ nix eval -E 'assert 1 + 2 == 3; true' # Top-level eval errors should be printed to stderr with a traceback. topLevelThrow="$(expectStderr 1 nix eval --expr 'throw "a sample throw message"')" [[ "$topLevelThrow" =~ "a sample throw message" ]] -[[ "$topLevelThrow" =~ "while calling the 'throw' builtin" ]] +[[ "$topLevelThrow" =~ "caused by explicit throw" ]] # But errors inside something should print an elided version, and exit with 0. outputOfNestedThrow="$(nix eval --expr '{ throws = throw "a sample throw message"; }')" diff --git a/tests/functional/lang/eval-fail-duplicate-traces.err.exp b/tests/functional/lang/eval-fail-duplicate-traces.err.exp index cedaebd3b..d9e2ec945 100644 --- a/tests/functional/lang/eval-fail-duplicate-traces.err.exp +++ b/tests/functional/lang/eval-fail-duplicate-traces.err.exp @@ -41,7 +41,7 @@ error: | ^ 5| if n > 0 - … while calling the 'throw' builtin + … caused by explicit throw at /pwd/lang/eval-fail-duplicate-traces.nix:7:10: 6| then throwAfter (n - 1) 7| else throw "Uh oh!"; diff --git a/tests/functional/lang/eval-fail-foldlStrict-strict-op-application.err.exp b/tests/functional/lang/eval-fail-foldlStrict-strict-op-application.err.exp index 4903bc82d..6955fad13 100644 --- a/tests/functional/lang/eval-fail-foldlStrict-strict-op-application.err.exp +++ b/tests/functional/lang/eval-fail-foldlStrict-strict-op-application.err.exp @@ -27,7 +27,7 @@ error: | ^ 6| - … while calling the 'throw' builtin + … caused by explicit throw at /pwd/lang/eval-fail-foldlStrict-strict-op-application.nix:5:9: 4| null 5| [ (_: throw "Not the final value, but is still forced!") (_: 23) ] diff --git a/tests/functional/lang/eval-fail-mutual-recursion.err.exp b/tests/functional/lang/eval-fail-mutual-recursion.err.exp index c034afcd5..c03d2e840 100644 --- a/tests/functional/lang/eval-fail-mutual-recursion.err.exp +++ b/tests/functional/lang/eval-fail-mutual-recursion.err.exp @@ -54,7 +54,7 @@ error: (21 duplicate frames omitted) - … while calling the 'throw' builtin + … caused by explicit throw at /pwd/lang/eval-fail-mutual-recursion.nix:34:10: 33| then throwAfterB true 10 34| else throw "Uh oh!"; diff --git a/tests/functional/lang/eval-fail-not-throws.err.exp b/tests/functional/lang/eval-fail-not-throws.err.exp index fc81f7277..5882a260a 100644 --- a/tests/functional/lang/eval-fail-not-throws.err.exp +++ b/tests/functional/lang/eval-fail-not-throws.err.exp @@ -5,7 +5,7 @@ error: | ^ 2| - … while calling the 'throw' builtin + … caused by explicit throw at /pwd/lang/eval-fail-not-throws.nix:1:4: 1| ! (throw "uh oh!") | ^ diff --git a/tests/functional/lang/eval-fail-toJSON.err.exp b/tests/functional/lang/eval-fail-toJSON.err.exp index ad267711b..18c334923 100644 --- a/tests/functional/lang/eval-fail-toJSON.err.exp +++ b/tests/functional/lang/eval-fail-toJSON.err.exp @@ -40,7 +40,7 @@ error: | ^ 8| } - … while calling the 'throw' builtin + … caused by explicit throw at /pwd/lang/eval-fail-toJSON.nix:7:13: 6| { 7| c.d = throw "hah no"; From 7a66b4b9473bb260d418f18ec78f778e64089847 Mon Sep 17 00:00:00 2001 From: Qyriad Date: Mon, 24 Jun 2024 19:08:53 -0600 Subject: [PATCH 7/7] mildly cleanup libexpr/eval.hh Change-Id: I40d01a8f8b7fb101279c6f88ebdf1f0969d9d7f0 --- src/libexpr/eval.hh | 54 +++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 8e390e46d..2b52fc57f 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -9,14 +9,13 @@ #include "symbol-table.hh" #include "config.hh" #include "experimental-features.hh" -#include "input-accessor.hh" #include "search-path.hh" #include "repl-exit-status.hh" +#include #include #include #include -#include #include namespace nix { @@ -38,11 +37,29 @@ namespace eval_cache { class EvalCache; } +/** Alias for std::map which uses boehmgc's allocator conditional on us actually + * using boehmgc in this build. + */ +#if HAVE_BOEHMGC + template + using GcMap = std::map< + KeyT, + ValueT, + std::less, + traceable_allocator> + >; +#else + using GcMap = std::map +#endif + /** * Function that implements a primop. */ -typedef void (* PrimOpFun) (EvalState & state, const PosIdx pos, Value * * args, Value & v); +using PrimOpImpl = void(EvalState & state, PosIdx pos, Value ** args, Value & v); + +/** Pointer to a function that implements a primop. */ +using PrimOpFun = PrimOpImpl *; /** * Info about a primitive operation, and its implementation @@ -76,7 +93,7 @@ struct PrimOp /** * Implementation of the primop. */ - std::function::type> fun; + std::function fun; /** * Optional experimental for this to be gated on. @@ -115,11 +132,7 @@ struct Constant bool impureOnly = false; }; -#if HAVE_BOEHMGC - typedef std::map, traceable_allocator > > ValMap; -#else - typedef std::map ValMap; -#endif +using ValMap = GcMap; struct Env { @@ -214,7 +227,8 @@ public: /** * Debugger */ - ReplExitStatus (* debugRepl)(ref es, const ValMap & extraEnv); + using ReplCallbackImpl = ReplExitStatus(ref es, ValMap const & extraEnv); + ReplCallbackImpl * debugRepl; bool debugStop; bool inDebugger = false; int trylevel; @@ -252,21 +266,13 @@ private: /** * A cache from path names to parse trees. */ -#if HAVE_BOEHMGC - typedef std::map, traceable_allocator>> FileParseCache; -#else - typedef std::map FileParseCache; -#endif + using FileParseCache = GcMap; FileParseCache fileParseCache; /** * A cache from path names to values. */ -#if HAVE_BOEHMGC - typedef std::map, traceable_allocator>> FileEvalCache; -#else - typedef std::map FileEvalCache; -#endif + using FileEvalCache = GcMap; FileEvalCache fileEvalCache; SearchPath searchPath; @@ -737,15 +743,15 @@ private: bool countCalls; - typedef std::map PrimOpCalls; + using PrimOpCalls = std::map; PrimOpCalls primOpCalls; - typedef std::map FunctionCalls; + using FunctionCalls = std::map; FunctionCalls functionCalls; void incrFunctionCall(ExprLambda * fun); - typedef std::map AttrSelects; + using AttrSelects = std::map; AttrSelects attrSelects; friend struct ExprOpUpdate; @@ -787,7 +793,7 @@ std::string showType(const Value & v); */ SourcePath resolveExprPath(SourcePath path); -static const std::string corepkgsPrefix{"/__corepkgs__/"}; +static constexpr std::string_view corepkgsPrefix{"/__corepkgs__/"}; }