7 files changed, 144 insertions, 116 deletions

diff --git a/src/libexpr/attr-set.cc b/src/libexpr/attr-set.cc
index 17886a426..b6091c955 100644
--- a/src/libexpr/attr-set.cc
+++ b/src/libexpr/attr-set.cc
@@ -24,9 +24,7 @@ void EvalState::mkAttrs(Value & v, size_t capacity)
         v = vEmptySet;
         return;
     }
-    clearValue(v);
-    v.setAttrs();
-    v.attrs = allocBindings(capacity);
+    v.mkAttrs(allocBindings(capacity));
     nrAttrsets++;
     nrAttrsInAttrsets += capacity;
 }
diff --git a/src/libexpr/eval-inline.hh b/src/libexpr/eval-inline.hh
index e56ce261c..f6dead6b0 100644
--- a/src/libexpr/eval-inline.hh
+++ b/src/libexpr/eval-inline.hh
@@ -36,13 +36,11 @@ void EvalState::forceValue(Value & v, const Pos & pos)
         Env * env = v.thunk.env;
         Expr * expr = v.thunk.expr;
         try {
-            v.setBlackhole();
+            v.mkBlackhole();
             //checkInterrupt();
             expr->eval(*this, *env, v);
         } catch (...) {
-            v.setThunk();
-            v.thunk.env = env;
-            v.thunk.expr = expr;
+            v.mkThunk(env, expr);
             throw;
         }
     }
diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 2f8d6d259..5a641d02c 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -430,9 +430,7 @@ EvalState::EvalState(const Strings & _searchPath, ref<Store> store)
         }
     }
 
-    clearValue(vEmptySet);
-    vEmptySet.setAttrs();
-    vEmptySet.attrs = allocBindings(0);
+    vEmptySet.mkAttrs(allocBindings(0));
 
     createBaseEnv();
 }
@@ -548,16 +546,14 @@ Value * EvalState::addPrimOp(const string & name,
        the primop to a dummy value. */
     if (arity == 0) {
         auto vPrimOp = allocValue();
-        vPrimOp->setPrimOp();
-        vPrimOp->primOp = new PrimOp { .fun = primOp, .arity = 1, .name = sym };
+        vPrimOp->mkPrimOp(new PrimOp { .fun = primOp, .arity = 1, .name = sym });
         Value v;
         mkApp(v, *vPrimOp, *vPrimOp);
         return addConstant(name, v);
     }
 
     Value * v = allocValue();
-    v->setPrimOp();
-    v->primOp = new PrimOp { .fun = primOp, .arity = arity, .name = sym };
+    v->mkPrimOp(new PrimOp { .fun = primOp, .arity = arity, .name = sym });
     staticBaseEnv.vars[symbols.create(name)] = baseEnvDispl;
     baseEnv.values[baseEnvDispl++] = v;
     baseEnv.values[0]->attrs->push_back(Attr(sym, v));
@@ -572,8 +568,7 @@ Value * EvalState::addPrimOp(PrimOp && primOp)
     if (primOp.arity == 0) {
         primOp.arity = 1;
         auto vPrimOp = allocValue();
-        vPrimOp->setPrimOp();
-        vPrimOp->primOp = new PrimOp(std::move(primOp));
+        vPrimOp->mkPrimOp(new PrimOp(std::move(primOp)));
         Value v;
         mkApp(v, *vPrimOp, *vPrimOp);
         return addConstant(primOp.name, v);
@@ -584,8 +579,7 @@ Value * EvalState::addPrimOp(PrimOp && primOp)
         primOp.name = symbols.create(std::string(primOp.name, 2));
 
     Value * v = allocValue();
-    v->setPrimOp();
-    v->primOp = new PrimOp(std::move(primOp));
+    v->mkPrimOp(new PrimOp(std::move(primOp)));
     staticBaseEnv.vars[envName] = baseEnvDispl;
     baseEnv.values[baseEnvDispl++] = v;
     baseEnv.values[0]->attrs->push_back(Attr(primOp.name, v));
@@ -708,15 +702,13 @@ LocalNoInline(void addErrorTrace(Error & e, const Pos & pos, const char * s, con
 
 void mkString(Value & v, const char * s)
 {
-    mkStringNoCopy(v, dupString(s));
+    v.mkString(dupString(s));
 }
 
 
 Value & mkString(Value & v, std::string_view s, const PathSet & context)
 {
-    v.setString();
-    v.string.s = dupStringWithLen(s.data(), s.size());
-    v.string.context = 0;
+    v.mkString(dupStringWithLen(s.data(), s.size()));
     if (!context.empty()) {
         size_t n = 0;
         v.string.context = (const char * *)
@@ -731,7 +723,7 @@ Value & mkString(Value & v, std::string_view s, const PathSet & context)
 
 void mkPath(Value & v, const char * s)
 {
-    mkPathNoCopy(v, dupString(s));
+    v.mkPath(dupString(s));
 }
 
 
@@ -792,16 +784,9 @@ Env & EvalState::allocEnv(size_t size)
 
 void EvalState::mkList(Value & v, size_t size)
 {
-    clearValue(v);
-    if (size == 1)
-        v.setList1();
-    else if (size == 2)
-        v.setList2();
-    else {
-        v.setListN();
-        v.bigList.size = size;
-        v.bigList.elems = size ? (Value * *) allocBytes(size * sizeof(Value *)) : 0;
-    }
+    v.mkList(size);
+    if (size > 2)
+        v.bigList.elems = (Value * *) allocBytes(size * sizeof(Value *));
     nrListElems += size;
 }
 
@@ -810,9 +795,7 @@ unsigned long nrThunks = 0;
 
 static inline void mkThunk(Value & v, Env & env, Expr * expr)
 {
-    v.setThunk();
-    v.thunk.env = &env;
-    v.thunk.expr = expr;
+    v.mkThunk(&env, expr);
     nrThunks++;
 }
 
@@ -1207,9 +1190,7 @@ void ExprOpHasAttr::eval(EvalState & state, Env & env, Value & v)
 
 void ExprLambda::eval(EvalState & state, Env & env, Value & v)
 {
-    v.setLambda();
-    v.lambda.env = &env;
-    v.lambda.fun = this;
+    v.mkLambda(&env, this);
 }
 
 
@@ -1252,9 +1233,7 @@ void EvalState::callPrimOp(Value & fun, Value & arg, Value & v, const Pos & pos)
     } else {
         Value * fun2 = allocValue();
         *fun2 = fun;
-        v.setPrimOpApp();
-        v.primOpApp.left = fun2;
-        v.primOpApp.right = &arg;
+        v.mkPrimOpApp(fun2, &arg);
     }
 }
 
diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh
index e4cbc660f..b80a7de4e 100644
--- a/src/libexpr/nixexpr.hh
+++ b/src/libexpr/nixexpr.hh
@@ -129,7 +129,7 @@ struct ExprPath : Expr
 {
     string s;
     Value v;
-    ExprPath(const string & s) : s(s) { mkPathNoCopy(v, this->s.c_str()); };
+    ExprPath(const string & s) : s(s) { v.mkPath(this->s.c_str()); };
     COMMON_METHODS
     Value * maybeThunk(EvalState & state, Env & env);
 };
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 4106f1ec8..45066e9cf 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -1434,7 +1434,7 @@ static void prim_readDir(EvalState & state, const Pos & pos, Value * * args, Val
         Value * ent_val = state.allocAttr(v, state.symbols.create(ent.name));
         if (ent.type == DT_UNKNOWN)
             ent.type = getFileType(path + "/" + ent.name);
-        mkStringNoCopy(*ent_val,
+        ent_val->mkString(
             ent.type == DT_REG ? "regular" :
             ent.type == DT_DIR ? "directory" :
             ent.type == DT_LNK ? "symlink" :
diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh
index 61ea1d64b..b317c1898 100644
--- a/src/libexpr/value.hh
+++ b/src/libexpr/value.hh
@@ -114,24 +114,6 @@ friend void printValue(std::ostream & str, std::set<const Value *> & active, con
 
 public:
 
-    inline void setInt() { internalType = tInt; };
-    inline void setBool() { internalType = tBool; };
-    inline void setString() { internalType = tString; };
-    inline void setPath() { internalType = tPath; };
-    inline void setNull() { internalType = tNull; };
-    inline void setAttrs() { internalType = tAttrs; };
-    inline void setList1() { internalType = tList1; };
-    inline void setList2() { internalType = tList2; };
-    inline void setListN() { internalType = tListN; };
-    inline void setThunk() { internalType = tThunk; };
-    inline void setApp() { internalType = tApp; };
-    inline void setLambda() { internalType = tLambda; };
-    inline void setBlackhole() { internalType = tBlackhole; };
-    inline void setPrimOp() { internalType = tPrimOp; };
-    inline void setPrimOpApp() { internalType = tPrimOpApp; };
-    inline void setExternal() { internalType = tExternal; };
-    inline void setFloat() { internalType = tFloat; };
-
     // Functions needed to distinguish the type
     // These should be removed eventually, by putting the functionality that's
     // needed by callers into methods of this type
@@ -222,6 +204,123 @@ public:
         abort();
     }
 
+    /* After overwriting an app node, be sure to clear pointers in the
+       Value to ensure that the target isn't kept alive unnecessarily. */
+    inline void clearValue()
+    {
+        app.left = app.right = 0;
+    }
+
+    inline void mkInt(NixInt n)
+    {
+        clearValue();
+        internalType = tInt;
+        integer = n;
+    }
+
+    inline void mkBool(bool b)
+    {
+        clearValue();
+        internalType = tBool;
+        boolean = b;
+    }
+
+    inline void mkString(const char * s, const char * * context = 0)
+    {
+        internalType = tString;
+        string.s = s;
+        string.context = context;
+    }
+
+    inline void mkPath(const char * s)
+    {
+        clearValue();
+        internalType = tPath;
+        path = s;
+    }
+
+    inline void mkNull()
+    {
+        clearValue();
+        internalType = tNull;
+    }
+
+    inline void mkAttrs(Bindings * a)
+    {
+        clearValue();
+        internalType = tAttrs;
+        attrs = a;
+    }
+
+    inline void mkList(size_t size)
+    {
+        clearValue();
+        if (size == 1)
+            internalType = tList1;
+        else if (size == 2)
+            internalType = tList2;
+        else {
+            internalType = tListN;
+            bigList.size = size;
+        }
+    }
+
+    inline void mkThunk(Env * e, Expr * ex)
+    {
+        internalType = tThunk;
+        thunk.env = e;
+        thunk.expr = ex;
+    }
+
+    inline void mkApp(Value * l, Value * r)
+    {
+        internalType = tApp;
+        app.left = l;
+        app.right = r;
+    }
+
+    inline void mkLambda(Env * e, ExprLambda * f)
+    {
+        internalType = tLambda;
+        lambda.env = e;
+        lambda.fun = f;
+    }
+
+    inline void mkBlackhole()
+    {
+        internalType = tBlackhole;
+        // Value will be overridden anyways
+    }
+
+    inline void mkPrimOp(PrimOp * p)
+    {
+        clearValue();
+        internalType = tPrimOp;
+        primOp = p;
+    }
+
+
+    inline void mkPrimOpApp(Value * l, Value * r)
+    {
+        internalType = tPrimOpApp;
+        app.left = l;
+        app.right = r;
+    }
+
+    inline void mkExternal(ExternalValueBase * e)
+    {
+        clearValue();
+        internalType = tExternal;
+        external = e;
+    }
+
+    inline void mkFloat(NixFloat n)
+    {
+        clearValue();
+        internalType = tFloat;
+        fpoint = n;
+    }
+
     bool isList() const
     {
         return internalType == tList1 || internalType == tList2 || internalType == tListN;
@@ -251,86 +350,42 @@ public:
 };
 
 
-/* After overwriting an app node, be sure to clear pointers in the
-   Value to ensure that the target isn't kept alive unnecessarily. */
-static inline void clearValue(Value & v)
-{
-    v.app.left = v.app.right = 0;
-}
-
 
+// TODO: Remove these static functions, replace call sites with v.mk* instead
 static inline void mkInt(Value & v, NixInt n)
 {
-    clearValue(v);
-    v.setInt();
-    v.integer = n;
+    v.mkInt(n);
 }
 
-
 static inline void mkFloat(Value & v, NixFloat n)
 {
-    clearValue(v);
-    v.setFloat();
-    v.fpoint = n;
+    v.mkFloat(n);
 }
 
-
 static inline void mkBool(Value & v, bool b)
 {
-    clearValue(v);
-    v.setBool();
-    v.boolean = b;
+    v.mkBool(b);
 }
 
-
 static inline void mkNull(Value & v)
 {
-    clearValue(v);
-    v.setNull();
+    v.mkNull();
 }
 
-
 static inline void mkApp(Value & v, Value & left, Value & right)
 {
-    v.setApp();
-    v.app.left = &left;
-    v.app.right = &right;
+    v.mkApp(&left, &right);
 }
 
-
-static inline void mkPrimOpApp(Value & v, Value & left, Value & right)
-{
-    v.setPrimOpApp();
-    v.app.left = &left;
-    v.app.right = &right;
-}
-
-
-static inline void mkStringNoCopy(Value & v, const char * s)
-{
-    v.setString();
-    v.string.s = s;
-    v.string.context = 0;
-}
-
-
 static inline void mkString(Value & v, const Symbol & s)
 {
-    mkStringNoCopy(v, ((const string &) s).c_str());
+    v.mkString(((const string &) s).c_str());
 }
 
 
 void mkString(Value & v, const char * s);
 
 
-static inline void mkPathNoCopy(Value & v, const char * s)
-{
-    clearValue(v);
-    v.setPath();
-    v.path = s;
-}
-
-
 void mkPath(Value & v, const char * s);
 
 
diff --git a/src/nix/repl.cc b/src/nix/repl.cc
index 673155078..a992d8732 100644
--- a/src/nix/repl.cc
+++ b/src/nix/repl.cc
@@ -551,9 +551,7 @@ bool NixRepl::processLine(string line)
         {
             Expr * e = parseString(string(line, p + 1));
             Value & v(*state->allocValue());
-            v.setThunk();
-            v.thunk.env = env;
-            v.thunk.expr = e;
+            v.mkThunk(env, e);
             addVarToScope(state->symbols.create(name), v);
         } else {
             Value v;