Merge pull request #13936 from xokdvium/empty-list-bindings

libexpr: Make constant Values global constants, move out of EvalState

src/libexpr/attr-path.cc

@@ -110,8 +110,8 @@ std::pair<SourcePath, uint32_t> findPackageFilename(EvalState & state, Value & v
 {
     Value * v2;
     try {
-        auto dummyArgs = state.allocBindings(0);
-        v2 = findAlongAttrPath(state, "meta.position", *dummyArgs, v).first;
+        auto & dummyArgs = Bindings::emptyBindings;
+        v2 = findAlongAttrPath(state, "meta.position", dummyArgs, v).first;
     } catch (Error &) {
         throw NoPositionInfo("package '%s' has no source location information", what);
     }

src/libexpr/attr-set.cc

@@ -5,13 +5,15 @@
 
 namespace nix {
 
+Bindings Bindings::emptyBindings;
+
 /* Allocate a new array of attributes for an attribute set with a specific
    capacity. The space is implicitly reserved after the Bindings
    structure. */
 Bindings * EvalState::allocBindings(size_t capacity)
 {
     if (capacity == 0)
-        return &emptyBindings;
+        return &Bindings::emptyBindings;
     if (capacity > std::numeric_limits<Bindings::size_t>::max())
         throw Error("attribute set of size %d is too big", capacity);
     nrAttrsets++;

src/libexpr/eval.cc

@@ -202,7 +202,6 @@ EvalState::EvalState(
     , settings{settings}
     , symbols(StaticEvalSymbols::staticSymbolTable())
     , repair(NoRepair)
-    , emptyBindings(Bindings())
     , storeFS(makeMountedSourceAccessor({
           {CanonPath::root, makeEmptySourceAccessor()},
           /* In the pure eval case, we can simply require
@@ -285,10 +284,6 @@ EvalState::EvalState(
 
     static_assert(sizeof(Env) <= 16, "environment must be <= 16 bytes");
 
-    vEmptyList.mkList(buildList(0));
-    vNull.mkNull();
-    vTrue.mkBool(true);
-    vFalse.mkBool(false);
     vStringRegular.mkStringNoCopy("regular");
     vStringDirectory.mkStringNoCopy("directory");
     vStringSymlink.mkStringNoCopy("symlink");
@@ -895,7 +890,7 @@ ListBuilder::ListBuilder(EvalState & state, size_t size)
 
 Value * EvalState::getBool(bool b)
 {
-    return b ? &vTrue : &vFalse;
+    return b ? &Value::vTrue : &Value::vFalse;
 }
 
 unsigned long nrThunks = 0;
@@ -1301,7 +1296,7 @@ void ExprList::eval(EvalState & state, Env & env, Value & v)
 Value * ExprList::maybeThunk(EvalState & state, Env & env)
 {
     if (elems.empty()) {
-        return &state.vEmptyList;
+        return &Value::vEmptyList;
     }
     return Expr::maybeThunk(state, env);
 }

src/libexpr/include/nix/expr/attr-set.hh

@@ -54,6 +54,12 @@ public:
     typedef uint32_t size_t;
     PosIdx pos;
 
+    /**
+     * An instance of bindings objects with 0 attributes.
+     * This object must never be modified.
+     */
+    static Bindings emptyBindings;
+
 private:
     size_t size_ = 0;
     Attr attrs[0];

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

@@ -313,34 +313,6 @@ public:
      */
     RepairFlag repair;
 
-    Bindings emptyBindings;
-
-    /**
-     * Empty list constant.
-     */
-    Value vEmptyList;
-
-    /**
-     * `null` constant.
-     *
-     * This is _not_ a singleton. Pointer equality is _not_ sufficient.
-     */
-    Value vNull;
-
-    /**
-     * `true` constant.
-     *
-     * This is _not_ a singleton. Pointer equality is _not_ sufficient.
-     */
-    Value vTrue;
-
-    /**
-     * `true` constant.
-     *
-     * This is _not_ a singleton. Pointer equality is _not_ sufficient.
-     */
-    Value vFalse;
-
     /** `"regular"` */
     Value vStringRegular;
     /** `"directory"` */

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

@@ -833,6 +833,35 @@ struct Value : public ValueStorage<sizeof(void *)>
 {
     friend std::string showType(const Value & v);
 
+    /**
+     * Empty list constant.
+     *
+     * This is _not_ a singleton. Pointer equality is _not_ sufficient.
+     */
+    static Value vEmptyList;
+
+    /**
+     * `null` constant.
+     *
+     * This is _not_ a singleton. Pointer equality is _not_ sufficient.
+     */
+    static Value vNull;
+
+    /**
+     * `true` constant.
+     *
+     * This is _not_ a singleton. Pointer equality is _not_ sufficient.
+     */
+    static Value vTrue;
+
+    /**
+     * `true` constant.
+     *
+     * This is _not_ a singleton. Pointer equality is _not_ sufficient.
+     */
+    static Value vFalse;
+
+private:
     template<InternalType... discriminator>
     bool isa() const noexcept
     {

src/libexpr/meson.build

@@ -163,6 +163,7 @@ sources = files(
   'search-path.cc',
   'value-to-json.cc',
   'value-to-xml.cc',
+  'value.cc',
   'value/context.cc',
 )
 

src/libexpr/primops.cc

@@ -1075,11 +1075,11 @@ static void prim_tryEval(EvalState & state, const PosIdx pos, Value ** args, Val
     try {
         state.forceValue(*args[0], pos);
         attrs.insert(state.s.value, args[0]);
-        attrs.insert(state.symbols.create("success"), &state.vTrue);
+        attrs.insert(state.symbols.create("success"), &Value::vTrue);
     } catch (AssertionError & e) {
         // `value = false;` is unfortunate but removing it is a breaking change.
-        attrs.insert(state.s.value, &state.vFalse);
-        attrs.insert(state.symbols.create("success"), &state.vFalse);
+        attrs.insert(state.s.value, &Value::vFalse);
+        attrs.insert(state.symbols.create("success"), &Value::vFalse);
     }
 
     // restore the debugRepl pointer if we saved it earlier.
@@ -3326,14 +3326,14 @@ static void prim_functionArgs(EvalState & state, const PosIdx pos, Value ** args
 {
     state.forceValue(*args[0], pos);
     if (args[0]->isPrimOpApp() || args[0]->isPrimOp()) {
-        v.mkAttrs(&state.emptyBindings);
+        v.mkAttrs(&Bindings::emptyBindings);
         return;
     }
     if (!args[0]->isLambda())
         state.error<TypeError>("'functionArgs' requires a function").atPos(pos).debugThrow();
 
     if (!args[0]->lambda().fun->hasFormals()) {
-        v.mkAttrs(&state.emptyBindings);
+        v.mkAttrs(&Bindings::emptyBindings);
         return;
     }
 
@@ -4613,7 +4613,7 @@ void prim_match(EvalState & state, const PosIdx pos, Value ** args, Value & v)
         auto list = state.buildList(match.size() - 1);
         for (const auto & [i, v2] : enumerate(list))
             if (!match[i + 1].matched)
-                v2 = &state.vNull;
+                v2 = &Value::vNull;
             else
                 v2 = mkString(state, match[i + 1]);
         v.mkList(list);
@@ -4705,7 +4705,7 @@ void prim_split(EvalState & state, const PosIdx pos, Value ** args, Value & v)
             auto list2 = state.buildList(slen);
             for (const auto & [si, v2] : enumerate(list2)) {
                 if (!match[si + 1].matched)
-                    v2 = &state.vNull;
+                    v2 = &Value::vNull;
                 else
                     v2 = mkString(state, match[si + 1]);
             }
@@ -5059,7 +5059,7 @@ void EvalState::createBaseEnv(const EvalSettings & evalSettings)
 
     addConstant(
         "null",
-        &vNull,
+        &Value::vNull,
         {
             .type = nNull,
             .doc = R"(

src/libexpr/value.cc

@@ -0,0 +1,29 @@
+#include "nix/expr/value.hh"
+
+namespace nix {
+
+Value Value::vEmptyList = []() {
+    Value res;
+    res.setStorage(List{.size = 0, .elems = nullptr});
+    return res;
+}();
+
+Value Value::vNull = []() {
+    Value res;
+    res.mkNull();
+    return res;
+}();
+
+Value Value::vTrue = []() {
+    Value res;
+    res.mkBool(true);
+    return res;
+}();
+
+Value Value::vFalse = []() {
+    Value res;
+    res.mkBool(false);
+    return res;
+}();
+
+} // namespace nix

src/nix/flake.cc

@@ -522,7 +522,7 @@ struct CmdFlakeCheck : FlakeCommand
         auto checkNixOSConfiguration = [&](const std::string & attrPath, Value & v, const PosIdx pos) {
             try {
                 Activity act(*logger, lvlInfo, actUnknown, fmt("checking NixOS configuration '%s'", attrPath));
-                Bindings & bindings(*state->allocBindings(0));
+                Bindings & bindings = Bindings::emptyBindings;
                 auto vToplevel = findAlongAttrPath(*state, "config.system.build.toplevel", bindings, v).first;
                 state->forceValue(*vToplevel, pos);
                 if (!state->isDerivation(*vToplevel))

src/nix/nix-env/nix-env.cc

@@ -158,7 +158,7 @@ static void loadSourceExpr(EvalState & state, const SourcePath & path, Value & v
        directory). */
     else if (st.type == SourceAccessor::tDirectory) {
         auto attrs = state.buildBindings(maxAttrs);
-        attrs.insert(state.symbols.create("_combineChannels"), &state.vEmptyList);
+        attrs.insert(state.symbols.create("_combineChannels"), &Value::vEmptyList);
         StringSet seen;
         getAllExprs(state, path, seen, attrs);
         v.mkAttrs(attrs);

src/nix/nix-env/user-env.cc

@@ -24,7 +24,7 @@ PackageInfos queryInstalled(EvalState & state, const Path & userEnv)
     if (pathExists(manifestFile)) {
         Value v;
         state.evalFile(state.rootPath(CanonPath(manifestFile)).resolveSymlinks(), v);
-        Bindings & bindings(*state.allocBindings(0));
+        Bindings & bindings = Bindings::emptyBindings;
         getDerivations(state, v, "", bindings, elems, false);
     }
     return elems;

src/nix/upgrade-nix.cc

@@ -162,7 +162,7 @@ struct CmdUpgradeNix : MixDryRun, StoreCommand
         auto state = std::make_unique<EvalState>(LookupPath{}, store, fetchSettings, evalSettings);
         auto v = state->allocValue();
         state->eval(state->parseExprFromString(res.data, state->rootPath(CanonPath("/no-such-path"))), *v);
-        Bindings & bindings(*state->allocBindings(0));
+        Bindings & bindings = Bindings::emptyBindings;
         auto v2 = findAlongAttrPath(*state, settings.thisSystem, bindings, *v).first;
 
         return store->parseStorePath(