From d04fcb57fd3cd49b0cf5bc83206f8e38784ec416 Mon Sep 17 00:00:00 2001 From: piegames Date: Sun, 1 Mar 2026 19:32:29 +0100 Subject: [PATCH] libexpr: Don't call `setName` on dynamic attrs And also document in great detail why this is a wrong thing to do Change-Id: Ifd1331ee7ee4e05322593ada801bab1c3ea8d349 --- lix/libexpr/eval-expr.cc | 24 +++++++++++++++++++++++- lix/libexpr/nixexpr.hh | 4 ++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/lix/libexpr/eval-expr.cc b/lix/libexpr/eval-expr.cc index cc802098d..d4e6f6111 100644 --- a/lix/libexpr/eval-expr.cc +++ b/lix/libexpr/eval-expr.cc @@ -187,7 +187,29 @@ Value ExprSet::eval(EvalState & state, Env & env) .debugThrow(); } - i.valueExpr->setName(nameSym); + // clang-format off + /* This line is so wrong that it is best kept in here with the documentation why it is wrong, + * lest some naive soul may add it once again some year in the future. + * See the following witness as to why it is wrong: + * + * nix-repl> fun = (name: { ${name} = x: x; }) # This function creates a dynamic attribute with a variable name + * Added fun. + * nix-repl> revSeq = x: y: builtins.seq x (builtins.seq y x) # evaluate x, then y in sequence, then return x + * Added revSeq. + * nix-repl> fun "foo" # The code seemingly works + * { foo = «lambda foo @ «string»:1:26»; } + * nix-repl> fun "bar" # + * { bar = «lambda bar @ «string»:1:26»; } + * nix-repl> revSeq (fun "foo") (fun "bar") # Until it doesn't + * { foo = «lambda bar @ «string»:1:26»; } + * + * What happened? Expressions are AST bound, therefore all lambdas share the same Expr and thus *the same name*. + * Using `setName` here updates the name of *all* lambdas from that expression, not just of the value at hand. + * And this is why all expressions must be treated as immutable after parsing. + */ + /* i.valueExpr->setName(nameSym); */ + // clang-format on + /* Keep sorted order so find can catch duplicates */ v.attrs()->push_back(Attr(nameSym, i.valueExpr->maybeThunk(state, *dynamicEnv), i.pos)); v.attrs()->sort(); // FIXME: inefficient diff --git a/lix/libexpr/nixexpr.hh b/lix/libexpr/nixexpr.hh index 0f374266b..8d7cabeef 100644 --- a/lix/libexpr/nixexpr.hh +++ b/lix/libexpr/nixexpr.hh @@ -130,6 +130,10 @@ public: virtual void accept(ExprVisitor & ev, std::unique_ptr & ptr) = 0; virtual Value eval(EvalState & state, Env & env); virtual Value maybeThunk(EvalState & state, Env & env); + /* Lambdas have a name associated with them, when they are declared in a binding: + * `identity = x: x` will print the resulting value as `«lambda identity @ «string»:1:14»`. + * This is set in the parser. After parsing, all expressions are immutable. + */ virtual void setName(Symbol name); PosIdx getPos() const { return pos; }