From b70d22baca3e8826392b61aa53955c6da74b8724 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Fri, 18 Dec 2020 14:38:49 +0100 Subject: [PATCH] Replace Value type setters with mk* functions Move clearValue inside Value mkInt instead of setInt mkBool instead of setBool mkString instead of setString mkPath instead of setPath mkNull instead of setNull mkAttrs instead of setAttrs mkList instead of setList* mkThunk instead of setThunk mkApp instead of setApp mkLambda instead of setLambda mkBlackhole instead of setBlackhole mkPrimOp instead of setPrimOp mkPrimOpApp instead of setPrimOpApp mkExternal instead of setExternal mkFloat instead of setFloat Add note that the static mk* function should be removed eventually --- src/libexpr/attr-set.cc | 4 +- src/libexpr/eval-inline.hh | 6 +- src/libexpr/eval.cc | 49 +++------- src/libexpr/nixexpr.hh | 2 +- src/libexpr/primops.cc | 2 +- src/libexpr/value.hh | 193 ++++++++++++++++++++++++------------- src/nix/repl.cc | 4 +- 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) } } - 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 & 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;