From 5ffc9fd2536710bdc5493c1ff9cf8fa74f5bc1d5 Mon Sep 17 00:00:00 2001 From: Taeer Bar-Yam Date: Thu, 6 Nov 2025 19:51:09 +0100 Subject: [PATCH 1/3] parser.y: remove pointless std::move()s --- src/libexpr/include/nix/expr/nixexpr.hh | 8 ++++---- src/libexpr/include/nix/expr/parser-state.hh | 9 +++++---- src/libexpr/parser.y | 20 ++++++++++---------- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/libexpr/include/nix/expr/nixexpr.hh b/src/libexpr/include/nix/expr/nixexpr.hh index 3f7c883f3..3bf92cc12 100644 --- a/src/libexpr/include/nix/expr/nixexpr.hh +++ b/src/libexpr/include/nix/expr/nixexpr.hh @@ -289,7 +289,7 @@ struct ExprSelect : Expr std::pmr::polymorphic_allocator & alloc, const PosIdx & pos, Expr * e, - std::span attrPath, + const std::span & attrPath, Expr * def) : pos(pos) , nAttrPath(attrPath.size()) @@ -339,7 +339,7 @@ struct ExprOpHasAttr : Expr Expr * e; std::span attrPath; - ExprOpHasAttr(std::pmr::polymorphic_allocator & alloc, Expr * e, std::vector attrPath) + ExprOpHasAttr(std::pmr::polymorphic_allocator & alloc, Expr * e, const std::vector & attrPath) : e(e) , attrPath({alloc.allocate_object(attrPath.size()), attrPath.size()}) { @@ -433,7 +433,7 @@ struct ExprList : Expr { std::span elems; - ExprList(std::pmr::polymorphic_allocator & alloc, std::vector exprs) + ExprList(std::pmr::polymorphic_allocator & alloc, const std::vector & exprs) : elems({alloc.allocate_object(exprs.size()), exprs.size()}) { std::ranges::copy(exprs, elems.begin()); @@ -562,7 +562,7 @@ public: const PosTable & positions, std::pmr::polymorphic_allocator & alloc, PosIdx pos, - FormalsBuilder formals, + const FormalsBuilder & formals, Expr * body) : ExprLambda(positions, alloc, pos, Symbol(), formals, body) {}; diff --git a/src/libexpr/include/nix/expr/parser-state.hh b/src/libexpr/include/nix/expr/parser-state.hh index 661584ea0..0020d001e 100644 --- a/src/libexpr/include/nix/expr/parser-state.hh +++ b/src/libexpr/include/nix/expr/parser-state.hh @@ -96,7 +96,8 @@ struct ParserState ExprAttrs * attrs, AttrPath && attrPath, const ParserLocation & loc, Expr * e, const ParserLocation & exprLoc); void addAttr(ExprAttrs * attrs, AttrPath & attrPath, const Symbol & symbol, ExprAttrs::AttrDef && def); void validateFormals(FormalsBuilder & formals, PosIdx pos = noPos, Symbol arg = {}); - Expr * stripIndentation(const PosIdx pos, std::vector>> && es); + Expr * + stripIndentation(const PosIdx pos, const std::vector>> & es); PosIdx at(const ParserLocation & loc); }; @@ -238,8 +239,8 @@ inline void ParserState::validateFormals(FormalsBuilder & formals, PosIdx pos, S {.msg = HintFmt("duplicate formal function argument '%1%'", symbols[arg]), .pos = positions[pos]}); } -inline Expr * -ParserState::stripIndentation(const PosIdx pos, std::vector>> && es) +inline Expr * ParserState::stripIndentation( + const PosIdx pos, const std::vector>> & es) { if (es.empty()) return exprs.add(""_sds); @@ -343,7 +344,7 @@ ParserState::stripIndentation(const PosIdx pos, std::vector(exprs.alloc, pos, true, std::move(es2)); + return exprs.add(exprs.alloc, pos, true, es2); } inline PosIdx LexerState::at(const ParserLocation & loc) diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 8b56eba15..82c7b964f 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -186,7 +186,7 @@ expr_function | formal_set ':' expr_function[body] { state->validateFormals($formal_set); - auto me = state->exprs.add(state->positions, state->exprs.alloc, CUR_POS, std::move($formal_set), $body); + auto me = state->exprs.add(state->positions, state->exprs.alloc, CUR_POS, $formal_set, $body); $$ = me; SET_DOC_POS(me, @1); } @@ -194,7 +194,7 @@ expr_function { auto arg = state->symbols.create($ID); state->validateFormals($formal_set, CUR_POS, arg); - auto me = state->exprs.add(state->positions, state->exprs.alloc, CUR_POS, arg, std::move($formal_set), $body); + auto me = state->exprs.add(state->positions, state->exprs.alloc, CUR_POS, arg, $formal_set, $body); $$ = me; SET_DOC_POS(me, @1); } @@ -202,7 +202,7 @@ expr_function { auto arg = state->symbols.create($ID); state->validateFormals($formal_set, CUR_POS, arg); - auto me = state->exprs.add(state->positions, state->exprs.alloc, CUR_POS, arg, std::move($formal_set), $body); + auto me = state->exprs.add(state->positions, state->exprs.alloc, CUR_POS, arg, $formal_set, $body); $$ = me; SET_DOC_POS(me, @1); } @@ -251,7 +251,7 @@ expr_op | expr_op OR expr_op { $$ = state->exprs.add(state->at(@2), $1, $3); } | expr_op IMPL expr_op { $$ = state->exprs.add(state->at(@2), $1, $3); } | expr_op UPDATE expr_op { $$ = state->exprs.add(state->at(@2), $1, $3); } - | expr_op '?' attrpath { $$ = state->exprs.add(state->exprs.alloc, $1, std::move($3)); } + | expr_op '?' attrpath { $$ = state->exprs.add(state->exprs.alloc, $1, $3); } | expr_op '+' expr_op { $$ = state->exprs.add(state->exprs.alloc, state->at(@2), false, {{state->at(@1), $1}, {state->at(@3), $3}}); } | expr_op '-' expr_op { $$ = state->exprs.add(state->at(@2), state->exprs.add(state->s.sub), {$1, $3}); } @@ -272,9 +272,9 @@ expr_app expr_select : expr_simple '.' attrpath - { $$ = state->exprs.add(state->exprs.alloc, CUR_POS, $1, std::move($3), nullptr); } + { $$ = state->exprs.add(state->exprs.alloc, CUR_POS, $1, $3, nullptr); } | expr_simple '.' attrpath OR_KW expr_select - { $$ = state->exprs.add(state->exprs.alloc, CUR_POS, $1, std::move($3), $5); $5->warnIfCursedOr(state->symbols, state->positions); } + { $$ = state->exprs.add(state->exprs.alloc, CUR_POS, $1, $3, $5); $5->warnIfCursedOr(state->symbols, state->positions); } | /* Backwards compatibility: because Nixpkgs has a function named ‘or’, allow stuff like ‘map or [...]’. This production is problematic (see https://github.com/NixOS/nix/issues/11118) and will be refactored in the @@ -304,12 +304,12 @@ expr_simple $2); } | IND_STRING_OPEN ind_string_parts IND_STRING_CLOSE { - $$ = state->stripIndentation(CUR_POS, std::move($2)); + $$ = state->stripIndentation(CUR_POS, $2); } | path_start PATH_END | path_start string_parts_interpolated PATH_END { $2.insert($2.begin(), {state->at(@1), $1}); - $$ = state->exprs.add(state->exprs.alloc, CUR_POS, false, std::move($2)); + $$ = state->exprs.add(state->exprs.alloc, CUR_POS, false, $2); } | SPATH { std::string_view path($1.p + 1, $1.l - 2); @@ -338,12 +338,12 @@ expr_simple { $2->pos = CUR_POS; $$ = $2; } | '{' '}' { $$ = state->exprs.add(CUR_POS); } - | '[' list ']' { $$ = state->exprs.add(state->exprs.alloc, std::move($2)); } + | '[' list ']' { $$ = state->exprs.add(state->exprs.alloc, $2); } ; string_parts : STR { $$ = $1; } - | string_parts_interpolated { $$ = state->exprs.add(state->exprs.alloc, CUR_POS, true, std::move($1)); } + | string_parts_interpolated { $$ = state->exprs.add(state->exprs.alloc, CUR_POS, true, $1); } | { $$ = std::string_view(); } ; From 2d728f0c56c28ac0bcc2bf156c236c21d1f27945 Mon Sep 17 00:00:00 2001 From: Taeer Bar-Yam Date: Thu, 6 Nov 2025 19:53:17 +0100 Subject: [PATCH 2/3] parser.y: get rid of most nix:: prefix --- src/libexpr/parser.y | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 82c7b964f..df700cab2 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -126,26 +126,26 @@ static Expr * makeCall(Exprs & exprs, PosIdx pos, Expr * fn, Expr * arg) { %define api.value.type variant -%type start expr expr_function expr_if expr_op -%type expr_select expr_simple expr_app -%type expr_pipe_from expr_pipe_into +%type start expr expr_function expr_if expr_op +%type expr_select expr_simple expr_app +%type expr_pipe_from expr_pipe_into %type > list -%type binds binds1 -%type formals formal_set -%type formal -%type > attrpath -%type >> attrs -%type >> string_parts_interpolated -%type >>> ind_string_parts -%type path_start -%type > string_parts string_attr -%type attr -%token ID -%token STR IND_STR -%token INT_LIT -%token FLOAT_LIT -%token PATH HPATH SPATH PATH_END -%token URI +%type binds binds1 +%type formals formal_set +%type formal +%type > attrpath +%type >> attrs +%type >> string_parts_interpolated +%type >>> ind_string_parts +%type path_start +%type > string_parts string_attr +%type attr +%token ID +%token STR IND_STR +%token INT_LIT +%token FLOAT_LIT +%token PATH HPATH SPATH PATH_END +%token URI %token IF THEN ELSE ASSERT WITH LET IN_KW REC INHERIT EQ NEQ AND OR IMPL OR_KW %token PIPE_FROM PIPE_INTO /* <| and |> */ %token DOLLAR_CURLY /* == ${ */ @@ -425,7 +425,7 @@ binds1 if (!$accum->inheritFromExprs) $accum->inheritFromExprs = std::make_unique>(); $accum->inheritFromExprs->push_back($expr); - auto from = new nix::ExprInheritFrom(state->at(@expr), $accum->inheritFromExprs->size() - 1); + auto from = state->exprs.add(state->at(@expr), $accum->inheritFromExprs->size() - 1); for (auto & [i, iPos] : $attrs) { if ($accum->attrs.find(i.symbol) != $accum->attrs.end()) state->dupAttr(i.symbol, iPos, $accum->attrs[i.symbol].pos); From 90ba96a3d610566124529516d71f84f2a8c26ec0 Mon Sep 17 00:00:00 2001 From: Taeer Bar-Yam Date: Mon, 10 Nov 2025 21:33:20 +0100 Subject: [PATCH 3/3] libexpr: use std::span rather than const std::vector & --- src/libexpr/include/nix/expr/nixexpr.hh | 34 +++++++++++++++++--- src/libexpr/include/nix/expr/parser-state.hh | 7 ++-- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/src/libexpr/include/nix/expr/nixexpr.hh b/src/libexpr/include/nix/expr/nixexpr.hh index 3bf92cc12..673c14cb3 100644 --- a/src/libexpr/include/nix/expr/nixexpr.hh +++ b/src/libexpr/include/nix/expr/nixexpr.hh @@ -289,7 +289,7 @@ struct ExprSelect : Expr std::pmr::polymorphic_allocator & alloc, const PosIdx & pos, Expr * e, - const std::span & attrPath, + std::span attrPath, Expr * def) : pos(pos) , nAttrPath(attrPath.size()) @@ -339,7 +339,7 @@ struct ExprOpHasAttr : Expr Expr * e; std::span attrPath; - ExprOpHasAttr(std::pmr::polymorphic_allocator & alloc, Expr * e, const std::vector & attrPath) + ExprOpHasAttr(std::pmr::polymorphic_allocator & alloc, Expr * e, std::span attrPath) : e(e) , attrPath({alloc.allocate_object(attrPath.size()), attrPath.size()}) { @@ -433,7 +433,7 @@ struct ExprList : Expr { std::span elems; - ExprList(std::pmr::polymorphic_allocator & alloc, const std::vector & exprs) + ExprList(std::pmr::polymorphic_allocator & alloc, std::span exprs) : elems({alloc.allocate_object(exprs.size()), exprs.size()}) { std::ranges::copy(exprs, elems.begin()); @@ -753,7 +753,19 @@ struct ExprConcatStrings : Expr std::pmr::polymorphic_allocator & alloc, const PosIdx & pos, bool forceString, - const std::vector> & es) + std::span> es) + : pos(pos) + , forceString(forceString) + , es({alloc.allocate_object>(es.size()), es.size()}) + { + std::ranges::copy(es, this->es.begin()); + }; + + ExprConcatStrings( + std::pmr::polymorphic_allocator & alloc, + const PosIdx & pos, + bool forceString, + std::initializer_list> es) : pos(pos) , forceString(forceString) , es({alloc.allocate_object>(es.size()), es.size()}) @@ -833,7 +845,19 @@ public: add(std::pmr::polymorphic_allocator & alloc, const PosIdx & pos, bool forceString, - const std::vector> & es) + std::span> es) + requires(std::same_as) + { + return alloc.new_object(alloc, pos, forceString, es); + } + + template + [[gnu::always_inline]] + C * + add(std::pmr::polymorphic_allocator & alloc, + const PosIdx & pos, + bool forceString, + std::initializer_list> es) requires(std::same_as) { return alloc.new_object(alloc, pos, forceString, es); diff --git a/src/libexpr/include/nix/expr/parser-state.hh b/src/libexpr/include/nix/expr/parser-state.hh index 0020d001e..9e1d17b53 100644 --- a/src/libexpr/include/nix/expr/parser-state.hh +++ b/src/libexpr/include/nix/expr/parser-state.hh @@ -96,8 +96,7 @@ struct ParserState ExprAttrs * attrs, AttrPath && attrPath, const ParserLocation & loc, Expr * e, const ParserLocation & exprLoc); void addAttr(ExprAttrs * attrs, AttrPath & attrPath, const Symbol & symbol, ExprAttrs::AttrDef && def); void validateFormals(FormalsBuilder & formals, PosIdx pos = noPos, Symbol arg = {}); - Expr * - stripIndentation(const PosIdx pos, const std::vector>> & es); + Expr * stripIndentation(const PosIdx pos, std::span>> es); PosIdx at(const ParserLocation & loc); }; @@ -239,8 +238,8 @@ inline void ParserState::validateFormals(FormalsBuilder & formals, PosIdx pos, S {.msg = HintFmt("duplicate formal function argument '%1%'", symbols[arg]), .pos = positions[pos]}); } -inline Expr * ParserState::stripIndentation( - const PosIdx pos, const std::vector>> & es) +inline Expr * +ParserState::stripIndentation(const PosIdx pos, std::span>> es) { if (es.empty()) return exprs.add(""_sds);