libexpr: Replace Value::mkAttrs with constructor calls

The pseudo-constructor `Value::mkAttrs` would previously be called on an
default-constructed (under-initialized) `Value` to create a properly
initialized `Value` that represents an attribute set. This change
removes mkAttrs and constructs attr `Value`s directly.

Change-Id: I11cd801eefecd454a7a5b6229d6770d06a6a6964

lix/legacy/nix-env.cc

@@ -186,7 +186,7 @@ static void loadSourceExpr(EvalState & state, const SourcePath & path_, Value &
         attrs.alloc("_combineChannels") = Value::EMPTY_LIST;
         StringSet seen;
         getAllExprs(state.ctx, path, seen, attrs);
-        v.mkAttrs(attrs);
+        v = {NewValueAs::attrs, attrs};
     }
 
     else throw Error("path '%s' is not a directory or a Nix expression", path);

lix/legacy/user-env.cc

@@ -65,7 +65,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
             auto outputAttrs = state.ctx.buildBindings(2);
             outputAttrs.alloc(state.ctx.symbols.sym_outPath)
                 .mkString(state.ctx.store->printStorePath(*j.second));
-            attrs.alloc(j.first).mkAttrs(outputAttrs);
+            attrs.alloc(j.first) = {NewValueAs::attrs, outputAttrs};
 
             /* This is only necessary when installing store paths, e.g.,
                `nix-env -i /nix/store/abcd...-foo'. */
@@ -83,9 +83,9 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
             meta.insert(state.ctx.symbols.create(j), *v);
         }
 
-        attrs.alloc(state.ctx.symbols.sym_meta).mkAttrs(meta);
+        attrs.alloc(state.ctx.symbols.sym_meta) = {NewValueAs::attrs, meta};
 
-        manifest->elems[n++].mkAttrs(attrs);
+        manifest->elems[n++] = {NewValueAs::attrs, attrs};
 
         if (drvPath) references.insert(*drvPath);
     }
@@ -109,8 +109,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
     auto attrs = state.ctx.buildBindings(3);
     state.ctx.paths.mkStorePathString(manifestFile, attrs.alloc("manifest"));
     attrs.insert(state.ctx.symbols.create("derivations"), vManifest);
-    Value args;
-    args.mkAttrs(attrs);
+    Value args = {NewValueAs::attrs, attrs};
 
     Value topLevel{NewValueAs::app, state.ctx.mem, envBuilder, args};
 

lix/libcmd/repl.cc

@@ -1504,9 +1504,7 @@ Value NixRepl::replInitInfo()
     currentSystem.mkString(evalSettings.getCurrentSystem());
     builder.insert(evaluator.symbols.create("currentSystem"), currentSystem);
 
-    Value info;
-    info.mkAttrs(builder.finish());
-    return info;
+    return {NewValueAs::attrs, builder.finish()};
 }
 
 
@@ -1574,9 +1572,7 @@ Value NixRepl::bindingsToAttrs()
         builder.insert(symbol, env->values[displacement]);
     }
 
-    Value attrs;
-    attrs.mkAttrs(builder.finish());
-    return attrs;
+    return {NewValueAs::attrs, builder.finish()};
 }
 
 

lix/libexpr/attr-set.cc

@@ -41,13 +41,4 @@ void Bindings::sort()
 {
     if (size_) std::sort(begin(), end());
 }
-
-
-Value & Value::mkAttrs(BindingsBuilder & bindings)
-{
-    mkAttrs(bindings.finish());
-    return *this;
-}
-
-
 }

lix/libexpr/eval-expr.cc

@@ -80,7 +80,7 @@ Env * ExprAttrs::buildInheritFromEnv(EvalState & state, Env & up)
 void ExprSet::eval(EvalState & state, Env & env, Value & v)
 {
     Bindings::Size capacity = attrs.size() + dynamicAttrs.size();
-    v.mkAttrs(state.ctx.buildBindings(capacity).finish());
+    v = {NewValueAs::attrs, state.ctx.buildBindings(capacity).finish()};
     auto dynamicEnv = &env;
 
     if (recursive) {
@@ -141,7 +141,7 @@ void ExprSet::eval(EvalState & state, Env & env, Value & v)
                 }
             }
             newBnds->sort();
-            v.mkAttrs(newBnds);
+            v = {NewValueAs::attrs, newBnds};
         }
     }
 
@@ -383,7 +383,7 @@ void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v)
         attrs.insert(*j++);
     }
 
-    v.mkAttrs(attrs.alreadySorted());
+    v = {NewValueAs::attrs, attrs.alreadySorted()};
 
     state.ctx.stats.nrOpUpdateValuesCopied += v.attrs()->size();
 }

lix/libexpr/eval.cc

@@ -860,7 +860,7 @@ void EvalState::mkPos(Value & v, PosIdx p)
         auto attrs = ctx.buildBindings(3);
         attrs.alloc(ctx.symbols.sym_file).mkString(path->to_string());
         makePositionThunks(*this, p, attrs.alloc(ctx.symbols.sym_line), attrs.alloc(ctx.symbols.sym_column));
-        v.mkAttrs(attrs);
+        v = {NewValueAs::attrs, attrs};
     } else
         v.mkNull();
 }

lix/libexpr/flake/flake.cc

@@ -1016,7 +1016,7 @@ void prim_parseFlakeRef(
             [&vv](const Explicit<bool> & value) { vv.mkBool(value.t); }
         }, value);
     }
-    v.mkAttrs(binds);
+    v = {NewValueAs::attrs, binds};
 }
 
 void prim_flakeRefToString(

lix/libexpr/json-to-value.cc

@@ -43,7 +43,7 @@ class JSONSax : nlohmann::json_sax<JSON> {
             auto attrs2 = state.ctx.buildBindings(attrs.size());
             for (auto & i : attrs)
                 attrs2.insert(i.first, i.second);
-            parent->value().mkAttrs(attrs2.alreadySorted());
+            parent->value() = {NewValueAs::attrs, attrs2.alreadySorted()};
             return std::move(parent);
         }
         void add() override

lix/libexpr/primops.cc

@@ -731,7 +731,7 @@ static void prim_tryEval(EvalState & state, Value * * args, Value & v)
         attrs.alloc(state.ctx.symbols.sym_value).mkBool(false);
     attrs.alloc("success").mkBool(success);
 
-    v.mkAttrs(attrs);
+    v = {NewValueAs::attrs, attrs};
 }
 
 /* Return an environment variable.  Use with care. */
@@ -1266,7 +1266,7 @@ drvName, Bindings * attrs, Value & v)
     for (auto & i : drv.outputs)
         mkOutputString(state, result, drvPath, i);
 
-    v.mkAttrs(result);
+    v = {NewValueAs::attrs, result};
 }
 
 /* Return a placeholder string for the specified output that will be
@@ -1548,7 +1548,7 @@ static void prim_readDir(EvalState & state, Value * * args, Value & v)
         }
     }
 
-    v.mkAttrs(attrs);
+    v = {NewValueAs::attrs, attrs};
 }
 
 /*************************************************************
@@ -1962,7 +1962,7 @@ static void prim_removeAttrs(EvalState & state, Value * * args, Value & v)
         args[0]->attrs()->begin(), args[0]->attrs()->end(),
         names.begin(), names.end(),
         std::back_inserter(attrs));
-    v.mkAttrs(attrs.alreadySorted());
+    v = {NewValueAs::attrs, attrs.alreadySorted()};
 }
 
 /* Builds a set from a list specifying (name, value) pairs.  To be
@@ -2000,7 +2000,7 @@ static void prim_listToAttrs(EvalState & state, Value * * args, Value & v)
         }
     }
 
-    v.mkAttrs(attrs);
+    v = {NewValueAs::attrs, attrs};
 }
 
 static void prim_intersectAttrs(EvalState & state, Value * * args, Value & v)
@@ -2068,7 +2068,7 @@ static void prim_intersectAttrs(EvalState & state, Value * * args, Value & v)
         }
     }
 
-    v.mkAttrs(attrs.alreadySorted());
+    v = {NewValueAs::attrs, attrs.alreadySorted()};
 }
 
 static void prim_catAttrs(EvalState & state, Value * * args, Value & v)
@@ -2102,7 +2102,7 @@ static void prim_functionArgs(EvalState & state, Value * * args, Value & v)
 {
     state.forceValue(*args[0], noPos);
     if (args[0]->isPrimOpApp() || args[0]->isPrimOp()) {
-        v.mkAttrs(&Bindings::EMPTY);
+        v = {NewValueAs::attrs, &Bindings::EMPTY};
         return;
     }
     if (!args[0]->isLambda())
@@ -2110,7 +2110,7 @@ static void prim_functionArgs(EvalState & state, Value * * args, Value & v)
 
     AttrsPattern * formals = dynamic_cast<AttrsPattern *>(args[0]->lambda().fun->pattern.get());
     if (!formals) {
-        v.mkAttrs(&Bindings::EMPTY);
+        v = {NewValueAs::attrs, &Bindings::EMPTY};
         return;
     }
 
@@ -2118,7 +2118,7 @@ static void prim_functionArgs(EvalState & state, Value * * args, Value & v)
     for (auto & i : formals->formals)
         // !!! should optimise booleans (allocate only once)
         attrs.alloc(i.name, i.pos).mkBool(i.def != nullptr);
-    v.mkAttrs(attrs);
+    v = {NewValueAs::attrs, attrs};
 }
 
 /*  */
@@ -2134,7 +2134,7 @@ static void prim_mapAttrs(EvalState & state, Value * * args, Value & v)
         attrs.alloc(i.name) = {NewValueAs::app, state.ctx.mem, *args[0], appArgs};
     }
 
-    v.mkAttrs(attrs.alreadySorted());
+    v = {NewValueAs::attrs, attrs.alreadySorted()};
 }
 
 static void prim_zipAttrsWith(EvalState & state, Value * * args, Value & v)
@@ -2190,7 +2190,7 @@ static void prim_zipAttrsWith(EvalState & state, Value * * args, Value & v)
         }
     }
 
-    v.mkAttrs(attrs.alreadySorted());
+    v = {NewValueAs::attrs, attrs.alreadySorted()};
 }
 
 
@@ -2503,7 +2503,7 @@ static void prim_partition(EvalState & state, Value * * args, Value & v)
         wlist->elems[i] = elems[idx];
     }
 
-    v.mkAttrs(attrs);
+    v = {NewValueAs::attrs, attrs};
 }
 
 static void prim_groupBy(EvalState & state, Value * * args, Value & v)
@@ -2536,7 +2536,7 @@ static void prim_groupBy(EvalState & state, Value * * args, Value & v)
         }
     }
 
-    v.mkAttrs(attrs2.alreadySorted());
+    v = {NewValueAs::attrs, attrs2.alreadySorted()};
 }
 
 static void prim_concatMap(EvalState & state, Value * * args, Value & v)
@@ -3005,7 +3005,7 @@ static void prim_parseDrvName(EvalState & state, Value * * args, Value & v)
     auto attrs = state.ctx.buildBindings(2);
     attrs.alloc(state.ctx.symbols.sym_name).mkString(parsed.name);
     attrs.alloc("version").mkString(parsed.version);
-    v.mkAttrs(attrs);
+    v = {NewValueAs::attrs, attrs};
 }
 
 static void prim_compareVersions(EvalState & state, Value * * args, Value & v)
@@ -3055,7 +3055,7 @@ Value EvalBuiltins::prepareNixPath(const SearchPath & searchPath)
         auto attrs = mem.buildBindings(symbols, 2);
         attrs.alloc("path").mkString(i.path.s);
         attrs.alloc("prefix").mkString(i.prefix.s);
-        v->elems[n++].mkAttrs(attrs);
+        v->elems[n++] = {NewValueAs::attrs, attrs};
     }
     return {NewValueAs::list, v};
 }

lix/libexpr/primops/context.cc

@@ -146,10 +146,10 @@ void prim_getContext(EvalState & state, Value * * args, Value & v)
             for (const auto & [i, output] : enumerate(info.second.outputs))
                 content->elems[i].mkString(output);
         }
-        attrs.alloc(state.ctx.store->printStorePath(info.first)).mkAttrs(infoAttrs);
+        attrs.alloc(state.ctx.store->printStorePath(info.first)) = {NewValueAs::attrs, infoAttrs};
     }
 
-    v.mkAttrs(attrs);
+    v = {NewValueAs::attrs, attrs};
 }
 
 

lix/libexpr/primops/fetchMercurial.cc

@@ -98,7 +98,7 @@ void prim_fetchMercurial(EvalState & state, Value ** args, Value & v)
     attrs2.alloc("shortRev").mkString(rev2.gitRev().substr(0, 12));
     if (auto revCount = input2.getRevCount())
         attrs2.alloc("revCount").mkInt(*revCount);
-    v.mkAttrs(attrs2);
+    v = {NewValueAs::attrs, attrs2};
 
     state.ctx.paths.allowPath(tree.storePath);
 }

lix/libexpr/primops/fetchTree.cc

@@ -68,7 +68,7 @@ void emitTreeAttrs(
             fmt("%s", std::put_time(std::gmtime(&*lastModified), "%Y%m%d%H%M%S")));
     }
 
-    v.mkAttrs(attrs);
+    v = {NewValueAs::attrs, attrs};
 }
 
 std::string fixURI(std::string uri, EvalState & state, const std::string & defaultScheme = "file")

lix/libexpr/primops/fromTOML.cc

@@ -25,7 +25,7 @@ void prim_fromTOML(EvalState & state, Value ** args, Value & val)
                 self(attrs.alloc(elem.first), elem.second);
             }
 
-            v.mkAttrs(attrs);
+            v = {NewValueAs::attrs, attrs};
         } break;
         case toml::value_t::array: {
             auto array = toml::get<std::vector<toml::value>>(t);

lix/libexpr/value.cc

@@ -38,6 +38,8 @@ static void copyContextToValue(Value::String & s, const NixStringContext & conte
     }
 }
 
+Value::Value(attrs_t, BindingsBuilder & bindings) : Value(NewValueAs::attrs, bindings.finish()) {}
+
 Value::Value(primop_t, PrimOp & primop) : raw(tag(tAuxiliary, &primop)) {}
 
 void Value::print(EvalState & state, std::ostream & str, PrintOptions options)

lix/libexpr/value.hh

@@ -524,6 +524,7 @@ public:
     /// The bindings are not not copied; this constructor assumes @ref bindings
     /// has already been suitably allocated by something like nix::buildBindings.
     Value(attrs_t, Bindings * bindings) : raw(tag(tAttrs, bindings)) {}
+    Value(attrs_t, BindingsBuilder & bindings);
 
     /// Constructs a nix language lazy delayed computation, or "thunk".
     ///
@@ -787,13 +788,6 @@ public:
         *this = {NewValueAs::null};
     }
 
-    inline void mkAttrs(Bindings * a)
-    {
-        raw = tag(tAttrs, a);
-    }
-
-    Value & mkAttrs(BindingsBuilder & bindings);
-
     void mkPrimOp(PrimOp * p);
 
     inline void mkExternal(ExternalValueBase * e)

tests/unit/libexpr/value/print.cc

@@ -69,8 +69,7 @@ TEST_F(ValuePrintingTests, tAttrs)
     builder.insert(evaluator.symbols.create("one"), vOne);
     builder.insert(evaluator.symbols.create("two"), vTwo);
 
-    Value vAttrs;
-    vAttrs.mkAttrs(builder.finish());
+    Value vAttrs = {NewValueAs::attrs, builder.finish()};
 
     test(vAttrs, "{ one = 1; two = 2; }");
 }
@@ -206,13 +205,11 @@ TEST_F(ValuePrintingTests, depthAttrs)
     vTwo.mkInt(2);
 
     BindingsBuilder builderEmpty = evaluator.buildBindings(0);
-    Value vAttrsEmpty;
-    vAttrsEmpty.mkAttrs(builderEmpty.finish());
+    Value vAttrsEmpty = {NewValueAs::attrs, builderEmpty.finish()};
 
     BindingsBuilder builderNested = evaluator.buildBindings(1);
     builderNested.insert(evaluator.symbols.create("zero"), vZero);
-    Value vAttrsNested;
-    vAttrsNested.mkAttrs(builderNested.finish());
+    Value vAttrsNested = {NewValueAs::attrs, builderNested.finish()};
 
     BindingsBuilder builder = evaluator.buildBindings(10);
     builder.insert(evaluator.symbols.create("one"), vOne);
@@ -220,16 +217,14 @@ TEST_F(ValuePrintingTests, depthAttrs)
     builder.insert(evaluator.symbols.create("empty"), vAttrsEmpty);
     builder.insert(evaluator.symbols.create("nested"), vAttrsNested);
 
-    Value vAttrs;
-    vAttrs.mkAttrs(builder.finish());
+    Value vAttrs = {NewValueAs::attrs, builder.finish()};
 
     BindingsBuilder builder2 = evaluator.buildBindings(10);
     builder2.insert(evaluator.symbols.create("one"), vOne);
     builder2.insert(evaluator.symbols.create("two"), vTwo);
     builder2.insert(evaluator.symbols.create("nested"), vAttrs);
 
-    Value vNested;
-    vNested.mkAttrs(builder2.finish());
+    Value vNested = {NewValueAs::attrs, builder2.finish()};
 
     test(vNested, "{ nested = { ... }; one = 1; two = 2; }", PrintOptions { .maxDepth = 1 });
     test(vNested, "{ nested = { empty = { }; nested = { ... }; one = 1; two = 2; }; one = 1; two = 2; }", PrintOptions { .maxDepth = 2 });
@@ -249,16 +244,14 @@ TEST_F(ValuePrintingTests, depthList)
     builder.insert(evaluator.symbols.create("one"), vOne);
     builder.insert(evaluator.symbols.create("two"), vTwo);
 
-    Value vAttrs;
-    vAttrs.mkAttrs(builder.finish());
+    Value vAttrs = {NewValueAs::attrs, builder.finish()};
 
     BindingsBuilder builder2 = evaluator.buildBindings(10);
     builder2.insert(evaluator.symbols.create("one"), vOne);
     builder2.insert(evaluator.symbols.create("two"), vTwo);
     builder2.insert(evaluator.symbols.create("nested"), vAttrs);
 
-    Value vNested;
-    vNested.mkAttrs(builder2.finish());
+    Value vNested = {NewValueAs::attrs, builder2.finish()};
 
     auto list = evaluator.mem.newList(5);
     list->elems[0] = vOne;
@@ -313,8 +306,7 @@ TEST_F(ValuePrintingTests, attrsTypeFirst)
     builder.insert(evaluator.symbols.create("type"), vType);
     builder.insert(evaluator.symbols.create("apple"), vApple);
 
-    Value vAttrs;
-    vAttrs.mkAttrs(builder.finish());
+    Value vAttrs = {NewValueAs::attrs, builder.finish()};
 
     test(vAttrs,
          "{ type = \"puppy\"; apple = \"apple\"; }",
@@ -424,8 +416,7 @@ TEST_F(ValuePrintingTests, ansiColorsAttrs)
     builder.insert(evaluator.symbols.create("one"), vOne);
     builder.insert(evaluator.symbols.create("two"), vTwo);
 
-    Value vAttrs;
-    vAttrs.mkAttrs(builder.finish());
+    Value vAttrs = {NewValueAs::attrs, builder.finish()};
 
     test(vAttrs,
          "{ one = " ANSI_CYAN "1" ANSI_NORMAL "; two = " ANSI_CYAN "2" ANSI_NORMAL "; }",
@@ -442,8 +433,7 @@ TEST_F(ValuePrintingTests, ansiColorsDerivation)
     BindingsBuilder builder = evaluator.buildBindings(10);
     builder.insert(evaluator.symbols.sym_type, vDerivation);
 
-    Value vAttrs;
-    vAttrs.mkAttrs(builder.finish());
+    Value vAttrs = {NewValueAs::attrs, builder.finish()};
 
     test(vAttrs,
          ANSI_GREEN "«derivation»" ANSI_NORMAL,
@@ -642,15 +632,13 @@ TEST_F(ValuePrintingTests, ansiColorsAttrsRepeated)
     BindingsBuilder innerBuilder = evaluator.buildBindings(1);
     innerBuilder.insert(evaluator.symbols.create("x"), vZero);
 
-    Value vInner;
-    vInner.mkAttrs(innerBuilder.finish());
+    Value vInner = {NewValueAs::attrs, innerBuilder.finish()};
 
     BindingsBuilder builder = evaluator.buildBindings(10);
     builder.insert(evaluator.symbols.create("a"), vInner);
     builder.insert(evaluator.symbols.create("b"), vInner);
 
-    Value vAttrs;
-    vAttrs.mkAttrs(builder.finish());
+    Value vAttrs = {NewValueAs::attrs, builder.finish()};
 
     test(vAttrs,
          "{ a = { x = " ANSI_CYAN "0" ANSI_NORMAL "; }; b = " ANSI_MAGENTA "«repeated»" ANSI_NORMAL "; }",
@@ -667,8 +655,7 @@ TEST_F(ValuePrintingTests, ansiColorsListRepeated)
     BindingsBuilder innerBuilder = evaluator.buildBindings(1);
     innerBuilder.insert(evaluator.symbols.create("x"), vZero);
 
-    Value vInner;
-    vInner.mkAttrs(innerBuilder.finish());
+    Value vInner = {NewValueAs::attrs, innerBuilder.finish()};
 
     auto vList = evaluator.mem.newList(3);
     vList->elems[0] = vInner;
@@ -690,8 +677,7 @@ TEST_F(ValuePrintingTests, listRepeated)
     BindingsBuilder innerBuilder = evaluator.buildBindings(1);
     innerBuilder.insert(evaluator.symbols.create("x"), vZero);
 
-    Value vInner;
-    vInner.mkAttrs(innerBuilder.finish());
+    Value vInner = {NewValueAs::attrs, innerBuilder.finish()};
 
     auto list = evaluator.mem.newList(3);
     list->elems[0] = vInner;
@@ -719,8 +705,7 @@ TEST_F(ValuePrintingTests, ansiColorsAttrsElided)
     builder.insert(evaluator.symbols.create("one"), vOne);
     builder.insert(evaluator.symbols.create("two"), vTwo);
 
-    Value vAttrs;
-    vAttrs.mkAttrs(builder.finish());
+    Value vAttrs = {NewValueAs::attrs, builder.finish()};
 
     test(vAttrs,
          "{ one = " ANSI_CYAN "1" ANSI_NORMAL "; " ANSI_FAINT "«1 attribute elided»" ANSI_NORMAL " }",
@@ -733,7 +718,7 @@ TEST_F(ValuePrintingTests, ansiColorsAttrsElided)
     vThree.mkInt(3);
 
     builder.insert(evaluator.symbols.create("three"), vThree);
-    vAttrs.mkAttrs(builder.finish());
+    vAttrs = {NewValueAs::attrs, builder.finish()};
 
     test(vAttrs,
          "{ one = " ANSI_CYAN "1" ANSI_NORMAL "; " ANSI_FAINT "«2 attributes elided»" ANSI_NORMAL " }",