diff --git a/doc/manual/rl-next/error-message-improvements.md b/doc/manual/rl-next/error-message-improvements.md new file mode 100644 index 000000000..7e1585708 --- /dev/null +++ b/doc/manual/rl-next/error-message-improvements.md @@ -0,0 +1,21 @@ +--- +synopsis: Small error message improvements +issues: [] +cls: [2185, 2187] +category: Improvements +credits: [piegames] +--- + +When an attribute selection fails, the error message now correctly points to the attribute in the chain that failed instead of at the beginning of the entire chain. +```diff + error: attribute 'x' missing +- at /pwd/lang/eval-fail-remove.nix:4:3: ++ at /pwd/lang/eval-fail-remove.nix:4:29: + 3| in + 4| (removeAttrs attrs ["x"]).x +- | ^ ++ | ^ + 5| +``` + +Failed asserts don't print the failed assertion expression anymore in the error message. That code was buggy and the information was redundant anyways, given that the error position already more accurately shows what exactly failed. diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc index 3e1963350..6f52566a3 100644 --- a/lix/libexpr/eval.cc +++ b/lix/libexpr/eval.cc @@ -1071,12 +1071,13 @@ Env * ExprAttrs::buildInheritFromEnv(EvalState & state, Env & up) return &inheritEnv; } -void ExprAttrs::eval(EvalState & state, Env & env, Value & v) +void ExprSet::eval(EvalState & state, Env & env, Value & v) { v.mkAttrs(state.ctx.buildBindings(attrs.size() + dynamicAttrs.size()).finish()); auto dynamicEnv = &env; if (recursive) { + /* Create a new environment that contains the attributes in this `rec'. */ Env & env2(state.ctx.mem.allocEnv(attrs.size())); @@ -1084,7 +1085,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v) dynamicEnv = &env2; Env * inheritEnv = inheritFromExprs ? buildInheritFromEnv(state, env2) : nullptr; - AttrDefs::iterator overrides = attrs.find(state.ctx.s.overrides); + ExprAttrs::AttrDefs::iterator overrides = attrs.find(state.ctx.s.overrides); bool hasOverrides = overrides != attrs.end(); /* The recursive attributes are evaluated in the new @@ -1093,7 +1094,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v) Displacement displ = 0; for (auto & i : attrs) { Value * vAttr; - if (hasOverrides && i.second.kind != AttrDef::Kind::Inherited) { + if (hasOverrides && i.second.kind != ExprAttrs::AttrDef::Kind::Inherited) { vAttr = state.ctx.mem.allocValue(); vAttr->mkThunk(i.second.chooseByKind(&env2, &env, inheritEnv), *i.second.e); state.ctx.stats.nrThunks++; @@ -1118,7 +1119,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v) for (auto & i : *v.attrs) newBnds->push_back(i); for (auto & i : *vOverrides->attrs) { - AttrDefs::iterator j = attrs.find(i.name); + ExprAttrs::AttrDefs::iterator j = attrs.find(i.name); if (j != attrs.end()) { (*newBnds)[j->second.displ] = i; env2.values[j->second.displ] = i.value; @@ -1167,16 +1168,16 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v) { /* Create a new environment that contains the attributes in this `let'. */ - Env & env2(state.ctx.mem.allocEnv(attrs->attrs.size())); + Env & env2(state.ctx.mem.allocEnv(attrs.size())); env2.up = &env; - Env * inheritEnv = attrs->inheritFromExprs ? attrs->buildInheritFromEnv(state, env2) : nullptr; + Env * inheritEnv = inheritFromExprs ? buildInheritFromEnv(state, env2) : nullptr; /* The recursive attributes are evaluated in the new environment, while the inherited attributes are evaluated in the original environment. */ Displacement displ = 0; - for (auto & i : attrs->attrs) { + for (auto & i : attrs) { env2.values[displ++] = i.second.e->maybeThunk( state, *i.second.chooseByKind(&env2, &env, inheritEnv)); @@ -1234,9 +1235,7 @@ static std::string showAttrPath(EvalState & state, Env & env, const AttrPath & a out << state.ctx.symbols[getName(i, state, env)]; } catch (Error & e) { assert(!i.symbol); - out << "\"${"; - i.expr->show(state.ctx.symbols, out); - out << "}\""; + out << "\"${...}\""; } } return out.str(); @@ -1251,6 +1250,8 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) Value * vCurrent = &vFirst; // Position for the current attrset Value in this select chain. PosIdx posCurrent; + // Position for the current selector in this select chain. + PosIdx posCurrentSyntax; try { e->eval(state, env, vFirst); @@ -1258,8 +1259,7 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) assert(this->e != nullptr); e.addTrace( state.ctx.positions[getPos()], - "while evaluating '%s' to select '%s' on it", - ExprPrinter(state, *this->e), + "while evaluating an expression to select '%s' on it", showAttrPath(state.ctx.symbols, this->attrPath) ); throw; @@ -1281,36 +1281,12 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) Symbol const name = getName(currentAttrName, state, env); - // 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.ctx.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.ctx.symbols[partName]; - } - - return ss.str(); - }; - try { state.forceValue(*vCurrent, pos); } catch (Error & e) { e.addTrace( - state.ctx.positions[getPos()], - "while evaluating '%s' to select '%s' on it", - partsSoFar(), + state.ctx.positions[currentAttrName.pos], + "while evaluating an expression to select '%s' on it", state.ctx.symbols[name] ); throw; @@ -1331,8 +1307,8 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) showType(*vCurrent), ValuePrinter(state, *vCurrent, errorPrintOptions) ).addTrace( - pos, - HintFmt("while selecting '%s' on '%s'", state.ctx.symbols[name], partsSoFar()) + currentAttrName.pos, + HintFmt("while selecting '%s'", state.ctx.symbols[name]) ).debugThrow(); } @@ -1354,7 +1330,7 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) } auto suggestions = Suggestions::bestMatches(allAttrNames, state.ctx.symbols[name]); state.ctx.errors.make("attribute '%s' missing", state.ctx.symbols[name]) - .atPos(pos) + .atPos(currentAttrName.pos) .withSuggestions(suggestions) .withFrame(env, *this) .debugThrow(); @@ -1364,10 +1340,11 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) // Set our currently operated-on attrset to this one, and keep going. vCurrent = attrIt->value; posCurrent = attrIt->pos; + posCurrentSyntax = currentAttrName.pos; if (state.ctx.stats.countCalls) state.ctx.stats.attrSelects[posCurrent]++; } - state.forceValue(*vCurrent, (posCurrent ? posCurrent : this->pos)); + state.forceValue(*vCurrent, (posCurrent ? posCurrent : posCurrentSyntax)); } catch (Error & e) { auto pos2r = state.ctx.positions[posCurrent]; @@ -1826,9 +1803,10 @@ void ExprIf::eval(EvalState & state, Env & env, Value & v) void ExprAssert::eval(EvalState & state, Env & env, Value & v) { if (!state.evalBool(env, *cond, pos, "in the condition of the assert statement")) { - std::ostringstream out; - cond->show(state.ctx.symbols, out); - state.ctx.errors.make("assertion '%1%' failed", out.str()).atPos(pos).withFrame(env, *this).debugThrow(); + state.ctx.errors.make("assertion failed") + .atPos(pos) + .withFrame(env, *this) + .debugThrow(); } body->eval(state, env, v); } diff --git a/lix/libexpr/eval.hh b/lix/libexpr/eval.hh index cadc99032..b391f68f8 100644 --- a/lix/libexpr/eval.hh +++ b/lix/libexpr/eval.hh @@ -748,7 +748,7 @@ private: inline Value * lookupVar(Env * env, const ExprVar & var, bool noEval); friend struct ExprVar; - friend struct ExprAttrs; + friend struct ExprSet; friend struct ExprLet; /** diff --git a/lix/libexpr/nixexpr.cc b/lix/libexpr/nixexpr.cc index 1f51fbf45..d7844d6a7 100644 --- a/lix/libexpr/nixexpr.cc +++ b/lix/libexpr/nixexpr.cc @@ -21,11 +21,11 @@ std::ostream & operator <<(std::ostream & str, const SymbolStr & symbol) return printIdentifier(str, s); } -AttrName::AttrName(Symbol s) : symbol(s) +AttrName::AttrName(PosIdx pos, Symbol s) : pos(pos), symbol(s) { } -AttrName::AttrName(std::unique_ptr e) : expr(std::move(e)) +AttrName::AttrName(PosIdx pos, std::unique_ptr e) : pos(pos), expr(std::move(e)) { } @@ -139,7 +139,7 @@ void ExprAttrs::showBindings(const SymbolTable & symbols, std::ostream & str) co } } -void ExprAttrs::show(const SymbolTable & symbols, std::ostream & str) const +void ExprSet::show(const SymbolTable & symbols, std::ostream & str) const { if (recursive) str << "rec "; str << "{ "; @@ -202,7 +202,7 @@ void ExprCall::show(const SymbolTable & symbols, std::ostream & str) const void ExprLet::show(const SymbolTable & symbols, std::ostream & str) const { str << "(let "; - attrs->showBindings(symbols, str); + showBindings(symbols, str); str << "in "; body->show(symbols, str); str << ")"; @@ -423,7 +423,7 @@ std::shared_ptr ExprAttrs::bindInheritSources( return inner; } -void ExprAttrs::bindVars(Evaluator & es, const std::shared_ptr & env) +void ExprSet::bindVars(Evaluator & es, const std::shared_ptr & env) { if (es.debug) es.debug->exprEnvs.insert(std::make_pair(this, env)); @@ -511,18 +511,18 @@ void ExprCall::bindVars(Evaluator & es, const std::shared_ptr & void ExprLet::bindVars(Evaluator & es, const std::shared_ptr & env) { auto newEnv = [&] () -> std::shared_ptr { - auto newEnv = std::make_shared(nullptr, env.get(), attrs->attrs.size()); + auto newEnv = std::make_shared(nullptr, env.get(), attrs.size()); Displacement displ = 0; - for (auto & i : attrs->attrs) + for (auto & i : attrs) newEnv->vars.emplace_back(i.first, i.second.displ = displ++); return newEnv; }(); - // No need to sort newEnv since attrs->attrs is in sorted order. + // No need to sort newEnv since attrs is in sorted order. - auto inheritFromEnv = attrs->bindInheritSources(es, newEnv); - for (auto & i : attrs->attrs) + auto inheritFromEnv = bindInheritSources(es, newEnv); + for (auto & i : attrs) i.second.e->bindVars(es, i.second.chooseByKind(newEnv, env, inheritFromEnv)); if (es.debug) diff --git a/lix/libexpr/nixexpr.hh b/lix/libexpr/nixexpr.hh index bf2a3e8dc..6321a0803 100644 --- a/lix/libexpr/nixexpr.hh +++ b/lix/libexpr/nixexpr.hh @@ -28,10 +28,11 @@ struct StaticEnv; */ struct AttrName { + PosIdx pos; Symbol symbol; std::unique_ptr expr; - AttrName(Symbol s); - AttrName(std::unique_ptr e); + AttrName(PosIdx pos, Symbol s); + AttrName(PosIdx pos, std::unique_ptr e); }; typedef std::vector AttrPath; @@ -182,7 +183,7 @@ struct ExprSelect : Expr AttrPath attrPath; ExprSelect(const PosIdx & pos, std::unique_ptr e, AttrPath attrPath, std::unique_ptr def) : pos(pos), e(std::move(e)), def(std::move(def)), attrPath(std::move(attrPath)) { }; - ExprSelect(const PosIdx & pos, std::unique_ptr e, Symbol name) : pos(pos), e(std::move(e)) { attrPath.push_back(AttrName(name)); }; + ExprSelect(const PosIdx & pos, std::unique_ptr e, const PosIdx namePos, Symbol name) : pos(pos), e(std::move(e)) { attrPath.push_back(AttrName(namePos, name)); }; PosIdx getPos() const override { return pos; } COMMON_METHODS }; @@ -196,10 +197,16 @@ struct ExprOpHasAttr : Expr COMMON_METHODS }; -struct ExprAttrs : Expr +/* Helper struct to contain the data shared across lets and sets */ +struct ExprAttrs { - bool recursive; - PosIdx pos; + ExprAttrs() = default; + ExprAttrs(const ExprAttrs &) = delete; + ExprAttrs & operator=(const ExprAttrs &) = delete; + ExprAttrs(ExprAttrs &&) = default; + ExprAttrs & operator=(ExprAttrs &&) = default; + virtual ~ExprAttrs() = default; + struct AttrDef { enum class Kind { /** `attr = expr;` */ @@ -243,10 +250,6 @@ struct ExprAttrs : Expr }; typedef std::vector DynamicAttrDefs; DynamicAttrDefs dynamicAttrs; - ExprAttrs(const PosIdx &pos) : recursive(false), pos(pos) { }; - ExprAttrs() : recursive(false) { }; - PosIdx getPos() const override { return pos; } - COMMON_METHODS std::shared_ptr bindInheritSources( Evaluator & es, const std::shared_ptr & env); @@ -254,6 +257,16 @@ struct ExprAttrs : Expr void showBindings(const SymbolTable & symbols, std::ostream & str) const; }; +struct ExprSet : Expr, ExprAttrs { + PosIdx pos; + bool recursive = false; + + ExprSet(const PosIdx &pos, bool recursive = false) : pos(pos), recursive(recursive) { }; + ExprSet() { }; + PosIdx getPos() const override { return pos; } + COMMON_METHODS +}; + struct ExprList : Expr { std::vector> elems; @@ -368,11 +381,9 @@ struct ExprCall : Expr COMMON_METHODS }; -struct ExprLet : Expr +struct ExprLet : Expr, ExprAttrs { - std::unique_ptr attrs; std::unique_ptr body; - ExprLet(std::unique_ptr attrs, std::unique_ptr body) : attrs(std::move(attrs)), body(std::move(body)) { }; COMMON_METHODS }; diff --git a/lix/libexpr/parser/parser-impl1.inc.cc b/lix/libexpr/parser/parser-impl1.inc.cc index a1b5afa8a..77b37b806 100644 --- a/lix/libexpr/parser/parser-impl1.inc.cc +++ b/lix/libexpr/parser/parser-impl1.inc.cc @@ -260,10 +260,10 @@ template<> struct BuildAST : change_head { struct AttrState : SubexprState { using SubexprState::SubexprState; - std::vector attrs; + AttrPath attrs; template - void pushAttr(T && attr, PosIdx) { attrs.emplace_back(std::forward(attr)); } + void pushAttr(T && attr, PosIdx pos) { attrs.emplace_back(pos, std::forward(attr)); } }; template<> struct BuildAST { @@ -285,22 +285,38 @@ template<> struct BuildAST { template<> struct BuildAST : BuildAST {}; struct BindingsState : SubexprState { - using SubexprState::SubexprState; + explicit BindingsState(ExprState & up, ExprAttrs & attrs) : SubexprState(up), attrs(attrs) {} - ExprAttrs attrs; + ExprAttrs & attrs; + PosIdx pos; AttrPath path; std::unique_ptr value; }; +struct BindingsStateSet : BindingsState { + ExprSet set = {}; + BindingsStateSet(ExprState & up, State & ps, auto &...) : BindingsState(up, set) { } +}; + +struct BindingsStateRecSet : BindingsState { + ExprSet set = { PosIdx{}, true }; + BindingsStateRecSet(ExprState & up, State & ps, auto &...) : BindingsState(up, set) { } +}; + +struct BindingsStateLet : BindingsState { + ExprLet let = {}; + BindingsStateLet(ExprState & up, State & ps, auto &...) : BindingsState(up, let) { } +}; + struct InheritState : SubexprState { using SubexprState::SubexprState; - std::vector> attrs; + std::vector attrs; std::unique_ptr from; PosIdx fromPos; template - void pushAttr(T && attr, PosIdx pos) { attrs.emplace_back(std::forward(attr), pos); } + void pushAttr(T && attr, PosIdx pos) { attrs.emplace_back(pos, std::forward(attr)); } }; template<> struct BuildAST { @@ -314,15 +330,15 @@ template<> struct BuildAST : change_head { static void success0(InheritState & s, BindingsState & b, State & ps) { auto & attrs = b.attrs.attrs; // TODO this should not reuse generic attrpath rules. - for (auto & [i, iPos] : s.attrs) { + for (auto & i : s.attrs) { if (i.symbol) continue; if (auto str = dynamic_cast(i.expr.get())) - i = AttrName(ps.symbols.create(str->s)); + i = AttrName(i.pos, ps.symbols.create(str->s)); else { throw ParseError({ .msg = HintFmt("dynamic attributes not allowed in inherit"), - .pos = ps.positions[iPos] + .pos = ps.positions[i.pos] }); } } @@ -331,9 +347,9 @@ template<> struct BuildAST : change_head { b.attrs.inheritFromExprs = std::make_unique>>(); auto fromExpr = ref(std::move(s.from)); b.attrs.inheritFromExprs->push_back(fromExpr); - for (auto & [i, iPos] : s.attrs) { + for (auto & i : s.attrs) { if (attrs.find(i.symbol) != attrs.end()) - ps.dupAttr(i.symbol, iPos, attrs[i.symbol].pos); + ps.dupAttr(i.symbol, i.pos, attrs[i.symbol].pos); auto inheritFrom = std::make_unique( s.fromPos, b.attrs.inheritFromExprs->size() - 1, @@ -342,19 +358,19 @@ template<> struct BuildAST : change_head { attrs.emplace( i.symbol, ExprAttrs::AttrDef( - std::make_unique(iPos, std::move(inheritFrom), i.symbol), - iPos, + std::make_unique(i.pos, std::move(inheritFrom), i.pos, i.symbol), + i.pos, ExprAttrs::AttrDef::Kind::InheritedFrom)); } } else { - for (auto & [i, iPos] : s.attrs) { + for (auto & i : s.attrs) { if (attrs.find(i.symbol) != attrs.end()) - ps.dupAttr(i.symbol, iPos, attrs[i.symbol].pos); + ps.dupAttr(i.symbol, i.pos, attrs[i.symbol].pos); attrs.emplace( i.symbol, ExprAttrs::AttrDef( - std::make_unique(iPos, i.symbol), - iPos, + std::make_unique(i.pos, i.symbol), + i.pos, ExprAttrs::AttrDef::Kind::Inherited)); } } @@ -663,8 +679,8 @@ template<> struct BuildAST { } }; -template<> struct BuildAST : change_head { - static void success(const auto & in, BindingsState & b, ExprState & s, State & ps) { +template<> struct BuildAST : change_head { + static void success(const auto & in, BindingsStateRecSet & b, ExprState & s, State & ps) { // Added 2024-09-18. Turn into an error at some point in the future. // See the documentation on deprecated features for more details. if (!ps.featureSettings.isEnabled(Dep::AncientLet)) @@ -675,30 +691,23 @@ template<> struct BuildAST : change_head(b.attrs.pos, b.attrs.pos, std::make_unique(std::move(b.attrs)), ps.s.body); + auto pos = ps.at(in); + b.set.pos = pos; + s.pushExpr(pos, pos, std::make_unique(std::move(b.set)), pos, ps.s.body); } }; -template<> struct BuildAST : change_head { - static void success(const auto & in, BindingsState & b, ExprState & s, State & ps) { - // Before inserting new attrs, check for __override and throw an error - // (the error will initially be a warning to ease migration) - if (!featureSettings.isEnabled(Dep::RecSetOverrides) && b.attrs.attrs.contains(ps.s.overrides)) { - ps.overridesFound(ps.at(in)); - } - - b.attrs.pos = ps.at(in); - b.attrs.recursive = true; - s.pushExpr(b.attrs.pos, std::move(b.attrs)); +template<> struct BuildAST : change_head { + static void success(const auto & in, BindingsStateRecSet & b, ExprState & s, State & ps) { + b.set.pos = ps.at(in); + s.pushExpr(ps.at(in), std::move(b.set)); } }; -template<> struct BuildAST : change_head { - static void success(const auto & in, BindingsState & b, ExprState & s, State & ps) { - b.attrs.pos = ps.at(in); - s.pushExpr(b.attrs.pos, std::move(b.attrs)); +template<> struct BuildAST : change_head { + static void success(const auto & in, BindingsStateSet & b, ExprState & s, State & ps) { + b.set.pos = ps.at(in); + s.pushExpr(ps.at(in), std::move(b.set)); } }; @@ -835,15 +844,15 @@ template<> struct BuildAST { } }; -template<> struct BuildAST : change_head { - static void success(const auto & in, BindingsState & b, ExprState & s, State & ps) { - if (!b.attrs.dynamicAttrs.empty()) +template<> struct BuildAST : change_head { + static void success(const auto & in, BindingsStateLet & b, ExprState & s, State & ps) { + if (!b.let.dynamicAttrs.empty()) throw ParseError({ .msg = HintFmt("dynamic attributes not allowed in let"), .pos = ps.positions[ps.at(in)] }); - - s.pushExpr(ps.at(in), std::make_unique(std::move(b.attrs)), b->popExprOnly()); + b.let.body = b->popExprOnly(); + s.pushExpr(ps.at(in), std::move(b.let)); } }; diff --git a/lix/libexpr/parser/state.hh b/lix/libexpr/parser/state.hh index 89a87e85e..098c38745 100644 --- a/lix/libexpr/parser/state.hh +++ b/lix/libexpr/parser/state.hh @@ -102,7 +102,7 @@ inline void State::addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_ ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(i->symbol); if (j != attrs->attrs.end()) { if (j->second.kind != ExprAttrs::AttrDef::Kind::Inherited) { - ExprAttrs * attrs2 = dynamic_cast(j->second.e.get()); + ExprSet * attrs2 = dynamic_cast(j->second.e.get()); if (!attrs2) { attrPath.erase(i + 1, attrPath.end()); dupAttr(attrPath, pos, j->second.pos); @@ -115,12 +115,12 @@ inline void State::addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_ } else { auto next = attrs->attrs.emplace(std::piecewise_construct, std::tuple(i->symbol), - std::tuple(std::make_unique(), pos)); - attrs = static_cast(next.first->second.e.get()); + std::tuple(std::make_unique(), pos)); + attrs = static_cast(next.first->second.e.get()); } } else { - auto & next = attrs->dynamicAttrs.emplace_back(std::move(i->expr), std::make_unique(), pos); - attrs = static_cast(next.valueExpr.get()); + auto & next = attrs->dynamicAttrs.emplace_back(std::move(i->expr), std::make_unique(), pos); + attrs = static_cast(next.valueExpr.get()); } } // Expr insertion. @@ -132,8 +132,8 @@ inline void State::addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_ // e and the expr pointed by the attr path are two attribute sets, // we want to merge them. // Otherwise, throw an error. - auto * ae = dynamic_cast(e.get()); - auto * jAttrs = dynamic_cast(j->second.e.get()); + auto * ae = dynamic_cast(e.get()); + auto * jAttrs = dynamic_cast(j->second.e.get()); if (jAttrs && ae) { if (ae->inheritFromExprs && !jAttrs->inheritFromExprs) jAttrs->inheritFromExprs = std::make_unique>>(); @@ -157,8 +157,9 @@ inline void State::addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_ } else { // Before inserting new attrs, check for __override and throw an error // (the error will initially be a warning to ease migration) - if (attrs->recursive && !featureSettings.isEnabled(Dep::RecSetOverrides) && i->symbol == s.overrides) { - overridesFound(pos); + if (!featureSettings.isEnabled(Dep::RecSetOverrides) && i->symbol == s.overrides) { + if (auto set = dynamic_cast(attrs); set && set->recursive) + overridesFound(pos); } // This attr path is not defined. Let's create it. diff --git a/lix/libexpr/print.cc b/lix/libexpr/print.cc index 6083f4a40..05bf74863 100644 --- a/lix/libexpr/print.cc +++ b/lix/libexpr/print.cc @@ -581,10 +581,4 @@ 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.ctx.symbols, output); - return output; -} - } diff --git a/lix/libexpr/print.hh b/lix/libexpr/print.hh index b2e9d8ecd..5f389dcd8 100644 --- a/lix/libexpr/print.hh +++ b/lix/libexpr/print.hh @@ -81,26 +81,4 @@ 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); -}; - } diff --git a/lix/libexpr/value.cc b/lix/libexpr/value.cc index 22f1f33d1..ae62db3e8 100644 --- a/lix/libexpr/value.cc +++ b/lix/libexpr/value.cc @@ -55,8 +55,8 @@ bool Value::isTrivial() const internalType != tApp && internalType != tPrimOpApp && (internalType != tThunk - || (dynamic_cast(thunk.expr) - && static_cast(thunk.expr)->dynamicAttrs.empty()) + || (dynamic_cast(thunk.expr) + && static_cast(thunk.expr)->dynamicAttrs.empty()) || dynamic_cast(thunk.expr) || dynamic_cast(thunk.expr)); } diff --git a/tests/functional/lang/eval-fail-assert.err.exp b/tests/functional/lang/eval-fail-assert.err.exp index 4bb63c29a..e7e573f2b 100644 --- a/tests/functional/lang/eval-fail-assert.err.exp +++ b/tests/functional/lang/eval-fail-assert.err.exp @@ -13,7 +13,7 @@ error: | ^ 3| in - error: assertion '(arg == "y")' failed + error: assertion failed at /pwd/lang/eval-fail-assert.nix:2:12: 1| let 2| x = arg: assert arg == "y"; 123; diff --git a/tests/functional/lang/eval-fail-attr-name-type.err.exp b/tests/functional/lang/eval-fail-attr-name-type.err.exp index 6848a35ed..b68d77794 100644 --- a/tests/functional/lang/eval-fail-attr-name-type.err.exp +++ b/tests/functional/lang/eval-fail-attr-name-type.err.exp @@ -1,5 +1,5 @@ error: - … while evaluating the attribute 'puppy."${key}"' + … while evaluating the attribute 'puppy."${...}"' at /pwd/lang/eval-fail-attr-name-type.nix:3:5: 2| attrs = { 3| puppy.doggy = {}; diff --git a/tests/functional/lang/eval-fail-recursion.err.exp b/tests/functional/lang/eval-fail-recursion.err.exp index f0057b2d5..3231a2df9 100644 --- a/tests/functional/lang/eval-fail-recursion.err.exp +++ b/tests/functional/lang/eval-fail-recursion.err.exp @@ -1,5 +1,5 @@ error: - … while evaluating 'a' to select 'foo' on it + … while evaluating an expression to select 'foo' on it at /pwd/lang/eval-fail-recursion.nix:1:21: 1| let a = {} // a; in a.foo | ^ diff --git a/tests/functional/lang/eval-fail-remove.err.exp b/tests/functional/lang/eval-fail-remove.err.exp index 0ae9c1256..ab99c2b92 100644 --- a/tests/functional/lang/eval-fail-remove.err.exp +++ b/tests/functional/lang/eval-fail-remove.err.exp @@ -1,7 +1,7 @@ error: attribute 'x' missing - at /pwd/lang/eval-fail-remove.nix:4:3: + at /pwd/lang/eval-fail-remove.nix:4:29: 3| in 4| (removeAttrs attrs ["x"]).x - | ^ + | ^ 5| Did you mean y? diff --git a/tests/functional/lang/eval-fail-select-err.err.exp b/tests/functional/lang/eval-fail-select-err.err.exp index 70ee50737..50bcbe51d 100644 --- a/tests/functional/lang/eval-fail-select-err.err.exp +++ b/tests/functional/lang/eval-fail-select-err.err.exp @@ -6,11 +6,11 @@ error: | ^ 3| in somepkg.src.meta - … while evaluating 'somepkg.src' to select 'meta' on it - at /pwd/lang/eval-fail-select-err.nix:3:4: + … while evaluating an expression to select 'meta' on it + at /pwd/lang/eval-fail-select-err.nix:3:16: 2| somepkg.src = throw "invalid foo bar"; 3| in somepkg.src.meta - | ^ + | ^ 4| … caused by explicit throw diff --git a/tests/functional/lang/parse-okay-rec-set-override-warning.err.exp b/tests/functional/lang/parse-okay-rec-set-override-warning.err.exp index 5ed2d7dee..88d6e798b 100644 --- a/tests/functional/lang/parse-okay-rec-set-override-warning.err.exp +++ b/tests/functional/lang/parse-okay-rec-set-override-warning.err.exp @@ -1,2 +1,2 @@ warning: __overrides found at «stdin»:3:16. This feature is deprecated and will be removed in the future. Use --extra-deprecated-features rec-set-overrides to silence this warning. -warning: __overrides found at «stdin»:4:2. This feature is deprecated and will be removed in the future. Use --extra-deprecated-features rec-set-overrides to silence this warning. +warning: __overrides found at «stdin»:4:8. This feature is deprecated and will be removed in the future. Use --extra-deprecated-features rec-set-overrides to silence this warning. diff --git a/tests/unit/libexpr/value/print.cc b/tests/unit/libexpr/value/print.cc index a65c7f1c7..44cfb496f 100644 --- a/tests/unit/libexpr/value/print.cc +++ b/tests/unit/libexpr/value/print.cc @@ -514,7 +514,7 @@ TEST_F(ValuePrintingTests, ansiColorsAssert) ASSERT_EQ(v.type(), nAttrs); test(*v.attrs->begin()->value, - ANSI_RED "«error: assertion 'false' failed»" ANSI_NORMAL, + ANSI_RED "«error: assertion failed»" ANSI_NORMAL, PrintOptions { .ansiColors = true, .force = true