From 97abcda9cc8db9218fe679969fa52efaac9c086d Mon Sep 17 00:00:00 2001 From: Taeer Bar-Yam Date: Tue, 25 Nov 2025 17:35:35 +0100 Subject: [PATCH] parser.y: correctly abstract over to-be-constructed ExprString Fixes the regression from eab467ecfb829182548276df7d56a4d1c525057a with dynamic attributes that a simple string expressions. Co-authored-by: Sergei Zimmerman --- src/libexpr/include/nix/expr/parser-state.hh | 73 ++++++++++++++++++++ src/libexpr/parser.y | 43 +++++------- 2 files changed, 92 insertions(+), 24 deletions(-) diff --git a/src/libexpr/include/nix/expr/parser-state.hh b/src/libexpr/include/nix/expr/parser-state.hh index c2a49a3d3..5a94f62e8 100644 --- a/src/libexpr/include/nix/expr/parser-state.hh +++ b/src/libexpr/include/nix/expr/parser-state.hh @@ -47,6 +47,79 @@ struct ParserLocation } }; +/** + * This represents a string-like parse that possibly has yet to be constructed. + * + * Examples: + * "foo" + * ${"foo" + "bar"} + * "foo.bar" + * "foo-${a}" + * + * Using this type allows us to avoid construction altogether in cases where what we actually need is the string + * contents. For example in foo."bar.baz", there is no need to construct an AST node for "bar.baz", but we don't know + * that until we bubble the value up during parsing and see that it's a node in an AttrPath. + */ +class ToBeStringyExpr +{ +private: + using Raw = std::variant; + Raw raw; + +public: + ToBeStringyExpr() = default; + + ToBeStringyExpr(std::string_view v) + : raw(v) + { + } + + ToBeStringyExpr(Expr * expr) + : raw(expr) + { + assert(expr); + } + + /** + * Visits the expression and invokes an overloaded functor object \ref f. + * If the underlying Expr has a dynamic type of ExprString the overload taking std::string_view + * is invoked. + * + * Used to consistently handle simple StringExpr ${"string"} as non-dynamic attributes. + * @see https://github.com/NixOS/nix/issues/14642 + */ + template + void visit(F && f) + { + std::visit( + overloaded{ + [&](std::string_view str) { f(str); }, + [&](Expr * expr) { + ExprString * str = dynamic_cast(expr); + if (str) + f(str->v.string_view()); + else + f(expr); + }, + [](std::monostate) { unreachable(); }}, + raw); + } + + /** + * Get or create an Expr from either an existing Expr or from a string. + * Delays the allocation or an AST node in case the parser only cares about string contents. + */ + Expr * toExpr(Exprs & exprs) + { + return std::visit( + overloaded{ + [&](std::string_view str) -> Expr * { return exprs.add(exprs.alloc, str); }, + [&](Expr * expr) { return expr; }, + [](std::monostate) -> Expr * { unreachable(); }}, + raw); + } +}; + struct LexerState { /** diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index a9166c5b5..520086e28 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -138,7 +138,7 @@ static Expr * makeCall(Exprs & exprs, PosIdx pos, Expr * fn, Expr * arg) { %type >> string_parts_interpolated %type >>> ind_string_parts %type path_start -%type > string_parts string_attr +%type string_parts string_attr %type attr %token ID %token STR IND_STR @@ -297,12 +297,7 @@ expr_simple } | INT_LIT { $$ = state->exprs.add($1); } | FLOAT_LIT { $$ = state->exprs.add($1); } - | '"' string_parts '"' { - std::visit(overloaded{ - [&](std::string_view str) { $$ = state->exprs.add(state->exprs.alloc, str); }, - [&](Expr * expr) { $$ = expr; }}, - $2); - } + | '"' string_parts '"' { $$ = $2.toExpr(state->exprs); } | IND_STRING_OPEN ind_string_parts IND_STRING_CLOSE { $$ = state->stripIndentation(CUR_POS, $2); } @@ -342,9 +337,9 @@ expr_simple ; string_parts - : STR { $$ = $1; } - | string_parts_interpolated { $$ = state->exprs.add(state->exprs.alloc, CUR_POS, true, $1); } - | { $$ = std::string_view(); } + : STR { $$ = {$1}; } + | string_parts_interpolated { $$ = {state->exprs.add(state->exprs.alloc, CUR_POS, true, $1)}; } + | { $$ = {std::string_view()}; } ; string_parts_interpolated @@ -447,15 +442,15 @@ attrs : attrs attr { $$ = std::move($1); $$.emplace_back(state->symbols.create($2), state->at(@2)); } | attrs string_attr { $$ = std::move($1); - std::visit(overloaded { + $2.visit(overloaded{ [&](std::string_view str) { $$.emplace_back(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); + throw ParseError({ + .msg = HintFmt("dynamic attributes not allowed in inherit"), + .pos = state->positions[state->at(@2)] + }); + }} + ); } | { } ; @@ -464,17 +459,17 @@ attrpath : attrpath '.' attr { $$ = std::move($1); $$.emplace_back(state->symbols.create($3)); } | attrpath '.' string_attr { $$ = std::move($1); - std::visit(overloaded { + $3.visit(overloaded{ [&](std::string_view str) { $$.emplace_back(state->symbols.create(str)); }, - [&](Expr * expr) { $$.emplace_back(expr); } - }, std::move($3)); + [&](Expr * expr) { $$.emplace_back(expr); }} + ); } | attr { $$.emplace_back(state->symbols.create($1)); } | string_attr - { std::visit(overloaded { + { $1.visit(overloaded{ [&](std::string_view str) { $$.emplace_back(state->symbols.create(str)); }, - [&](Expr * expr) { $$.emplace_back(expr); } - }, std::move($1)); + [&](Expr * expr) { $$.emplace_back(expr); }} + ); } ; @@ -485,7 +480,7 @@ attr string_attr : '"' string_parts '"' { $$ = std::move($2); } - | DOLLAR_CURLY expr '}' { $$ = $2; } + | DOLLAR_CURLY expr '}' { $$ = {$2}; } ; list