From 32b286e5d6cadadd97b0f8112644671fbb7b54df Mon Sep 17 00:00:00 2001 From: Taeer Bar-Yam Date: Mon, 20 Oct 2025 21:47:25 +0200 Subject: [PATCH] libexpr: parser.y: api.value.type variant --- src/libexpr/eval.cc | 6 +- src/libexpr/include/nix/expr/nixexpr.hh | 6 +- src/libexpr/include/nix/expr/parser-state.hh | 16 +- src/libexpr/lexer.l | 34 ++-- src/libexpr/nixexpr.cc | 4 +- src/libexpr/parser.y | 158 ++++++++----------- 6 files changed, 99 insertions(+), 125 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 800c29fad..7a00f4ddf 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2050,10 +2050,10 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v) }; // List of returned strings. References to these Values must NOT be persisted. - SmallTemporaryValueVector values(es->size()); + SmallTemporaryValueVector values(es.size()); Value * vTmpP = values.data(); - for (auto & [i_pos, i] : *es) { + for (auto & [i_pos, i] : es) { Value & vTmp = *vTmpP++; i->eval(state, env, vTmp); @@ -2097,7 +2097,7 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v) .debugThrow(); } else { if (s.empty()) - s.reserve(es->size()); + s.reserve(es.size()); /* skip canonization of first path, which would only be not canonized in the first place if it's coming from a ./${foo} type path */ diff --git a/src/libexpr/include/nix/expr/nixexpr.hh b/src/libexpr/include/nix/expr/nixexpr.hh index 863a1369d..86ad01504 100644 --- a/src/libexpr/include/nix/expr/nixexpr.hh +++ b/src/libexpr/include/nix/expr/nixexpr.hh @@ -695,11 +695,11 @@ 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(es) {}; + , es(std::move(es)) {}; PosIdx getPos() const override { diff --git a/src/libexpr/include/nix/expr/parser-state.hh b/src/libexpr/include/nix/expr/parser-state.hh index 55dce3047..18d2051d0 100644 --- a/src/libexpr/include/nix/expr/parser-state.hh +++ b/src/libexpr/include/nix/expr/parser-state.hh @@ -282,7 +282,7 @@ ParserState::stripIndentation(const PosIdx pos, std::vector>; + std::vector> es2{}; atStartOfLine = true; size_t curDropped = 0; size_t n = es.size(); @@ -290,7 +290,7 @@ ParserState::stripIndentation(const PosIdx pos, std::vectoremplace_back(i->first, e); + es2.emplace_back(i->first, e); }; const auto trimString = [&](const StringToken & t) { std::string s2; @@ -324,7 +324,7 @@ ParserState::stripIndentation(const PosIdx pos, std::vectoremplace_back(i->first, new ExprString(alloc, s2)); + es2.emplace_back(i->first, new ExprString(alloc, s2)); } }; for (; i != es.end(); ++i, --n) { @@ -333,19 +333,17 @@ ParserState::stripIndentation(const PosIdx pos, std::vectorsize() == 0) { + if (es2.size() == 0) { auto * const result = new ExprString(""); - delete es2; return result; } /* If this is a single string, then don't do a concatenation. */ - if (es2->size() == 1 && dynamic_cast((*es2)[0].second)) { - auto * const result = (*es2)[0].second; - delete es2; + if (es2.size() == 1 && dynamic_cast((es2)[0].second)) { + auto * const result = (es2)[0].second; return result; } - return new ExprConcatStrings(pos, true, es2); + return new ExprConcatStrings(pos, true, std::move(es2)); } inline PosIdx LexerState::at(const ParserLocation & loc) diff --git a/src/libexpr/lexer.l b/src/libexpr/lexer.l index f420fc13f..74a9065a4 100644 --- a/src/libexpr/lexer.l +++ b/src/libexpr/lexer.l @@ -142,11 +142,11 @@ or { return OR_KW; } return PIPE_INTO; } -{ID} { yylval->id = {yytext, (size_t) yyleng}; return ID; } +{ID} { yylval->emplace(yytext, (size_t) yyleng); return ID; } {INT} { errno = 0; std::optional numMay = string2Int(yytext); if (numMay.has_value()) { - yylval->n = NixInt{*numMay}; + yylval->emplace(*numMay); } else { throw ParseError(ErrorInfo{ .msg = HintFmt("invalid integer '%1%'", yytext), @@ -156,7 +156,7 @@ or { return OR_KW; } return INT_LIT; } {FLOAT} { errno = 0; - yylval->nf = strtod(yytext, 0); + yylval->emplace(strtod(yytext, 0)); if (errno != 0) throw ParseError(ErrorInfo{ .msg = HintFmt("invalid float '%1%'", yytext), @@ -183,7 +183,7 @@ or { return OR_KW; } /* It is impossible to match strings ending with '$' with one regex because trailing contexts are only valid at the end of a rule. (A sane but undocumented limitation.) */ - yylval->str = unescapeStr(yytext, yyleng, [&]() { return state->positions[CUR_POS]; }); + yylval->emplace(unescapeStr(yytext, yyleng, [&]() { return state->positions[CUR_POS]; })); return STR; } \$\{ { PUSH_STATE(DEFAULT); return DOLLAR_CURLY; } @@ -198,27 +198,27 @@ or { return OR_KW; } \'\'(\ *\n)? { PUSH_STATE(IND_STRING); return IND_STRING_OPEN; } ([^\$\']|\$[^\{\']|\'[^\'\$])+ { - yylval->str = {yytext, (size_t) yyleng, true}; - forceNoNullByte(yylval->str, [&]() { return state->positions[CUR_POS]; }); + yylval->emplace(yytext, (size_t) yyleng, true); + forceNoNullByte(yylval->as(), [&]() { return state->positions[CUR_POS]; }); return IND_STR; } \'\'\$ | \$ { - yylval->str = {"$", 1}; + yylval->emplace("$", 1); return IND_STR; } \'\'\' { - yylval->str = {"''", 2}; + yylval->emplace("''", 2); return IND_STR; } \'\'\\{ANY} { - yylval->str = unescapeStr(yytext + 2, yyleng - 2, [&]() { return state->positions[CUR_POS]; }); + yylval->emplace(unescapeStr(yytext + 2, yyleng - 2, [&]() { return state->positions[CUR_POS]; })); return IND_STR; } \$\{ { PUSH_STATE(DEFAULT); return DOLLAR_CURLY; } \'\' { POP_STATE(); return IND_STRING_CLOSE; } \' { - yylval->str = {"'", 1}; + yylval->emplace("'", 1); return IND_STR; } @@ -232,14 +232,14 @@ or { return OR_KW; } {PATH_SEG} { POP_STATE(); PUSH_STATE(INPATH_SLASH); - yylval->path = {yytext, (size_t) yyleng}; + yylval->emplace(yytext, (size_t) yyleng); return PATH; } {HPATH_START} { POP_STATE(); PUSH_STATE(INPATH_SLASH); - yylval->path = {yytext, (size_t) yyleng}; + yylval->emplace(yytext, (size_t) yyleng); return HPATH; } @@ -248,7 +248,7 @@ or { return OR_KW; } PUSH_STATE(INPATH_SLASH); else PUSH_STATE(INPATH); - yylval->path = {yytext, (size_t) yyleng}; + yylval->emplace(yytext, (size_t) yyleng); return PATH; } {HPATH} { @@ -256,7 +256,7 @@ or { return OR_KW; } PUSH_STATE(INPATH_SLASH); else PUSH_STATE(INPATH); - yylval->path = {yytext, (size_t) yyleng}; + yylval->emplace(yytext, (size_t) yyleng); return HPATH; } @@ -272,7 +272,7 @@ or { return OR_KW; } PUSH_STATE(INPATH_SLASH); else PUSH_STATE(INPATH); - yylval->str = {yytext, (size_t) yyleng}; + yylval->emplace(yytext, (size_t) yyleng); return STR; } {ANY} | @@ -294,8 +294,8 @@ or { return OR_KW; } }); } -{SPATH} { yylval->path = {yytext, (size_t) yyleng}; return SPATH; } -{URI} { yylval->uri = {yytext, (size_t) yyleng}; return URI; } +{SPATH} { yylval->emplace(yytext, (size_t) yyleng); return SPATH; } +{URI} { yylval->emplace(yytext, (size_t) yyleng); return URI; } %{ // Doc comment rule diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 5b9d17d49..a77e42356 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -246,7 +246,7 @@ void ExprConcatStrings::show(const SymbolTable & symbols, std::ostream & str) co { bool first = true; str << "("; - for (auto & i : *es) { + for (auto & i : es) { if (first) first = false; else @@ -564,7 +564,7 @@ void ExprConcatStrings::bindVars(EvalState & es, const std::shared_ptres) + for (auto & i : this->es) i.second->bindVars(es, env); } diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 9186fcf4b..93c944dcf 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -14,6 +14,10 @@ %code requires { +// bison adds a bunch of switch statements with default: +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wswitch-enum" + #ifndef BISON_HEADER #define BISON_HEADER @@ -120,46 +124,28 @@ static Expr * makeCall(PosIdx pos, Expr * fn, Expr * arg) { %} -%union { - // !!! We're probably leaking stuff here. - nix::Expr * e; - nix::ExprList * list; - nix::ExprAttrs * attrs; - nix::Formals * formals; - nix::Formal * formal; - nix::NixInt n; - nix::NixFloat nf; - nix::StringToken id; // !!! -> Symbol - nix::StringToken path; - nix::StringToken uri; - nix::StringToken str; - std::vector * attrNames; - std::vector> * inheritAttrs; - std::vector> * string_parts; - std::variant * to_be_string; - std::vector>> * ind_string_parts; -} +%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 expr_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 start expr expr_function expr_if expr_op +%type expr_select expr_simple expr_app +%type expr_pipe_from expr_pipe_into +%type expr_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 %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 /* == ${ */ @@ -261,9 +247,9 @@ expr_op | 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(state->alloc, $1, std::move(*$3)); delete $3; } + | expr_op '?' attrpath { $$ = new ExprOpHasAttr(state->alloc, $1, std::move($3)); } | expr_op '+' expr_op - { $$ = new ExprConcatStrings(state->at(@2), false, new std::vector >({{state->at(@1), $1}, {state->at(@3), $3}})); } + { $$ = 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}); } @@ -282,9 +268,9 @@ expr_app expr_select : expr_simple '.' attrpath - { $$ = new ExprSelect(state->alloc, CUR_POS, $1, std::move(*$3), nullptr); delete $3; } + { $$ = new ExprSelect(state->alloc, CUR_POS, $1, std::move($3), nullptr); } | expr_simple '.' attrpath OR_KW expr_select - { $$ = new ExprSelect(state->alloc, CUR_POS, $1, std::move(*$3), $5); delete $3; $5->warnIfCursedOr(state->symbols, state->positions); } + { $$ = new ExprSelect(state->alloc, CUR_POS, $1, std::move($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 @@ -311,17 +297,15 @@ expr_simple std::visit(overloaded{ [&](std::string_view str) { $$ = new ExprString(state->alloc, str); }, [&](Expr * expr) { $$ = expr; }}, - *$2); - delete $2; + $2); } | IND_STRING_OPEN ind_string_parts IND_STRING_CLOSE { - $$ = state->stripIndentation(CUR_POS, std::move(*$2)); - delete $2; + $$ = state->stripIndentation(CUR_POS, std::move($2)); } | path_start PATH_END | path_start string_parts_interpolated PATH_END { - $2->insert($2->begin(), {state->at(@1), $1}); - $$ = new ExprConcatStrings(CUR_POS, false, $2); + $2.insert($2.begin(), {state->at(@1), $1}); + $$ = new ExprConcatStrings(CUR_POS, false, std::move($2)); } | SPATH { std::string_view path($1.p + 1, $1.l - 2); @@ -354,20 +338,19 @@ expr_simple ; string_parts - : STR { $$ = new std::variant($1); } - | string_parts_interpolated { $$ = new std::variant(new ExprConcatStrings(CUR_POS, true, $1)); } - | { $$ = new std::variant(std::string_view()); } + : STR { $$ = $1; } + | string_parts_interpolated { $$ = new ExprConcatStrings(CUR_POS, true, std::move($1)); } + | { $$ = std::string_view(); } ; string_parts_interpolated : string_parts_interpolated STR - { $$ = $1; $1->emplace_back(state->at(@2), new ExprString(state->alloc, $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); } + { $$ = $1; $$.emplace_back(state->at(@2), new ExprString(state->alloc, $2)); } + | string_parts_interpolated DOLLAR_CURLY expr '}' { $$ = $1; $$.emplace_back(state->at(@2), $3); } + | DOLLAR_CURLY expr '}' { $$.emplace_back(state->at(@1), $2); } | STR DOLLAR_CURLY expr '}' { - $$ = new std::vector>; - $$->emplace_back(state->at(@1), new ExprString(state->alloc, $1)); - $$->emplace_back(state->at(@2), $3); + $$.emplace_back(state->at(@1), new ExprString(state->alloc, $1)); + $$.emplace_back(state->at(@2), $3); } ; @@ -408,9 +391,9 @@ 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 IND_STR { $$ = $1; $$.emplace_back(state->at(@2), $2); } + | ind_string_parts DOLLAR_CURLY expr '}' { $$ = $1; $$.emplace_back(state->at(@2), $3); } + | { } ; binds @@ -421,19 +404,17 @@ binds binds1 : binds1[accum] attrpath '=' expr ';' { $$ = $accum; - state->addAttr($$, std::move(*$attrpath), @attrpath, $expr, @expr); - delete $attrpath; + state->addAttr($$, std::move($attrpath), @attrpath, $expr, @expr); } | binds[accum] INHERIT attrs ';' { $$ = $accum; - for (auto & [i, iPos] : *$attrs) { + for (auto & [i, iPos] : $attrs) { if ($accum->attrs.find(i.symbol) != $accum->attrs.end()) state->dupAttr(i.symbol, iPos, $accum->attrs[i.symbol].pos); $accum->attrs.emplace( i.symbol, ExprAttrs::AttrDef(new ExprVar(iPos, i.symbol), iPos, ExprAttrs::AttrDef::Kind::Inherited)); } - delete $attrs; } | binds[accum] INHERIT '(' expr ')' attrs ';' { $$ = $accum; @@ -441,7 +422,7 @@ binds1 $accum->inheritFromExprs = std::make_unique>(); $accum->inheritFromExprs->push_back($expr); auto from = new nix::ExprInheritFrom(state->at(@expr), $accum->inheritFromExprs->size() - 1); - for (auto & [i, iPos] : *$attrs) { + for (auto & [i, iPos] : $attrs) { if ($accum->attrs.find(i.symbol) != $accum->attrs.end()) state->dupAttr(i.symbol, iPos, $accum->attrs[i.symbol].pos); $accum->attrs.emplace( @@ -451,51 +432,45 @@ binds1 iPos, ExprAttrs::AttrDef::Kind::InheritedFrom)); } - delete $attrs; } | attrpath '=' expr ';' { $$ = new ExprAttrs; - state->addAttr($$, std::move(*$attrpath), @attrpath, $expr, @expr); - delete $attrpath; + state->addAttr($$, std::move($attrpath), @attrpath, $expr, @expr); } ; attrs - : attrs attr { $$ = $1; $1->emplace_back(AttrName(state->symbols.create($2)), state->at(@2)); } + : attrs attr { $$ = $1; $$.emplace_back(AttrName(state->symbols.create($2)), state->at(@2)); } | attrs string_attr { $$ = $1; std::visit(overloaded { - [&](std::string_view str) { $$->emplace_back(AttrName(state->symbols.create(str)), state->at(@2)); }, + [&](std::string_view str) { $$.emplace_back(AttrName(state->symbols.create(str)), state->at(@2)); }, [&](Expr * expr) { throw ParseError({ .msg = HintFmt("dynamic attributes not allowed in inherit"), .pos = state->positions[state->at(@2)] }); } - }, *$2); - delete $2; + }, $2); } - | { $$ = new std::vector>; } + | { } ; attrpath - : attrpath '.' attr { $$ = $1; $1->push_back(AttrName(state->symbols.create($3))); } + : attrpath '.' attr { $$ = $1; $$.push_back(AttrName(state->symbols.create($3))); } | attrpath '.' string_attr { $$ = $1; std::visit(overloaded { - [&](std::string_view str) { $$->push_back(AttrName(state->symbols.create(str))); }, - [&](Expr * expr) { $$->push_back(AttrName(expr)); } - }, *$3); - delete $3; + [&](std::string_view str) { $$.push_back(AttrName(state->symbols.create(str))); }, + [&](Expr * expr) { $$.push_back(AttrName(expr)); } + }, $3); } - | attr { $$ = new std::vector; $$->push_back(AttrName(state->symbols.create($1))); } + | attr { $$.push_back(AttrName(state->symbols.create($1))); } | string_attr - { $$ = new std::vector; - std::visit(overloaded { - [&](std::string_view str) { $$->push_back(AttrName(state->symbols.create(str))); }, - [&](Expr * expr) { $$->push_back(AttrName(expr)); } - }, *$1); - delete $1; + { std::visit(overloaded { + [&](std::string_view str) { $$.push_back(AttrName(state->symbols.create(str))); }, + [&](Expr * expr) { $$.push_back(AttrName(expr)); } + }, $1); } ; @@ -506,7 +481,7 @@ attr string_attr : '"' string_parts '"' { $$ = $2; } - | DOLLAR_CURLY expr '}' { $$ = new std::variant($2); } + | DOLLAR_CURLY expr '}' { $$ = $2; } ; expr_list @@ -524,14 +499,14 @@ formal_set formals : formals[accum] ',' formal - { $$ = $accum; $$->formals.emplace_back(*$formal); delete $formal; } + { $$ = $accum; $$->formals.emplace_back(std::move($formal)); } | formal - { $$ = new Formals; $$->formals.emplace_back(*$formal); delete $formal; } + { $$ = new Formals; $$->formals.emplace_back(std::move($formal)); } ; formal - : ID { $$ = new Formal{CUR_POS, state->symbols.create($1), 0}; } - | ID '?' expr { $$ = new Formal{CUR_POS, state->symbols.create($1), $3}; } + : ID { $$ = Formal{CUR_POS, state->symbols.create($1), 0}; } + | ID '?' expr { $$ = Formal{CUR_POS, state->symbols.create($1), $3}; } ; %% @@ -582,3 +557,4 @@ Expr * parseExprFromBuf( } +#pragma GCC diagnostic pop // end ignored "-Wswitch-enum"