From bcb774688feea4e1fc7919d4890361f4f8ea1f60 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 16 Jun 2024 23:10:09 +0200 Subject: [PATCH] libexpr: add expr memory management with the prepatory work done this mostly means turning plain pointers into unique_ptrs, with all the associated churn that necessitates. we might want to change some of these to box_ptrs at some point as well, but that would be a semantic change that isn't fully appropriate yet. Change-Id: I0c238c118617420650432f4ed45569baa3e3f413 --- src/libexpr/eval.cc | 2 +- src/libexpr/nixexpr.cc | 14 ++-- src/libexpr/nixexpr.hh | 88 ++++++++++---------- src/libexpr/parser-state.hh | 79 +++++++++--------- src/libexpr/parser.y | 128 +++++++++++++++++------------- src/libexpr/primops.cc | 2 +- src/libexpr/value-to-xml.cc | 2 +- src/nix/flake.cc | 2 +- tests/unit/libexpr/value/print.cc | 17 ++-- 9 files changed, 175 insertions(+), 159 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index c8329a602..25d98b23b 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1247,7 +1247,7 @@ Env * ExprAttrs::buildInheritFromEnv(EvalState & state, Env & up) inheritEnv.up = &up; Displacement displ = 0; - for (auto from : *inheritFromExprs) + for (auto & from : *inheritFromExprs) inheritEnv.values[displ++] = from->maybeThunk(state, up); return &inheritEnv; diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 03e1effc0..bc53ca053 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -79,7 +79,7 @@ void ExprAttrs::showBindings(const SymbolTable & symbols, std::ostream & str) co return sa < sb; }); std::vector inherits; - std::map> inheritsFrom; + std::map> inheritsFrom; for (auto & i : sorted) { switch (i->second.kind) { case AttrDef::Kind::Plain: @@ -90,7 +90,7 @@ void ExprAttrs::showBindings(const SymbolTable & symbols, std::ostream & str) co case AttrDef::Kind::InheritedFrom: { auto & select = dynamic_cast(*i->second.e); auto & from = dynamic_cast(*select.e); - inheritsFrom[&from].push_back(i->first); + inheritsFrom[from.displ].push_back(i->first); break; } } @@ -102,7 +102,7 @@ void ExprAttrs::showBindings(const SymbolTable & symbols, std::ostream & str) co } for (const auto & [from, syms] : inheritsFrom) { str << "inherit ("; - (*inheritFromExprs)[from->displ]->show(symbols, str); + (*inheritFromExprs)[from]->show(symbols, str); str << ")"; for (auto sym : syms) str << " " << symbols[sym]; str << "; "; @@ -151,7 +151,7 @@ void ExprLambda::show(const SymbolTable & symbols, std::ostream & str) const // the natural Symbol ordering is by creation time, which can lead to the // same expression being printed in two different ways depending on its // context. always use lexicographic ordering to avoid this. - for (auto & i : formals->lexicographicOrder(symbols)) { + for (const Formal & i : formals->lexicographicOrder(symbols)) { if (first) first = false; else str << ", "; str << symbols[i.name]; if (i.def) { @@ -176,7 +176,7 @@ void ExprCall::show(const SymbolTable & symbols, std::ostream & str) const { str << '('; fun->show(symbols, str); - for (auto e : args) { + for (auto & e : args) { str << ' '; e->show(symbols, str); } @@ -375,7 +375,7 @@ std::shared_ptr ExprAttrs::bindInheritSources( // not even *have* an expr that grabs anything from this env since it's fully // invisible, but the evaluator does not allow for this yet. auto inner = std::make_shared(nullptr, env.get(), 0); - for (auto from : *inheritFromExprs) + for (auto & from : *inheritFromExprs) from->bindVars(es, env); return inner; @@ -462,7 +462,7 @@ void ExprCall::bindVars(EvalState & es, const std::shared_ptr & es.exprEnvs.insert(std::make_pair(this, env)); fun->bindVars(es, env); - for (auto e : args) + for (auto & e : args) e->bindVars(es, env); } diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index 91516c092..45d44d1d1 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -28,9 +28,9 @@ struct StaticEnv; struct AttrName { Symbol symbol; - Expr * expr; + std::unique_ptr expr; AttrName(Symbol s) : symbol(s) {}; - AttrName(Expr * e) : expr(e) {}; + AttrName(std::unique_ptr e) : expr(std::move(e)) {}; }; typedef std::vector AttrPath; @@ -157,19 +157,19 @@ struct ExprInheritFrom : ExprVar struct ExprSelect : Expr { PosIdx pos; - Expr * e, * def; + std::unique_ptr e, def; AttrPath attrPath; - ExprSelect(const PosIdx & pos, Expr * e, AttrPath attrPath, Expr * def) : pos(pos), e(e), def(def), attrPath(std::move(attrPath)) { }; - ExprSelect(const PosIdx & pos, Expr * e, Symbol name) : pos(pos), e(e), def(0) { attrPath.push_back(AttrName(name)); }; + 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)); }; PosIdx getPos() const override { return pos; } COMMON_METHODS }; struct ExprOpHasAttr : Expr { - Expr * e; + std::unique_ptr e; AttrPath attrPath; - ExprOpHasAttr(Expr * e, AttrPath attrPath) : e(e), attrPath(std::move(attrPath)) { }; + ExprOpHasAttr(std::unique_ptr e, AttrPath attrPath) : e(std::move(e)), attrPath(std::move(attrPath)) { }; PosIdx getPos() const override { return e->getPos(); } COMMON_METHODS }; @@ -189,11 +189,11 @@ struct ExprAttrs : Expr }; Kind kind; - Expr * e; + std::unique_ptr e; PosIdx pos; Displacement displ; // displacement - AttrDef(Expr * e, const PosIdx & pos, Kind kind = Kind::Plain) - : kind(kind), e(e), pos(pos) { }; + AttrDef(std::unique_ptr e, const PosIdx & pos, Kind kind = Kind::Plain) + : kind(kind), e(std::move(e)), pos(pos) { }; AttrDef() { }; template @@ -212,12 +212,12 @@ struct ExprAttrs : Expr }; typedef std::map AttrDefs; AttrDefs attrs; - std::unique_ptr> inheritFromExprs; + std::unique_ptr>> inheritFromExprs; struct DynamicAttrDef { - Expr * nameExpr, * valueExpr; + std::unique_ptr nameExpr, valueExpr; PosIdx pos; - DynamicAttrDef(Expr * nameExpr, Expr * valueExpr, const PosIdx & pos) - : nameExpr(nameExpr), valueExpr(valueExpr), pos(pos) { }; + DynamicAttrDef(std::unique_ptr nameExpr, std::unique_ptr valueExpr, const PosIdx & pos) + : nameExpr(std::move(nameExpr)), valueExpr(std::move(valueExpr)), pos(pos) { }; }; typedef std::vector DynamicAttrDefs; DynamicAttrDefs dynamicAttrs; @@ -234,7 +234,7 @@ struct ExprAttrs : Expr struct ExprList : Expr { - std::vector elems; + std::vector> elems; ExprList() { }; COMMON_METHODS Value * maybeThunk(EvalState & state, Env & env) override; @@ -249,7 +249,7 @@ struct Formal { PosIdx pos; Symbol name; - Expr * def; + std::unique_ptr def; }; /** Attribute set destructuring in arguments of a lambda, if present */ @@ -266,9 +266,9 @@ struct Formals return it != formals.end() && it->name == arg; } - std::vector lexicographicOrder(const SymbolTable & symbols) const + std::vector> lexicographicOrder(const SymbolTable & symbols) const { - std::vector result(formals.begin(), formals.end()); + std::vector> result(formals.begin(), formals.end()); std::sort(result.begin(), result.end(), [&] (const Formal & a, const Formal & b) { std::string_view sa = symbols[a.name], sb = symbols[b.name]; @@ -292,14 +292,14 @@ struct ExprLambda : Expr Symbol arg; /** Formals are present when the lambda destructures an attr set as * argument, with or without ellipsis */ - Formals * formals; - Expr * body; - ExprLambda(PosIdx pos, Symbol arg, Formals * formals, Expr * body) - : pos(pos), arg(arg), formals(formals), body(body) + std::unique_ptr formals; + std::unique_ptr body; + ExprLambda(PosIdx pos, Symbol arg, std::unique_ptr formals, std::unique_ptr body) + : pos(pos), arg(arg), formals(std::move(formals)), body(std::move(body)) { }; - ExprLambda(PosIdx pos, Formals * formals, Expr * body) - : pos(pos), formals(formals), body(body) + ExprLambda(PosIdx pos, std::unique_ptr formals, std::unique_ptr body) + : pos(pos), formals(std::move(formals)), body(std::move(body)) { } void setName(Symbol name) override; @@ -336,11 +336,11 @@ struct ExprLambda : Expr struct ExprCall : Expr { - Expr * fun; - std::vector args; + std::unique_ptr fun; + std::vector> args; PosIdx pos; - ExprCall(const PosIdx & pos, Expr * fun, std::vector && args) - : fun(fun), args(std::move(args)), pos(pos) + ExprCall(const PosIdx & pos, std::unique_ptr fun, std::vector> && args) + : fun(std::move(fun)), args(std::move(args)), pos(pos) { } PosIdx getPos() const override { return pos; } COMMON_METHODS @@ -348,19 +348,19 @@ struct ExprCall : Expr struct ExprLet : Expr { - ExprAttrs * attrs; - Expr * body; - ExprLet(ExprAttrs * attrs, Expr * body) : attrs(attrs), body(body) { }; + 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 }; struct ExprWith : Expr { PosIdx pos; - Expr * attrs, * body; + std::unique_ptr attrs, body; size_t prevWith; ExprWith * parentWith; - ExprWith(const PosIdx & pos, Expr * attrs, Expr * body) : pos(pos), attrs(attrs), body(body) { }; + ExprWith(const PosIdx & pos, std::unique_ptr attrs, std::unique_ptr body) : pos(pos), attrs(std::move(attrs)), body(std::move(body)) { }; PosIdx getPos() const override { return pos; } COMMON_METHODS }; @@ -368,8 +368,8 @@ struct ExprWith : Expr struct ExprIf : Expr { PosIdx pos; - Expr * cond, * then, * else_; - ExprIf(const PosIdx & pos, Expr * cond, Expr * then, Expr * else_) : pos(pos), cond(cond), then(then), else_(else_) { }; + std::unique_ptr cond, then, else_; + ExprIf(const PosIdx & pos, std::unique_ptr cond, std::unique_ptr then, std::unique_ptr else_) : pos(pos), cond(std::move(cond)), then(std::move(then)), else_(std::move(else_)) { }; PosIdx getPos() const override { return pos; } COMMON_METHODS }; @@ -377,16 +377,16 @@ struct ExprIf : Expr struct ExprAssert : Expr { PosIdx pos; - Expr * cond, * body; - ExprAssert(const PosIdx & pos, Expr * cond, Expr * body) : pos(pos), cond(cond), body(body) { }; + std::unique_ptr cond, body; + ExprAssert(const PosIdx & pos, std::unique_ptr cond, std::unique_ptr body) : pos(pos), cond(std::move(cond)), body(std::move(body)) { }; PosIdx getPos() const override { return pos; } COMMON_METHODS }; struct ExprOpNot : Expr { - Expr * e; - ExprOpNot(Expr * e) : e(e) { }; + std::unique_ptr e; + ExprOpNot(std::unique_ptr e) : e(std::move(e)) { }; PosIdx getPos() const override { return e->getPos(); } COMMON_METHODS }; @@ -395,9 +395,9 @@ struct ExprOpNot : Expr struct name : Expr \ { \ PosIdx pos; \ - Expr * e1, * e2; \ - name(Expr * e1, Expr * e2) : e1(e1), e2(e2) { }; \ - name(const PosIdx & pos, Expr * e1, Expr * e2) : pos(pos), e1(e1), e2(e2) { }; \ + std::unique_ptr e1, e2; \ + name(std::unique_ptr e1, std::unique_ptr e2) : e1(std::move(e1)), e2(std::move(e2)) { }; \ + name(const PosIdx & pos, std::unique_ptr e1, std::unique_ptr e2) : pos(pos), e1(std::move(e1)), e2(std::move(e2)) { }; \ void show(const SymbolTable & symbols, std::ostream & str) const override \ { \ str << "("; e1->show(symbols, str); str << " " s " "; e2->show(symbols, str); str << ")"; \ @@ -422,8 +422,8 @@ struct ExprConcatStrings : Expr { PosIdx pos; bool forceString; - std::vector> es; - ExprConcatStrings(const PosIdx & pos, bool forceString, std::vector> es) + std::vector>> es; + ExprConcatStrings(const PosIdx & pos, bool forceString, std::vector>> es) : pos(pos), forceString(forceString), es(std::move(es)) { }; PosIdx getPos() const override { return pos; } COMMON_METHODS diff --git a/src/libexpr/parser-state.hh b/src/libexpr/parser-state.hh index 468b34bd3..cb1f12230 100644 --- a/src/libexpr/parser-state.hh +++ b/src/libexpr/parser-state.hh @@ -49,10 +49,10 @@ struct ParserState [[nodiscard]] ParseError dupAttr(const AttrPath & attrPath, const PosIdx pos, const PosIdx prevPos); [[nodiscard]] ParseError dupAttr(Symbol attr, const PosIdx pos, const PosIdx prevPos); - [[nodiscard]] std::optional addAttr(ExprAttrs * attrs, AttrPath && attrPath, Expr * e, const PosIdx pos); + [[nodiscard]] std::optional addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_ptr e, const PosIdx pos); [[nodiscard]] std::optional validateFormals(Formals * formals, PosIdx pos = noPos, Symbol arg = {}); - Expr * stripIndentation(const PosIdx pos, - std::vector>> && es); + std::unique_ptr stripIndentation(const PosIdx pos, + std::vector, StringToken>>> && es); PosIdx at(const ParserLocation & loc); }; @@ -73,7 +73,7 @@ inline ParseError ParserState::dupAttr(Symbol attr, const PosIdx pos, const PosI }); } -inline std::optional ParserState::addAttr(ExprAttrs * attrs, AttrPath && attrPath, Expr * e, const PosIdx pos) +inline std::optional ParserState::addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_ptr e, const PosIdx pos) { AttrPath::iterator i; // All attrpaths have at least one attr @@ -85,20 +85,25 @@ inline std::optional ParserState::addAttr(ExprAttrs * attrs, AttrPat 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); - if (!attrs2) return dupAttr({attrPath.begin(), i + 1}, pos, j->second.pos); + ExprAttrs * attrs2 = dynamic_cast(j->second.e.get()); + if (!attrs2) { + attrPath.erase(i + 1, attrPath.end()); + return dupAttr(attrPath, pos, j->second.pos); + } attrs = attrs2; - } else - return dupAttr({attrPath.begin(), i + 1}, pos, j->second.pos); + } else { + attrPath.erase(i + 1, attrPath.end()); + return dupAttr(attrPath, pos, j->second.pos); + } } else { - ExprAttrs * nested = new ExprAttrs; - attrs->attrs[i->symbol] = ExprAttrs::AttrDef(nested, pos); - attrs = nested; + 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()); } } else { - ExprAttrs *nested = new ExprAttrs; - attrs->dynamicAttrs.push_back(ExprAttrs::DynamicAttrDef(i->expr, nested, pos)); - attrs = nested; + auto & next = attrs->dynamicAttrs.emplace_back(std::move(i->expr), std::make_unique(), pos); + attrs = static_cast(next.valueExpr.get()); } } // Expr insertion. @@ -110,37 +115,37 @@ inline std::optional ParserState::addAttr(ExprAttrs * attrs, AttrPat // 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); - auto jAttrs = dynamic_cast(j->second.e); + 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>(); + 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. return dupAttr(ad.first, j2->second.pos, ad.second.pos); - jAttrs->attrs.emplace(ad.first, ad.second); if (ad.second.kind == ExprAttrs::AttrDef::Kind::InheritedFrom) { auto & sel = dynamic_cast(*ad.second.e); auto & from = dynamic_cast(*sel.e); from.displ += jAttrs->inheritFromExprs->size(); } + jAttrs->attrs.emplace(ad.first, std::move(ad.second)); } - jAttrs->dynamicAttrs.insert(jAttrs->dynamicAttrs.end(), ae->dynamicAttrs.begin(), ae->dynamicAttrs.end()); - if (ae->inheritFromExprs) { - jAttrs->inheritFromExprs->insert(jAttrs->inheritFromExprs->end(), - ae->inheritFromExprs->begin(), ae->inheritFromExprs->end()); - } + std::ranges::move(ae->dynamicAttrs, std::back_inserter(jAttrs->dynamicAttrs)); + if (ae->inheritFromExprs) + std::ranges::move(*ae->inheritFromExprs, std::back_inserter(*jAttrs->inheritFromExprs)); } else { return dupAttr(attrPath, pos, j->second.pos); } } else { // This attr path is not defined. Let's create it. - attrs->attrs.emplace(i->symbol, ExprAttrs::AttrDef(e, pos)); e->setName(i->symbol); + attrs->attrs.emplace(std::piecewise_construct, + std::tuple(i->symbol), + std::tuple(std::move(e), pos)); } } else { - attrs->dynamicAttrs.push_back(ExprAttrs::DynamicAttrDef(i->expr, e, pos)); + attrs->dynamicAttrs.emplace_back(std::move(i->expr), std::move(e), pos); } return {}; @@ -175,10 +180,10 @@ inline std::optional ParserState::validateFormals(Formals * formals, return {}; } -inline Expr * ParserState::stripIndentation(const PosIdx pos, - std::vector>> && es) +inline std::unique_ptr ParserState::stripIndentation(const PosIdx pos, + std::vector, StringToken>>> && es) { - if (es.empty()) return new ExprString(""); + if (es.empty()) return std::make_unique(""); /* Figure out the minimum indentation. Note that by design whitespace-only final lines are not taken into account. (So @@ -216,17 +221,17 @@ inline Expr * ParserState::stripIndentation(const PosIdx pos, } /* Strip spaces from each line. */ - std::vector> es2; + std::vector>> es2; atStartOfLine = true; size_t curDropped = 0; size_t n = es.size(); auto i = es.begin(); - const auto trimExpr = [&] (Expr * e) { + const auto trimExpr = [&] (std::unique_ptr e) { atStartOfLine = false; curDropped = 0; - es2.emplace_back(i->first, e); + es2.emplace_back(i->first, std::move(e)); }; - const auto trimString = [&] (const StringToken & t) { + const auto trimString = [&] (const StringToken t) { std::string s2; for (size_t j = 0; j < t.l; ++j) { if (atStartOfLine) { @@ -256,17 +261,17 @@ inline Expr * ParserState::stripIndentation(const PosIdx pos, s2 = std::string(s2, 0, p + 1); } - es2.emplace_back(i->first, new ExprString(std::move(s2))); + es2.emplace_back(i->first, std::make_unique(std::move(s2))); }; for (; i != es.end(); ++i, --n) { - std::visit(overloaded { trimExpr, trimString }, i->second); + std::visit(overloaded { trimExpr, trimString }, std::move(i->second)); } /* If this is a single string, then don't do a concatenation. */ - if (es2.size() == 1 && dynamic_cast(es2[0].second)) { - return es2[0].second; + if (es2.size() == 1 && dynamic_cast(es2[0].second.get())) { + return std::move(es2[0].second); } - return new ExprConcatStrings(pos, true, std::move(es2)); + return std::make_unique(pos, true, std::move(es2)); } inline PosIdx ParserState::at(const ParserLocation & loc) diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 5b53720ce..b825f2ed8 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -81,6 +81,21 @@ void yyerror(YYLTYPE * loc, yyscan_t scanner, ParserState * state, const char * }); } +template +static std::unique_ptr unp(T * e) +{ + return std::unique_ptr(e); +} + +template, typename... Args> +static std::vector vec(Args && ... args) +{ + std::vector result; + result.reserve(sizeof...(Args)); + (result.emplace_back(std::forward(args)), ...); + return result; +} + %} @@ -99,8 +114,8 @@ void yyerror(YYLTYPE * loc, yyscan_t scanner, ParserState * state, const char * nix::StringToken str; std::vector * attrNames; std::vector> * inheritAttrs; - std::vector> * string_parts; - std::vector>> * ind_string_parts; + std::vector>> * string_parts; + std::vector, nix::StringToken>>> * ind_string_parts; } %destructor { delete $$; } @@ -158,85 +173,86 @@ expr: expr_function; expr_function : ID ':' expr_function - { $$ = new ExprLambda(CUR_POS, state->symbols.create($1), 0, $3); } + { $$ = new ExprLambda(CUR_POS, state->symbols.create($1), nullptr, unp($3)); } | '{' formals '}' ':' expr_function { if (auto e = state->validateFormals($2)) THROW(*e); - $$ = new ExprLambda(CUR_POS, $2, $5); } + $$ = new ExprLambda(CUR_POS, unp($2), unp($5)); + } | '{' formals '}' '@' ID ':' expr_function { auto arg = state->symbols.create($5); if (auto e = state->validateFormals($2, CUR_POS, arg)) THROW(*e, $2, $7); - $$ = new ExprLambda(CUR_POS, arg, $2, $7); + $$ = new ExprLambda(CUR_POS, arg, unp($2), unp($7)); } | ID '@' '{' formals '}' ':' expr_function { auto arg = state->symbols.create($1); if (auto e = state->validateFormals($4, CUR_POS, arg)) THROW(*e, $4, $7); - $$ = new ExprLambda(CUR_POS, arg, $4, $7); + $$ = new ExprLambda(CUR_POS, arg, unp($4), unp($7)); } | ASSERT expr ';' expr_function - { $$ = new ExprAssert(CUR_POS, $2, $4); } + { $$ = new ExprAssert(CUR_POS, unp($2), unp($4)); } | WITH expr ';' expr_function - { $$ = new ExprWith(CUR_POS, $2, $4); } + { $$ = new ExprWith(CUR_POS, unp($2), unp($4)); } | LET binds IN expr_function { if (!$2->dynamicAttrs.empty()) THROW(ParseError({ .msg = HintFmt("dynamic attributes not allowed in let"), .pos = state->positions[CUR_POS] }), $2, $4); - $$ = new ExprLet($2, $4); + $$ = new ExprLet(unp($2), unp($4)); } | expr_if ; expr_if - : IF expr THEN expr ELSE expr { $$ = new ExprIf(CUR_POS, $2, $4, $6); } + : IF expr THEN expr ELSE expr { $$ = new ExprIf(CUR_POS, unp($2), unp($4), unp($6)); } | expr_op ; expr_op - : '!' expr_op %prec NOT { $$ = new ExprOpNot($2); } - | '-' expr_op %prec NEGATE { $$ = new ExprCall(CUR_POS, new ExprVar(state->s.sub), {new ExprInt(0), $2}); } - | expr_op EQ expr_op { $$ = new ExprOpEq($1, $3); } - | expr_op NEQ expr_op { $$ = new ExprOpNEq($1, $3); } - | expr_op '<' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->s.lessThan), {$1, $3}); } - | expr_op LEQ expr_op { $$ = new ExprOpNot(new ExprCall(state->at(@2), new ExprVar(state->s.lessThan), {$3, $1})); } - | expr_op '>' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->s.lessThan), {$3, $1}); } - | expr_op GEQ expr_op { $$ = new ExprOpNot(new ExprCall(state->at(@2), new ExprVar(state->s.lessThan), {$1, $3})); } - | expr_op AND expr_op { $$ = new ExprOpAnd(state->at(@2), $1, $3); } - | expr_op OR expr_op { $$ = new ExprOpOr(state->at(@2), $1, $3); } - | expr_op IMPL expr_op { $$ = new ExprOpImpl(state->at(@2), $1, $3); } - | expr_op UPDATE expr_op { $$ = new ExprOpUpdate(state->at(@2), $1, $3); } - | expr_op '?' attrpath { $$ = new ExprOpHasAttr($1, std::move(*$3)); delete $3; } + : '!' expr_op %prec NOT { $$ = new ExprOpNot(unp($2)); } + | '-' expr_op %prec NEGATE { $$ = new ExprCall(CUR_POS, std::make_unique(state->s.sub), vec(std::make_unique(0), unp($2))); } + | expr_op EQ expr_op { $$ = new ExprOpEq(unp($1), unp($3)); } + | expr_op NEQ expr_op { $$ = new ExprOpNEq(unp($1), unp($3)); } + | expr_op '<' expr_op { $$ = new ExprCall(state->at(@2), std::make_unique(state->s.lessThan), vec($1, $3)); } + | expr_op LEQ expr_op { $$ = new ExprOpNot(std::make_unique(state->at(@2), std::make_unique(state->s.lessThan), vec($3, $1))); } + | expr_op '>' expr_op { $$ = new ExprCall(state->at(@2), std::make_unique(state->s.lessThan), vec($3, $1)); } + | expr_op GEQ expr_op { $$ = new ExprOpNot(std::make_unique(state->at(@2), std::make_unique(state->s.lessThan), vec($1, $3))); } + | expr_op AND expr_op { $$ = new ExprOpAnd(state->at(@2), unp($1), unp($3)); } + | expr_op OR expr_op { $$ = new ExprOpOr(state->at(@2), unp($1), unp($3)); } + | expr_op IMPL expr_op { $$ = new ExprOpImpl(state->at(@2), unp($1), unp($3)); } + | expr_op UPDATE expr_op { $$ = new ExprOpUpdate(state->at(@2), unp($1), unp($3)); } + | expr_op '?' attrpath { $$ = new ExprOpHasAttr(unp($1), std::move(*$3)); delete $3; } | expr_op '+' expr_op - { $$ = new ExprConcatStrings(state->at(@2), false, {{state->at(@1), $1}, {state->at(@3), $3}}); } - | expr_op '-' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->s.sub), {$1, $3}); } - | expr_op '*' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->s.mul), {$1, $3}); } - | expr_op '/' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->s.div), {$1, $3}); } - | expr_op CONCAT expr_op { $$ = new ExprOpConcatLists(state->at(@2), $1, $3); } + { $$ = new ExprConcatStrings(state->at(@2), false, vec>>(std::pair(state->at(@1), unp($1)), std::pair(state->at(@3), unp($3)))); } + | expr_op '-' expr_op { $$ = new ExprCall(state->at(@2), std::make_unique(state->s.sub), vec($1, $3)); } + | expr_op '*' expr_op { $$ = new ExprCall(state->at(@2), std::make_unique(state->s.mul), vec($1, $3)); } + | expr_op '/' expr_op { $$ = new ExprCall(state->at(@2), std::make_unique(state->s.div), vec($1, $3)); } + | expr_op CONCAT expr_op { $$ = new ExprOpConcatLists(state->at(@2), unp($1), unp($3)); } | expr_app ; expr_app : expr_app expr_select { if (auto e2 = dynamic_cast($1)) { - e2->args.push_back($2); + e2->args.emplace_back($2); $$ = $1; } else - $$ = new ExprCall(CUR_POS, $1, {$2}); + $$ = new ExprCall(CUR_POS, unp($1), vec(unp($2))); } | expr_select ; expr_select : expr_simple '.' attrpath - { $$ = new ExprSelect(CUR_POS, $1, std::move(*$3), nullptr); delete $3; } + { $$ = new ExprSelect(CUR_POS, unp($1), std::move(*$3), nullptr); delete $3; } | expr_simple '.' attrpath OR_KW expr_select - { $$ = new ExprSelect(CUR_POS, $1, std::move(*$3), $5); delete $3; } + { $$ = new ExprSelect(CUR_POS, unp($1), std::move(*$3), unp($5)); delete $3; } | /* Backwards compatibility: because Nixpkgs has a rarely used function named ‘or’, allow stuff like ‘map or [...]’. */ expr_simple OR_KW - { $$ = new ExprCall(CUR_POS, $1, {new ExprVar(CUR_POS, state->s.or_)}); } + { $$ = new ExprCall(CUR_POS, unp($1), vec(std::make_unique(CUR_POS, state->s.or_))); } | expr_simple ; @@ -252,21 +268,21 @@ expr_simple | FLOAT { $$ = new ExprFloat($1); } | '"' string_parts '"' { $$ = $2; } | IND_STRING_OPEN ind_string_parts IND_STRING_CLOSE { - $$ = state->stripIndentation(CUR_POS, std::move(*$2)); + $$ = state->stripIndentation(CUR_POS, std::move(*$2)).release(); delete $2; } | path_start PATH_END | path_start string_parts_interpolated PATH_END { - $2->insert($2->begin(), {state->at(@1), $1}); + $2->emplace($2->begin(), state->at(@1), $1); $$ = new ExprConcatStrings(CUR_POS, false, std::move(*$2)); delete $2; } | SPATH { std::string path($1.p + 1, $1.l - 2); $$ = new ExprCall(CUR_POS, - new ExprVar(state->s.findFile), - {new ExprVar(state->s.nixPath), - new ExprString(std::move(path))}); + std::make_unique(state->s.findFile), + vec(std::make_unique(state->s.nixPath), + std::make_unique(std::move(path)))); } | URI { static bool noURLLiterals = experimentalFeatureSettings.isEnabled(Xp::NoUrlLiterals); @@ -281,7 +297,7 @@ expr_simple /* Let expressions `let {..., body = ...}' are just desugared into `(rec {..., body = ...}).body'. */ | LET '{' binds '}' - { $3->recursive = true; $$ = new ExprSelect(noPos, $3, state->s.body); } + { $3->recursive = true; $$ = new ExprSelect(noPos, unp($3), state->s.body); } | REC '{' binds '}' { $3->recursive = true; $$ = $3; } | '{' binds '}' @@ -302,9 +318,9 @@ string_parts_interpolated : string_parts_interpolated STR { $$ = $1; $1->emplace_back(state->at(@2), new ExprString(std::string($2))); } | string_parts_interpolated DOLLAR_CURLY expr '}' { $$ = $1; $1->emplace_back(state->at(@2), $3); } - | DOLLAR_CURLY expr '}' { $$ = new std::vector>; $$->emplace_back(state->at(@1), $2); } + | DOLLAR_CURLY expr '}' { $$ = new std::vector>>; $$->emplace_back(state->at(@1), $2); } | STR DOLLAR_CURLY expr '}' { - $$ = new std::vector>; + $$ = new std::vector>>; $$->emplace_back(state->at(@1), new ExprString(std::string($1))); $$->emplace_back(state->at(@2), $3); } @@ -332,14 +348,14 @@ path_start ind_string_parts : ind_string_parts IND_STR { $$ = $1; $1->emplace_back(state->at(@2), $2); } - | ind_string_parts DOLLAR_CURLY expr '}' { $$ = $1; $1->emplace_back(state->at(@2), $3); } - | { $$ = new std::vector>>; } + | ind_string_parts DOLLAR_CURLY expr '}' { $$ = $1; $1->emplace_back(state->at(@2), unp($3)); } + | { $$ = new std::vector, StringToken>>>; } ; binds : binds attrpath '=' expr ';' { $$ = $1; - if (auto e = state->addAttr($$, std::move(*$2), $4, state->at(@2))) THROW(*e, $1, $2); + if (auto e = state->addAttr($$, std::move(*$2), unp($4), state->at(@2))) THROW(*e, $1, $2); delete $2; } | binds INHERIT attrs ';' @@ -349,23 +365,23 @@ binds THROW(state->dupAttr(i.symbol, iPos, $$->attrs[i.symbol].pos), $1); $$->attrs.emplace( i.symbol, - ExprAttrs::AttrDef(new ExprVar(iPos, i.symbol), iPos, ExprAttrs::AttrDef::Kind::Inherited)); + ExprAttrs::AttrDef(std::make_unique(iPos, i.symbol), iPos, ExprAttrs::AttrDef::Kind::Inherited)); } delete $3; } | binds INHERIT '(' expr ')' attrs ';' { $$ = $1; if (!$$->inheritFromExprs) - $$->inheritFromExprs = std::make_unique>(); - $$->inheritFromExprs->push_back($4); - auto from = new nix::ExprInheritFrom(state->at(@4), $$->inheritFromExprs->size() - 1); + $$->inheritFromExprs = std::make_unique>>(); + $$->inheritFromExprs->push_back(unp($4)); for (auto & [i, iPos] : *$6) { if ($$->attrs.find(i.symbol) != $$->attrs.end()) THROW(state->dupAttr(i.symbol, iPos, $$->attrs[i.symbol].pos), $1); + auto from = std::make_unique(state->at(@4), $$->inheritFromExprs->size() - 1); $$->attrs.emplace( i.symbol, ExprAttrs::AttrDef( - new ExprSelect(iPos, from, i.symbol), + std::make_unique(iPos, std::move(from), i.symbol), iPos, ExprAttrs::AttrDef::Kind::InheritedFrom)); } @@ -400,7 +416,7 @@ attrpath $$->push_back(AttrName(state->symbols.create(str->s))); delete str; } else - $$->push_back(AttrName($3)); + $$->emplace_back(unp($3)); } | attr { $$ = new std::vector; $$->push_back(AttrName(state->symbols.create($1))); } | string_attr @@ -410,7 +426,7 @@ attrpath $$->push_back(AttrName(state->symbols.create(str->s))); delete str; } else - $$->push_back(AttrName($1)); + $$->emplace_back(unp($1)); } ; @@ -425,15 +441,15 @@ string_attr ; expr_list - : expr_list expr_select { $$ = $1; $1->elems.push_back($2); /* !!! dangerous */ } + : expr_list expr_select { $$ = $1; $1->elems.emplace_back($2); /* !!! dangerous */ } | { $$ = new ExprList; } ; formals : formal ',' formals - { $$ = $3; $$->formals.emplace_back(*$1); delete $1; } + { $$ = $3; $$->formals.emplace_back(std::move(*$1)); delete $1; } | formal - { $$ = new Formals; $$->formals.emplace_back(*$1); $$->ellipsis = false; delete $1; } + { $$ = new Formals; $$->formals.emplace_back(std::move(*$1)); $$->ellipsis = false; delete $1; } | { $$ = new Formals; $$->ellipsis = false; } | ELLIPSIS @@ -441,8 +457,8 @@ formals ; formal - : ID { $$ = new Formal{CUR_POS, state->symbols.create($1), 0}; } - | ID '?' expr { $$ = new Formal{CUR_POS, state->symbols.create($1), $3}; } + : ID { $$ = new Formal{CUR_POS, state->symbols.create($1), nullptr}; } + | ID '?' expr { $$ = new Formal{CUR_POS, state->symbols.create($1), unp($3)}; } ; %% diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 720ab1a3f..4b7e61dfe 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -2817,7 +2817,7 @@ static void prim_functionArgs(EvalState & state, const PosIdx pos, Value * * arg auto attrs = state.buildBindings(args[0]->lambda.fun->formals->formals.size()); for (auto & i : args[0]->lambda.fun->formals->formals) // !!! should optimise booleans (allocate only once) - attrs.alloc(i.name, i.pos).mkBool(i.def); + attrs.alloc(i.name, i.pos).mkBool(i.def != nullptr); v.mkAttrs(attrs); } diff --git a/src/libexpr/value-to-xml.cc b/src/libexpr/value-to-xml.cc index fda360eff..d77fdf96e 100644 --- a/src/libexpr/value-to-xml.cc +++ b/src/libexpr/value-to-xml.cc @@ -140,7 +140,7 @@ static void printValueAsXML(EvalState & state, bool strict, bool location, if (v.lambda.fun->arg) attrs["name"] = state.symbols[v.lambda.fun->arg]; if (v.lambda.fun->formals->ellipsis) attrs["ellipsis"] = "1"; XMLOpenElement _(doc, "attrspat", attrs); - for (auto & i : v.lambda.fun->formals->lexicographicOrder(state.symbols)) + for (const Formal & i : v.lambda.fun->formals->lexicographicOrder(state.symbols)) doc.writeEmptyElement("attr", singletonAttrs("name", state.symbols[i.name])); } else doc.writeEmptyElement("varpat", singletonAttrs("name", state.symbols[v.lambda.fun->arg])); diff --git a/src/nix/flake.cc b/src/nix/flake.cc index d24f746f8..63b250d13 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -446,7 +446,7 @@ struct CmdFlakeCheck : FlakeCommand if (v.lambda.fun->hasFormals() || !argHasName(v.lambda.fun->arg, "final")) throw Error("overlay does not take an argument named 'final'"); - auto body = dynamic_cast(v.lambda.fun->body); + auto body = dynamic_cast(v.lambda.fun->body.get()); if (!body || body->hasFormals() || !argHasName(body->arg, "prev")) diff --git a/tests/unit/libexpr/value/print.cc b/tests/unit/libexpr/value/print.cc index feabc07c1..cdbc8f8f9 100644 --- a/tests/unit/libexpr/value/print.cc +++ b/tests/unit/libexpr/value/print.cc @@ -113,10 +113,8 @@ TEST_F(ValuePrintingTests, vLambda) }; PosTable::Origin origin = state.positions.addOrigin(std::monostate(), 1); auto posIdx = state.positions.add(origin, 0); - auto body = ExprInt(0); - auto formals = Formals {}; - ExprLambda eLambda(posIdx, createSymbol("a"), &formals, &body); + ExprLambda eLambda(posIdx, createSymbol("a"), std::make_unique(), std::make_unique(0)); Value vLambda; vLambda.mkLambda(&env, &eLambda); @@ -515,11 +513,10 @@ TEST_F(ValuePrintingTests, ansiColorsDerivationError) TEST_F(ValuePrintingTests, ansiColorsAssert) { - ExprVar eFalse(state.symbols.create("false")); - eFalse.bindVars(state, state.staticBaseEnv); - ExprInt eInt(1); - - ExprAssert expr(noPos, &eFalse, &eInt); + ExprAssert expr(noPos, + std::make_unique(state.symbols.create("false")), + std::make_unique(1)); + expr.bindVars(state, state.staticBaseEnv); Value v; state.mkThunk_(v, expr); @@ -561,10 +558,8 @@ TEST_F(ValuePrintingTests, ansiColorsLambda) }; PosTable::Origin origin = state.positions.addOrigin(std::monostate(), 1); auto posIdx = state.positions.add(origin, 0); - auto body = ExprInt(0); - auto formals = Formals {}; - ExprLambda eLambda(posIdx, createSymbol("a"), &formals, &body); + ExprLambda eLambda(posIdx, createSymbol("a"), std::make_unique(), std::make_unique(0)); Value vLambda; vLambda.mkLambda(&env, &eLambda);