Merge pull request #14646 from NixOS/fix-14642-2.32

[Backport 2.32-maintenance] Fix dynamic attributes that are simple string expressions

maintainers/flake-module.nix

@@ -79,6 +79,8 @@
               # Not supported by nixfmt
               ''^tests/functional/lang/eval-okay-deprecate-cursed-or\.nix$''
               ''^tests/functional/lang/eval-okay-attrs5\.nix$''
+              ''^tests/functional/lang/eval-fail-dynamic-attrs-inherit\.nix$''
+              ''^tests/functional/lang/eval-fail-dynamic-attrs-inherit-2\.nix$''
 
               # More syntax tests
               # These tests, or parts of them, should have been parse-* test cases.

src/libexpr/include/nix/expr/nixexpr.hh

@@ -185,28 +185,13 @@ struct ExprFloat : Expr
 
 struct ExprString : Expr
 {
+    std::string s;
     Value v;
 
-    /**
+    ExprString(std::string && s)
-     * This is only for strings already allocated in our polymorphic allocator,
+        : s(std::move(s))
-     * or that live at least that long (e.g. c++ string literals)
-     */
-    ExprString(const char * s)
     {
-        v.mkStringNoCopy(s);
+        v.mkStringNoCopy(this->s.data());
-    };
-
-    ExprString(std::pmr::polymorphic_allocator<char> & alloc, std::string_view sv)
-    {
-        auto len = sv.length();
-        if (len == 0) {
-            v.mkStringNoCopy("");
-            return;
-        }
-        char * s = alloc.allocate(len + 1);
-        sv.copy(s, len);
-        s[len] = '\0';
-        v.mkStringNoCopy(s);
     };
 
     Value * maybeThunk(EvalState & state, Env & env) override;

src/libexpr/include/nix/expr/parser-state.hh

@@ -324,7 +324,7 @@ ParserState::stripIndentation(const PosIdx pos, std::vector<std::pair<PosIdx, st
 
         // Ignore empty strings for a minor optimisation and AST simplification
         if (s2 != "") {
-            es2->emplace_back(i->first, new ExprString(alloc, s2));
+            es2->emplace_back(i->first, new ExprString(std::move(s2)));
         }
     };
     for (; i != es.end(); ++i, --n) {

src/libexpr/nixexpr.cc

@@ -40,7 +40,7 @@ void ExprFloat::show(const SymbolTable & symbols, std::ostream & str) const
 
 void ExprString::show(const SymbolTable & symbols, std::ostream & str) const
 {
-    printLiteralString(str, v.string_view());
+    printLiteralString(str, s);
 }
 
 void ExprPath::show(const SymbolTable & symbols, std::ostream & str) const

src/libexpr/parser.y

@@ -136,7 +136,6 @@ static Expr * makeCall(PosIdx pos, Expr * fn, Expr * arg) {
   std::vector<nix::AttrName> * attrNames;
   std::vector<std::pair<nix::AttrName, nix::PosIdx>> * inheritAttrs;
   std::vector<std::pair<nix::PosIdx, nix::Expr *>> * string_parts;
-  std::variant<nix::Expr *, std::string_view> * to_be_string;
   std::vector<std::pair<nix::PosIdx, std::variant<nix::Expr *, nix::StringToken>>> * ind_string_parts;
 }
 
@@ -151,8 +150,7 @@ static Expr * makeCall(PosIdx pos, Expr * fn, Expr * arg) {
 %type <inheritAttrs> attrs
 %type <string_parts> string_parts_interpolated
 %type <ind_string_parts> ind_string_parts
-%type <e> path_start
+%type <e> path_start string_parts string_attr
-%type <to_be_string> string_parts string_attr
 %type <id> attr
 %token <id> ID
 %token <str> STR IND_STR
@@ -307,13 +305,7 @@ expr_simple
   }
   | INT_LIT { $$ = new ExprInt($1); }
   | FLOAT_LIT { $$ = new ExprFloat($1); }
-  | '"' string_parts '"' {
+  | '"' string_parts '"' { $$ = $2; }
-      std::visit(overloaded{
-          [&](std::string_view str) { $$ = new ExprString(state->alloc, str); },
-          [&](Expr * expr) { $$ = expr; }},
-      *$2);
-      delete $2;
-  }
   | IND_STRING_OPEN ind_string_parts IND_STRING_CLOSE {
       $$ = state->stripIndentation(CUR_POS, std::move(*$2));
       delete $2;
@@ -324,11 +316,11 @@ expr_simple
       $$ = new ExprConcatStrings(CUR_POS, false, $2);
   }
   | SPATH {
-      std::string_view path($1.p + 1, $1.l - 2);
+      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(state->alloc, path)});
+           new ExprString(std::move(path))});
   }
   | URI {
       static bool noURLLiterals = experimentalFeatureSettings.isEnabled(Xp::NoUrlLiterals);
@@ -337,7 +329,7 @@ expr_simple
               .msg = HintFmt("URL literals are disabled"),
               .pos = state->positions[CUR_POS]
           });
-      $$ = new ExprString(state->alloc, $1);
+      $$ = new ExprString(std::string($1));
   }
   | '(' expr ')' { $$ = $2; }
   /* Let expressions `let {..., body = ...}' are just desugared
@@ -354,19 +346,19 @@ expr_simple
   ;
 
 string_parts
-  : STR { $$ = new std::variant<Expr *, std::string_view>($1); }
+  : STR { $$ = new ExprString(std::string($1)); }
-  | string_parts_interpolated { $$ = new std::variant<Expr *, std::string_view>(new ExprConcatStrings(CUR_POS, true, $1)); }
+  | string_parts_interpolated { $$ = new ExprConcatStrings(CUR_POS, true, $1); }
-  | { $$ = new std::variant<Expr *, std::string_view>(std::string_view()); }
+  | { $$ = new ExprString(""); }
   ;
 
 string_parts_interpolated
   : string_parts_interpolated STR
-  { $$ = $1; $1->emplace_back(state->at(@2), new ExprString(state->alloc, $2)); }
+  { $$ = $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<std::pair<PosIdx, Expr *>>; $$->emplace_back(state->at(@1), $2); }
   | STR DOLLAR_CURLY expr '}' {
       $$ = new std::vector<std::pair<PosIdx, Expr *>>;
-      $$->emplace_back(state->at(@1), new ExprString(state->alloc, $1));
+      $$->emplace_back(state->at(@1), new ExprString(std::string($1)));
       $$->emplace_back(state->at(@2), $3);
     }
   ;
@@ -464,17 +456,16 @@ attrs
   : attrs attr { $$ = $1; $1->emplace_back(AttrName(state->symbols.create($2)), state->at(@2)); }
   | attrs string_attr
     { $$ = $1;
-      std::visit(overloaded {
+      ExprString * str = dynamic_cast<ExprString *>($2);
-          [&](std::string_view str) { $$->emplace_back(AttrName(state->symbols.create(str)), state->at(@2)); },
+      if (str) {
-          [&](Expr * expr) {
+          $$->emplace_back(AttrName(state->symbols.create(str->s)), state->at(@2));
+          delete str;
+      } else
           throw ParseError({
               .msg = HintFmt("dynamic attributes not allowed in inherit"),
               .pos = state->positions[state->at(@2)]
           });
     }
-      }, *$2);
-      delete $2;
-    }
   | { $$ = new std::vector<std::pair<AttrName, PosIdx>>; }
   ;
 
@@ -482,20 +473,22 @@ attrpath
   : attrpath '.' attr { $$ = $1; $1->push_back(AttrName(state->symbols.create($3))); }
   | attrpath '.' string_attr
     { $$ = $1;
-      std::visit(overloaded {
+      ExprString * str = dynamic_cast<ExprString *>($3);
-          [&](std::string_view str) { $$->push_back(AttrName(state->symbols.create(str))); },
+      if (str) {
-          [&](Expr * expr) { $$->push_back(AttrName(expr)); }
+          $$->push_back(AttrName(state->symbols.create(str->s)));
-      }, *$3);
+          delete str;
-      delete $3;
+      } else
+          $$->push_back(AttrName($3));
     }
   | attr { $$ = new std::vector<AttrName>; $$->push_back(AttrName(state->symbols.create($1))); }
   | string_attr
     { $$ = new std::vector<AttrName>;
-      std::visit(overloaded {
+      ExprString *str = dynamic_cast<ExprString *>($1);
-          [&](std::string_view str) { $$->push_back(AttrName(state->symbols.create(str))); },
+      if (str) {
-          [&](Expr * expr) { $$->push_back(AttrName(expr)); }
+          $$->push_back(AttrName(state->symbols.create(str->s)));
-      }, *$1);
+          delete str;
-      delete $1;
+      } else
+          $$->push_back(AttrName($1));
     }
   ;
 
@@ -506,7 +499,7 @@ attr
 
 string_attr
   : '"' string_parts '"' { $$ = $2; }
-  | DOLLAR_CURLY expr '}' { $$ = new std::variant<Expr *, std::string_view>($2); }
+  | DOLLAR_CURLY expr '}' { $$ = $2; }
   ;
 
 expr_list

tests/functional/lang/eval-fail-dynamic-attrs-inherit-2.err.exp

@@ -0,0 +1,6 @@
+error: dynamic attributes not allowed in inherit
+       at /pwd/lang/eval-fail-dynamic-attrs-inherit-2.nix:5:15:
+            4| {
+            5|   inherit (a) ${"b" + ""};
+             |               ^
+            6| }

tests/functional/lang/eval-fail-dynamic-attrs-inherit-2.nix

@@ -0,0 +1,6 @@
+let
+  a.b = 1;
+in
+{
+  inherit (a) ${"b" + ""};
+}

tests/functional/lang/eval-fail-dynamic-attrs-inherit.err.exp

@@ -0,0 +1,6 @@
+error: dynamic attributes not allowed in inherit
+       at /pwd/lang/eval-fail-dynamic-attrs-inherit.nix:5:11:
+            4| {
+            5|   inherit ${"a" + ""};
+             |           ^
+            6| }

tests/functional/lang/eval-fail-dynamic-attrs-inherit.nix

@@ -0,0 +1,6 @@
+let
+  a = 1;
+in
+{
+  inherit ${"a" + ""};
+}

tests/functional/lang/eval-fail-dynamic-attrs-let-2.err.exp

@@ -0,0 +1,5 @@
+error: dynamic attributes not allowed in let
+       at /pwd/lang/eval-fail-dynamic-attrs-let-2.nix:1:1:
+            1| let
+             | ^
+            2|   ${"${"a"}"} = 1;

tests/functional/lang/eval-fail-dynamic-attrs-let-2.nix

@@ -0,0 +1,4 @@
+let
+  ${"${"a"}"} = 1;
+in
+a

tests/functional/lang/eval-fail-dynamic-attrs-let-3.err.exp

@@ -0,0 +1,5 @@
+error: dynamic attributes not allowed in let
+       at /pwd/lang/eval-fail-dynamic-attrs-let-3.nix:1:1:
+            1| let
+             | ^
+            2|   "${"a"}" = 1;

tests/functional/lang/eval-fail-dynamic-attrs-let-3.nix

@@ -0,0 +1,4 @@
+let
+  "${"a"}" = 1;
+in
+a

tests/functional/lang/eval-fail-dynamic-attrs-let.err.exp

@@ -0,0 +1,5 @@
+error: dynamic attributes not allowed in let
+       at /pwd/lang/eval-fail-dynamic-attrs-let.nix:1:1:
+            1| let
+             | ^
+            2|   ${"a" + ""} = 1;

tests/functional/lang/eval-fail-dynamic-attrs-let.nix

@@ -0,0 +1,4 @@
+let
+  ${"a" + ""} = 1;
+in
+a

tests/functional/lang/eval-okay-dynamic-attrs-3.exp

@@ -0,0 +1 @@
+{ a = 1; attrs = { b = 1; c = 1; d = 1; }; b = 1; c = 1; d = 1; }

tests/functional/lang/eval-okay-dynamic-attrs-3.nix

@@ -0,0 +1,14 @@
+# dynamic attrs are not generally allowed in `let`, and inherit, but they are if they only contain a string
+let
+  ${"a"} = 1;
+  attrs = rec {
+    b = c;
+    ${"c"} = d;
+    d = a;
+  };
+in
+{
+  inherit ${"a"};
+  inherit attrs;
+  inherit (attrs) ${"b"} ${"c"} d;
+}