Remove EvalState::mkAttrs()

6 files changed, 52 insertions, 42 deletions

diff --git a/src/libexpr/attr-set.cc b/src/libexpr/attr-set.cc
index fc971957d..52ac47e9b 100644
--- a/src/libexpr/attr-set.cc
+++ b/src/libexpr/attr-set.cc
@@ -7,6 +7,7 @@
 namespace nix {
 
 
+
 /* Allocate a new array of attributes for an attribute set with a specific
    capacity. The space is implicitly reserved after the Bindings
    structure. */
@@ -16,15 +17,9 @@ Bindings * EvalState::allocBindings(size_t capacity)
         return &emptyBindings;
     if (capacity > std::numeric_limits<Bindings::size_t>::max())
         throw Error("attribute set of size %d is too big", capacity);
-    return new (allocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings((Bindings::size_t) capacity);
-}
-
-
-void EvalState::mkAttrs(Value & v, size_t capacity)
-{
-    v.mkAttrs(allocBindings(capacity));
     nrAttrsets++;
     nrAttrsInAttrsets += capacity;
+    return new (allocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings((Bindings::size_t) capacity);
 }
 
 
@@ -67,9 +62,7 @@ void Bindings::sort()
 
 Value & Value::mkAttrs(BindingsBuilder & bindings)
 {
-    clearValue();
-    internalType = tAttrs;
-    attrs = bindings.finish();
+    mkAttrs(bindings.finish());
     return *this;
 }
 
diff --git a/src/libexpr/attr-set.hh b/src/libexpr/attr-set.hh
index 903289b69..82c348287 100644
--- a/src/libexpr/attr-set.hh
+++ b/src/libexpr/attr-set.hh
@@ -118,13 +118,14 @@ public:
    call finish(), which sorts the bindings. */
 class BindingsBuilder
 {
-    EvalState & state;
     Bindings * bindings;
 
 public:
 
+    EvalState & state;
+
     BindingsBuilder(EvalState & state, Bindings * bindings)
-        : state(state), bindings(bindings)
+        : bindings(bindings), state(state)
     { }
 
     void insert(Symbol name, Value * value, ptr<Pos> pos = ptr(&noPos))
@@ -146,6 +147,11 @@ public:
         bindings->sort();
         return bindings;
     }
+
+    Bindings * alreadySorted()
+    {
+        return bindings;
+    }
 };
 
 }
diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 112690189..81aa86641 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -145,7 +145,7 @@ void printValue(std::ostream & str, std::set<const Value *> & active, const Valu
         str << v.fpoint;
         break;
     default:
-        throw Error("invalid value");
+        abort();
     }
 
     active.erase(&v);
@@ -1065,8 +1065,8 @@ void ExprPath::eval(EvalState & state, Env & env, Value & v)
 
 void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
 {
-    state.mkAttrs(v, attrs.size() + dynamicAttrs.size());
-    Env *dynamicEnv = &env;
+    v.mkAttrs(state.buildBindings(attrs.size() + dynamicAttrs.size()).finish());
+    auto dynamicEnv = &env;
 
     if (recursive) {
         /* Create a new environment that contains the attributes in
@@ -1592,7 +1592,7 @@ void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v)
     if (v1.attrs->size() == 0) { v = v2; return; }
     if (v2.attrs->size() == 0) { v = v1; return; }
 
-    state.mkAttrs(v, v1.attrs->size() + v2.attrs->size());
+    auto attrs = state.buildBindings(v1.attrs->size() + v2.attrs->size());
 
     /* Merge the sets, preferring values from the second set.  Make
        sure to keep the resulting vector in sorted order. */
@@ -1601,17 +1601,19 @@ void ExprOpUpdate::eval(EvalState & state, Env & env, Value & v)
 
     while (i != v1.attrs->end() && j != v2.attrs->end()) {
         if (i->name == j->name) {
-            v.attrs->push_back(*j);
+            attrs.insert(*j);
             ++i; ++j;
         }
         else if (i->name < j->name)
-            v.attrs->push_back(*i++);
+            attrs.insert(*i++);
         else
-            v.attrs->push_back(*j++);
+            attrs.insert(*j++);
     }
 
-    while (i != v1.attrs->end()) v.attrs->push_back(*i++);
-    while (j != v2.attrs->end()) v.attrs->push_back(*j++);
+    while (i != v1.attrs->end()) attrs.insert(*i++);
+    while (j != v2.attrs->end()) attrs.insert(*j++);
+
+    v.mkAttrs(attrs.alreadySorted());
 
     state.nrOpUpdateValuesCopied += v.attrs->size();
 }
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index c0d88201a..89814785e 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -347,7 +347,6 @@ public:
     }
 
     void mkList(Value & v, size_t length);
-    void mkAttrs(Value & v, size_t capacity);
     void mkThunk_(Value & v, Expr * expr);
     void mkPos(Value & v, ptr<Pos> pos);
 
@@ -400,6 +399,8 @@ private:
     friend struct ExprSelect;
     friend void prim_getAttr(EvalState & state, const Pos & pos, Value * * args, Value & v);
     friend void prim_match(EvalState & state, const Pos & pos, Value * * args, Value & v);
+
+    friend struct Value;
 };
 
 
diff --git a/src/libexpr/json-to-value.cc b/src/libexpr/json-to-value.cc
index 4dc41729a..88716250c 100644
--- a/src/libexpr/json-to-value.cc
+++ b/src/libexpr/json-to-value.cc
@@ -37,10 +37,10 @@ class JSONSax : nlohmann::json_sax<json> {
         ValueMap attrs;
         std::unique_ptr<JSONState> resolve(EvalState & state) override
         {
-            Value & v = parent->value(state);
-            state.mkAttrs(v, attrs.size());
+            auto attrs2 = state.buildBindings(attrs.size());
             for (auto & i : attrs)
-                v.attrs->push_back(Attr(i.first, i.second));
+                attrs2.insert(i.first, i.second);
+            parent->value(state).mkAttrs(attrs2.alreadySorted());
             return std::move(parent);
         }
         void add() override { v = nullptr; }
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 879dfa0a3..d7382af6b 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -2274,11 +2274,12 @@ static void prim_removeAttrs(EvalState & state, const Pos & pos, Value * * args,
     /* Copy all attributes not in that set.  Note that we don't need
        to sort v.attrs because it's a subset of an already sorted
        vector. */
-    state.mkAttrs(v, args[0]->attrs->size());
+    auto attrs = state.buildBindings(args[0]->attrs->size());
     for (auto & i : *args[0]->attrs) {
         if (!names.count(i.name))
-            v.attrs->push_back(i);
+            attrs.insert(i);
     }
+    v.mkAttrs(attrs.alreadySorted());
 }
 
 static RegisterPrimOp primop_removeAttrs({
@@ -2369,13 +2370,15 @@ static void prim_intersectAttrs(EvalState & state, const Pos & pos, Value * * ar
     state.forceAttrs(*args[0], pos);
     state.forceAttrs(*args[1], pos);
 
-    state.mkAttrs(v, std::min(args[0]->attrs->size(), args[1]->attrs->size()));
+    auto attrs = state.buildBindings(std::min(args[0]->attrs->size(), args[1]->attrs->size()));
 
     for (auto & i : *args[0]->attrs) {
         Bindings::iterator j = args[1]->attrs->find(i.name);
         if (j != args[1]->attrs->end())
-            v.attrs->push_back(*j);
+            attrs.insert(*j);
     }
+
+    v.mkAttrs(attrs.alreadySorted());
 }
 
 static RegisterPrimOp primop_intersectAttrs({
@@ -2429,7 +2432,7 @@ static void prim_functionArgs(EvalState & state, const Pos & pos, Value * * args
 {
     state.forceValue(*args[0], pos);
     if (args[0]->isPrimOpApp() || args[0]->isPrimOp()) {
-        state.mkAttrs(v, 0);
+        v.mkAttrs(&state.emptyBindings);
         return;
     }
     if (!args[0]->isLambda())
@@ -2439,7 +2442,7 @@ static void prim_functionArgs(EvalState & state, const Pos & pos, Value * * args
         });
 
     if (!args[0]->lambda.fun->hasFormals()) {
-        state.mkAttrs(v, 0);
+        v.mkAttrs(&state.emptyBindings);
         return;
     }
 
@@ -2472,15 +2475,17 @@ static void prim_mapAttrs(EvalState & state, const Pos & pos, Value * * args, Va
 {
     state.forceAttrs(*args[1], pos);
 
-    state.mkAttrs(v, args[1]->attrs->size());
+    auto attrs = state.buildBindings(args[1]->attrs->size());
 
     for (auto & i : *args[1]->attrs) {
         Value * vName = state.allocValue();
         Value * vFun2 = state.allocValue();
         vName->mkString(i.name);
         vFun2->mkApp(args[0], vName);
-        state.allocAttr(v, i.name)->mkApp(vFun2, i.value);
+        attrs.alloc(i.name).mkApp(vFun2, i.value);
     }
+
+    v.mkAttrs(attrs.alreadySorted());
 }
 
 static RegisterPrimOp primop_mapAttrs({
@@ -2526,12 +2531,13 @@ static void prim_zipAttrsWith(EvalState & state, const Pos & pos, Value * * args
         }
     }
 
-    state.mkAttrs(v, attrsSeen.size());
+    auto attrs = state.buildBindings(attrsSeen.size());
     for (auto & [sym, elem] : attrsSeen) {
-        Value * list = state.allocAttr(v, sym);
-        state.mkList(*list, elem.first);
-        elem.second = list->listElems();
+        auto & list = attrs.alloc(sym);
+        state.mkList(list, elem.first);
+        elem.second = list.listElems();
     }
+    v.mkAttrs(attrs.alreadySorted());
 
     for (unsigned int n = 0; n < listSize; ++n) {
         Value * vElem = listElems[n];
@@ -3054,14 +3060,16 @@ static void prim_groupBy(EvalState & state, const Pos & pos, Value * * args, Val
         vector->second.push_back(vElem);
     }
 
-    state.mkAttrs(v, attrs.size());
+    auto attrs2 = state.buildBindings(attrs.size());
 
     for (auto & i : attrs) {
-        Value * list = state.allocAttr(v, i.first);
+        auto & list = attrs2.alloc(i.first);
         auto size = i.second.size();
-        state.mkList(*list, size);
-        memcpy(list->listElems(), i.second.data(), sizeof(Value *) * size);
+        state.mkList(list, size);
+        memcpy(list.listElems(), i.second.data(), sizeof(Value *) * size);
     }
+
+    v.mkAttrs(attrs2.alreadySorted());
 }
 
 static RegisterPrimOp primop_groupBy({
@@ -3829,7 +3837,7 @@ void EvalState::createBaseEnv()
     Value v;
 
     /* `builtins' must be first! */
-    mkAttrs(v, 128);
+    v.mkAttrs(buildBindings(128).finish());
     addConstant("builtins", v);
 
     v.mkBool(true);