diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc index 3e1963350..5dd4b6beb 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)); 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..1f089a28a 100644 --- a/lix/libexpr/nixexpr.cc +++ b/lix/libexpr/nixexpr.cc @@ -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..a29a5d9fd 100644 --- a/lix/libexpr/nixexpr.hh +++ b/lix/libexpr/nixexpr.hh @@ -196,10 +196,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 +249,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 +256,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 +380,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..d35024937 100644 --- a/lix/libexpr/parser/parser-impl1.inc.cc +++ b/lix/libexpr/parser/parser-impl1.inc.cc @@ -285,13 +285,29 @@ 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; @@ -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)), 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/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/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.