From 057b725ae3bf7337d04d2abe7941805df1012551 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Sat, 20 Sep 2025 17:08:21 +0200 Subject: [PATCH] libexpr: use const references or pointers to attribute sets Prior to this change, references or pointers could be mutated. In practice, we do not require this capability in the codebase except in zipAttrsWith. This cleans up all easy sites in preparation to have a smarter representation of attribute sets albeit one that requires constant references. Change-Id: I2be20cce040a9228bde9e5f7b42c0499fba9550b Signed-off-by: Raito Bezarius Co-authored-by: Sergei Zimmerman --- lix/legacy/nix-env.cc | 2 +- lix/legacy/user-env.cc | 4 ++-- lix/libcmd/repl.cc | 4 +++- lix/libexpr/eval.cc | 2 +- lix/libexpr/flake/flake.cc | 2 +- lix/libexpr/get-drvs.cc | 11 ++++++----- lix/libexpr/value-to-json.cc | 2 +- lix/libexpr/value-to-xml.cc | 2 +- tests/unit/libexpr/primops.cc | 18 +++++++++--------- tests/unit/libexpr/trivial.cc | 12 ++++++------ 10 files changed, 31 insertions(+), 28 deletions(-) diff --git a/lix/legacy/nix-env.cc b/lix/legacy/nix-env.cc index 9ce7fbc66..6c5be67a6 100644 --- a/lix/legacy/nix-env.cc +++ b/lix/legacy/nix-env.cc @@ -1299,7 +1299,7 @@ static void opQuery(Globals & globals, Strings opFlags, Strings opArgs) XMLOpenElement m(xml, "meta", attrs2); Bindings & attrs = *v->attrs; for (auto &i : attrs) { - Attr & a(*attrs.find(i.name)); + const Attr & a(*attrs.find(i.name)); if(a.value->type() != nString) continue; XMLAttrs attrs3; attrs3["type"] = globals.state->symbols[i.name]; diff --git a/lix/legacy/user-env.cc b/lix/legacy/user-env.cc index 5f147ddbe..ed98a32e6 100644 --- a/lix/legacy/user-env.cc +++ b/lix/legacy/user-env.cc @@ -114,9 +114,9 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, debug("evaluating user environment builder"); state.forceValue(topLevel, noPos); NixStringContext context; - Attr & aDrvPath(*topLevel.attrs->find(state.ctx.s.drvPath)); + const Attr & aDrvPath(*topLevel.attrs->find(state.ctx.s.drvPath)); auto topLevelDrv = state.coerceToStorePath(aDrvPath.pos, *aDrvPath.value, context, ""); - Attr & aOutPath(*topLevel.attrs->find(state.ctx.s.outPath)); + const Attr & aOutPath(*topLevel.attrs->find(state.ctx.s.outPath)); auto topLevelOut = state.coerceToStorePath(aOutPath.pos, *aOutPath.value, context, ""); /* Realise the resulting store expression. */ diff --git a/lix/libcmd/repl.cc b/lix/libcmd/repl.cc index f17f0b28c..7b3012287 100644 --- a/lix/libcmd/repl.cc +++ b/lix/libcmd/repl.cc @@ -1089,7 +1089,9 @@ void NixRepl::addToScope(T && things, NameFn nameFn, ValueFn valueFn) void NixRepl::addAttrsToScope(Value & attrs) { state.forceAttrs(attrs, noPos, "while evaluating an attribute set to be merged in the global scope"); - addToScope(*attrs.attrs, [](Attr & a) { return a.name; }, [](Attr & a) { return a.value; }); + addToScope( + *attrs.attrs, [](const Attr & a) { return a.name; }, [](const Attr & a) { return a.value; } + ); } void NixRepl::addValMapToScope(const ValMap & attrs) diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc index d8917cb39..1cb396779 100644 --- a/lix/libexpr/eval.cc +++ b/lix/libexpr/eval.cc @@ -1563,7 +1563,7 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & } }; - Attr * functor; + const Attr * functor; while (nrArgs > 0) { diff --git a/lix/libexpr/flake/flake.cc b/lix/libexpr/flake/flake.cc index eb91d4b62..ea5037727 100644 --- a/lix/libexpr/flake/flake.cc +++ b/lix/libexpr/flake/flake.cc @@ -231,7 +231,7 @@ static std::pair, std::optional> expectType(state, nAttrs, *value, pos); std::optional selfAttrs = std::nullopt; - for (nix::Attr & inputAttr : *(*value).attrs) { + for (const nix::Attr & inputAttr : *(*value).attrs) { std::string inputName{state.ctx.symbols[inputAttr.name]}; if (inputName == "self") { experimentalFeatureSettings.require(Xp::FlakeSelfAttrs); diff --git a/lix/libexpr/get-drvs.cc b/lix/libexpr/get-drvs.cc index 413401136..3465c8b58 100644 --- a/lix/libexpr/get-drvs.cc +++ b/lix/libexpr/get-drvs.cc @@ -133,7 +133,7 @@ void DrvInfo::fillOutputs(EvalState & state, bool withPaths) return; } - Attr * outputs = this->attrs->get(state.ctx.s.outputs); + const Attr * outputs = this->attrs->get(state.ctx.s.outputs); if (outputs == nullptr) { fillDefault(); return; @@ -161,7 +161,7 @@ void DrvInfo::fillOutputs(EvalState & state, bool withPaths) if (withPaths) { // Find the attr with this output's name... - Attr * out = this->attrs->get(state.ctx.symbols.create(outputName)); + const Attr * out = this->attrs->get(state.ctx.symbols.create(outputName)); if (out == nullptr) { // FIXME: throw error? continue; @@ -172,7 +172,7 @@ void DrvInfo::fillOutputs(EvalState & state, bool withPaths) state.forceAttrs(*out->value, outputs->pos, errMsg); // ...and evaluate its `outPath` attribute. - Attr * outPath = out->value->attrs->get(state.ctx.s.outPath); + const Attr * outPath = out->value->attrs->get(state.ctx.s.outPath); if (outPath == nullptr) { continue; // FIXME: throw error? @@ -216,7 +216,7 @@ DrvInfo::Outputs DrvInfo::queryOutputs(EvalState & state, bool withPaths, bool o // output by its attribute, e.g. `pkgs.lix.dev`, which (lol?) sets the magic // attribute `outputSpecified = true`, and changes the `outputName` attr to the // explicitly selected-into output. - if (Attr * outSpecAttr = attrs->get(state.ctx.s.outputSpecified)) { + if (const Attr * outSpecAttr = attrs->get(state.ctx.s.outputSpecified)) { bool outputSpecified = state.forceBool( *outSpecAttr->value, outSpecAttr->pos, @@ -503,7 +503,8 @@ static void getDerivations(EvalState & state, Value & vIn, PosIdx pos, should we recurse into it? => Only if it has a `recurseForDerivations = true' attribute. */ if (attr->value->type() == nAttrs) { - Attr * recurseForDrvs = attr->value->attrs->get(state.ctx.s.recurseForDerivations); + const Attr * recurseForDrvs = + attr->value->attrs->get(state.ctx.s.recurseForDerivations); if (recurseForDrvs == nullptr) { continue; } diff --git a/lix/libexpr/value-to-json.cc b/lix/libexpr/value-to-json.cc index af0f7b9e1..a2af9a9b6 100644 --- a/lix/libexpr/value-to-json.cc +++ b/lix/libexpr/value-to-json.cc @@ -58,7 +58,7 @@ JSON printValueAsJSON(EvalState & state, bool strict, for (auto & j : *v.attrs) names.emplace(state.ctx.symbols[j.name]); for (auto & j : names) { - Attr & a(*v.attrs->find(state.ctx.symbols.create(j))); + const Attr & a(*v.attrs->find(state.ctx.symbols.create(j))); try { out[j] = printValueAsJSON(state, strict, *a.value, a.pos, context, copyToStore); } catch (Error & e) { diff --git a/lix/libexpr/value-to-xml.cc b/lix/libexpr/value-to-xml.cc index 034e4f343..4f313f23a 100644 --- a/lix/libexpr/value-to-xml.cc +++ b/lix/libexpr/value-to-xml.cc @@ -36,7 +36,7 @@ static void showAttrs(EvalState & state, bool strict, bool location, names.emplace(state.ctx.symbols[i.name]); for (auto & i : names) { - Attr & a(*attrs.find(state.ctx.symbols.create(i))); + const Attr & a(*attrs.find(state.ctx.symbols.create(i))); XMLAttrs xmlAttrs; xmlAttrs["name"] = i; diff --git a/tests/unit/libexpr/primops.cc b/tests/unit/libexpr/primops.cc index e17f6a29e..ee1205746 100644 --- a/tests/unit/libexpr/primops.cc +++ b/tests/unit/libexpr/primops.cc @@ -184,7 +184,7 @@ namespace nix { TEST_F(PrimOpTest, removeAttrsRetains) { auto v = eval("builtins.removeAttrs { x = 1; y = 2; } [\"x\"]"); ASSERT_THAT(v, IsAttrsOfSize(1)); - ASSERT_NE(v.attrs->find(createSymbol("y")), nullptr); + ASSERT_NE(v.attrs->get(createSymbol("y")), nullptr); } TEST_F(PrimOpTest, listToAttrsEmptyList) { @@ -201,7 +201,7 @@ namespace nix { TEST_F(PrimOpTest, listToAttrs) { auto v = eval("builtins.listToAttrs [ { name = \"key\"; value = 123; } ]"); ASSERT_THAT(v, IsAttrsOfSize(1)); - auto key = v.attrs->find(createSymbol("key")); + auto key = v.attrs->get(createSymbol("key")); ASSERT_NE(key, nullptr); ASSERT_THAT(*key->value, IsIntEq(123)); } @@ -209,7 +209,7 @@ namespace nix { TEST_F(PrimOpTest, intersectAttrs) { auto v = eval("builtins.intersectAttrs { a = 1; b = 2; } { b = 3; c = 4; }"); ASSERT_THAT(v, IsAttrsOfSize(1)); - auto b = v.attrs->find(createSymbol("b")); + auto b = v.attrs->get(createSymbol("b")); ASSERT_NE(b, nullptr); ASSERT_THAT(*b->value, IsIntEq(3)); } @@ -225,11 +225,11 @@ namespace nix { auto v = eval("builtins.functionArgs ({ x, y ? 123}: 1)"); ASSERT_THAT(v, IsAttrsOfSize(2)); - auto x = v.attrs->find(createSymbol("x")); + auto x = v.attrs->get(createSymbol("x")); ASSERT_NE(x, nullptr); ASSERT_THAT(*x->value, IsFalse()); - auto y = v.attrs->find(createSymbol("y")); + auto y = v.attrs->get(createSymbol("y")); ASSERT_NE(y, nullptr); ASSERT_THAT(*y->value, IsTrue()); } @@ -238,13 +238,13 @@ namespace nix { auto v = eval("builtins.mapAttrs (name: value: value * 10) { a = 1; b = 2; }"); ASSERT_THAT(v, IsAttrsOfSize(2)); - auto a = v.attrs->find(createSymbol("a")); + auto a = v.attrs->get(createSymbol("a")); ASSERT_NE(a, nullptr); ASSERT_THAT(*a->value, IsThunk()); state.forceValue(*a->value, noPos); ASSERT_THAT(*a->value, IsIntEq(10)); - auto b = v.attrs->find(createSymbol("b")); + auto b = v.attrs->get(createSymbol("b")); ASSERT_NE(b, nullptr); ASSERT_THAT(*b->value, IsThunk()); state.forceValue(*b->value, noPos); @@ -704,11 +704,11 @@ namespace nix { auto v = eval(expr); ASSERT_THAT(v, IsAttrsOfSize(2)); - auto name = v.attrs->find(createSymbol("name")); + auto name = v.attrs->get(createSymbol("name")); ASSERT_TRUE(name); ASSERT_THAT(*name->value, IsStringEq(expectedName)); - auto version = v.attrs->find(createSymbol("version")); + auto version = v.attrs->get(createSymbol("version")); ASSERT_TRUE(version); ASSERT_THAT(*version->value, IsStringEq(expectedVersion)); } diff --git a/tests/unit/libexpr/trivial.cc b/tests/unit/libexpr/trivial.cc index eeb511cdf..99d515273 100644 --- a/tests/unit/libexpr/trivial.cc +++ b/tests/unit/libexpr/trivial.cc @@ -67,11 +67,11 @@ namespace nix { TEST_F(TrivialExpressionTest, updateAttrs) { auto v = eval("{ a = 1; } // { b = 2; a = 3; }"); ASSERT_THAT(v, IsAttrsOfSize(2)); - auto a = v.attrs->find(createSymbol("a")); + auto a = v.attrs->get(createSymbol("a")); ASSERT_NE(a, nullptr); ASSERT_THAT(*a->value, IsIntEq(3)); - auto b = v.attrs->find(createSymbol("b")); + auto b = v.attrs->get(createSymbol("b")); ASSERT_NE(b, nullptr); ASSERT_THAT(*b->value, IsIntEq(2)); } @@ -168,7 +168,7 @@ namespace nix { auto v = eval(expr); ASSERT_THAT(v, IsAttrsOfSize(1)); - auto a = v.attrs->find(createSymbol("a")); + auto a = v.attrs->get(createSymbol("a")); ASSERT_NE(a, nullptr); ASSERT_THAT(*a->value, IsThunk()); @@ -176,11 +176,11 @@ namespace nix { ASSERT_THAT(*a->value, IsAttrsOfSize(2)); - auto b = a->value->attrs->find(createSymbol("b")); + auto b = a->value->attrs->get(createSymbol("b")); ASSERT_NE(b, nullptr); ASSERT_THAT(*b->value, IsIntEq(1)); - auto c = a->value->attrs->find(createSymbol("c")); + auto c = a->value->attrs->get(createSymbol("c")); ASSERT_NE(c, nullptr); ASSERT_THAT(*c->value, IsIntEq(2)); } @@ -202,7 +202,7 @@ namespace nix { TEST_F(TrivialExpressionTest, bindOr) { auto v = eval("{ or = 1; }"); ASSERT_THAT(v, IsAttrsOfSize(1)); - auto b = v.attrs->find(createSymbol("or")); + auto b = v.attrs->get(createSymbol("or")); ASSERT_NE(b, nullptr); ASSERT_THAT(*b->value, IsIntEq(1)); }