From 3d16f2a2810a755b066738f3163abe7bd0d636ff Mon Sep 17 00:00:00 2001 From: Et7f3 Date: Sun, 12 Feb 2023 05:45:25 +0100 Subject: [PATCH 1/3] parser: use implicit rule --- src/libexpr/parser.y | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index ffb364a90..6f1041d2d 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -438,7 +438,7 @@ expr_select function named ‘or’, allow stuff like ‘map or [...]’. */ expr_simple OR_KW { $$ = new ExprCall(CUR_POS, $1, {new ExprVar(CUR_POS, data->symbols.create("or"))}); } - | expr_simple { $$ = $1; } + | expr_simple ; expr_simple @@ -455,7 +455,7 @@ expr_simple | IND_STRING_OPEN ind_string_parts IND_STRING_CLOSE { $$ = stripIndentation(CUR_POS, data->symbols, *$2); } - | path_start PATH_END { $$ = $1; } + | path_start PATH_END | path_start string_parts_interpolated PATH_END { $2->insert($2->begin(), {makeCurPos(@1, data), $1}); $$ = new ExprConcatStrings(CUR_POS, false, $2); @@ -596,7 +596,7 @@ attrpath ; attr - : ID { $$ = $1; } + : ID | OR_KW { $$ = {"or", 2}; } ; From fa89d317b7c7590a59db44bb369fcfe6baee4296 Mon Sep 17 00:00:00 2001 From: Et7f3 Date: Sat, 11 Feb 2023 00:34:31 +0100 Subject: [PATCH 2/3] ExprString: Avoid copy of string --- src/libexpr/nixexpr.hh | 2 +- src/libexpr/parser.y | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index ffe67f97d..20a586f60 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -186,7 +186,7 @@ struct ExprString : Expr { std::string s; Value v; - ExprString(std::string s) : s(std::move(s)) { v.mkString(this->s.data()); }; + ExprString(std::string &&s) : s(std::move(s)) { v.mkString(this->s.data()); }; Value * maybeThunk(EvalState & state, Env & env) override; COMMON_METHODS }; diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 6f1041d2d..1dd4a801f 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -268,7 +268,7 @@ static Expr * stripIndentation(const PosIdx pos, SymbolTable & symbols, s2 = std::string(s2, 0, p + 1); } - es2->emplace_back(i->first, new ExprString(s2)); + es2->emplace_back(i->first, new ExprString(std::move(s2))); }; for (; i != es.end(); ++i, --n) { std::visit(overloaded { trimExpr, trimString }, i->second); @@ -465,7 +465,7 @@ expr_simple $$ = new ExprCall(CUR_POS, new ExprVar(data->symbols.create("__findFile")), {new ExprVar(data->symbols.create("__nixPath")), - new ExprString(path)}); + new ExprString(std::move(path))}); } | URI { static bool noURLLiterals = settings.isExperimentalFeatureEnabled(Xp::NoUrlLiterals); From cec23f5ddab31fb7704aa7d822a508690e416ab6 Mon Sep 17 00:00:00 2001 From: Et7f3 Date: Sat, 11 Feb 2023 00:37:23 +0100 Subject: [PATCH 3/3] ExprOpHasAttr,ExprSelect,stripIndentation,binds,formals: delete losts objects We are looking for *$ because it indicate that it was constructed with a new but not release. De-referencing shallow copy so deleting as whole might create dangling pointer that's why we move it so we delete a empty containers + the nice perf boost. --- src/libexpr/nixexpr.hh | 4 ++-- src/libexpr/parser.y | 21 ++++++++++++--------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index 20a586f60..4a81eaa47 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -233,7 +233,7 @@ struct ExprSelect : Expr PosIdx pos; Expr * e, * def; AttrPath attrPath; - ExprSelect(const PosIdx & pos, Expr * e, const AttrPath & attrPath, Expr * def) : pos(pos), e(e), def(def), attrPath(attrPath) { }; + ExprSelect(const PosIdx & pos, Expr * e, const 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)); }; PosIdx getPos() const override { return pos; } COMMON_METHODS @@ -243,7 +243,7 @@ struct ExprOpHasAttr : Expr { Expr * e; AttrPath attrPath; - ExprOpHasAttr(Expr * e, const AttrPath & attrPath) : e(e), attrPath(attrPath) { }; + ExprOpHasAttr(Expr * e, const AttrPath && attrPath) : e(e), attrPath(std::move(attrPath)) { }; PosIdx getPos() const override { return e->getPos(); } COMMON_METHODS }; diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 1dd4a801f..dec5818fc 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -90,7 +90,7 @@ static void dupAttr(const EvalState & state, Symbol attr, const PosIdx pos, cons } -static void addAttr(ExprAttrs * attrs, AttrPath & attrPath, +static void addAttr(ExprAttrs * attrs, AttrPath && attrPath, Expr * e, const PosIdx pos, const nix::EvalState & state) { AttrPath::iterator i; @@ -188,7 +188,7 @@ static Formals * toFormals(ParseData & data, ParserFormals * formals, static Expr * stripIndentation(const PosIdx pos, SymbolTable & symbols, - std::vector>> & es) + std::vector>> && es) { if (es.empty()) return new ExprString(""); @@ -408,7 +408,7 @@ expr_op | expr_op OR expr_op { $$ = new ExprOpOr(makeCurPos(@2, data), $1, $3); } | expr_op IMPL expr_op { $$ = new ExprOpImpl(makeCurPos(@2, data), $1, $3); } | expr_op UPDATE expr_op { $$ = new ExprOpUpdate(makeCurPos(@2, data), $1, $3); } - | expr_op '?' attrpath { $$ = new ExprOpHasAttr($1, *$3); } + | expr_op '?' attrpath { $$ = new ExprOpHasAttr($1, std::move(*$3)); delete $3; } | expr_op '+' expr_op { $$ = new ExprConcatStrings(makeCurPos(@2, data), false, new std::vector >({{makeCurPos(@1, data), $1}, {makeCurPos(@3, data), $3}})); } | expr_op '-' expr_op { $$ = new ExprCall(makeCurPos(@2, data), new ExprVar(data->symbols.create("__sub")), {$1, $3}); } @@ -431,9 +431,9 @@ expr_app expr_select : expr_simple '.' attrpath - { $$ = new ExprSelect(CUR_POS, $1, *$3, 0); } + { $$ = new ExprSelect(CUR_POS, $1, std::move(*$3), nullptr); delete $3; } | expr_simple '.' attrpath OR_KW expr_select - { $$ = new ExprSelect(CUR_POS, $1, *$3, $5); } + { $$ = new ExprSelect(CUR_POS, $1, std::move(*$3), $5); delete $3; } | /* Backwards compatibility: because Nixpkgs has a rarely used function named ‘or’, allow stuff like ‘map or [...]’. */ expr_simple OR_KW @@ -453,7 +453,8 @@ expr_simple | FLOAT { $$ = new ExprFloat($1); } | '"' string_parts '"' { $$ = $2; } | IND_STRING_OPEN ind_string_parts IND_STRING_CLOSE { - $$ = stripIndentation(CUR_POS, data->symbols, *$2); + $$ = stripIndentation(CUR_POS, data->symbols, std::move(*$2)); + delete $2; } | path_start PATH_END | path_start string_parts_interpolated PATH_END { @@ -533,7 +534,7 @@ ind_string_parts ; binds - : binds attrpath '=' expr ';' { $$ = $1; addAttr($$, *$2, $4, makeCurPos(@2, data), data->state); } + : binds attrpath '=' expr ';' { $$ = $1; addAttr($$, std::move(*$2), $4, makeCurPos(@2, data), data->state); delete $2; } | binds INHERIT attrs ';' { $$ = $1; for (auto & i : *$3) { @@ -542,6 +543,7 @@ binds auto pos = makeCurPos(@3, data); $$->attrs.emplace(i.symbol, ExprAttrs::AttrDef(new ExprVar(CUR_POS, i.symbol), pos, true)); } + delete $3; } | binds INHERIT '(' expr ')' attrs ';' { $$ = $1; @@ -551,6 +553,7 @@ binds dupAttr(data->state, i.symbol, makeCurPos(@6, data), $$->attrs[i.symbol].pos); $$->attrs.emplace(i.symbol, ExprAttrs::AttrDef(new ExprSelect(CUR_POS, $4, i.symbol), makeCurPos(@6, data))); } + delete $6; } | { $$ = new ExprAttrs(makeCurPos(@0, data)); } ; @@ -612,9 +615,9 @@ expr_list formals : formal ',' formals - { $$ = $3; $$->formals.push_back(*$1); } + { $$ = $3; $$->formals.emplace_back(*$1); delete $1; } | formal - { $$ = new ParserFormals; $$->formals.push_back(*$1); $$->ellipsis = false; } + { $$ = new ParserFormals; $$->formals.emplace_back(*$1); $$->ellipsis = false; delete $1; } | { $$ = new ParserFormals; $$->ellipsis = false; } | ELLIPSIS