From afc6c1bad63e27d68adf49e673f8aafd36495a8a Mon Sep 17 00:00:00 2001 From: Shea Levy Date: Mon, 15 Jul 2013 17:10:18 -0400 Subject: [PATCH] Simplify inherited attribute handling This reduces the difference between inherited and non-inherited attribute handling to the choice of which env to use (in recs and lets) by setting the AttrDef::e to a new ExprVar in the parser rather than carrying a separate AttrDef::v VarRef member. As an added bonus, this allows inherited attributes that inherit from a with to delay forcing evaluation of the with's attributes. Signed-off-by: Shea Levy --- src/libexpr/eval.cc | 40 +++++++------------ src/libexpr/eval.hh | 2 +- src/libexpr/nixexpr.cc | 9 ++--- src/libexpr/nixexpr.hh | 6 +-- src/libexpr/parser.y | 2 +- tests/lang/eval-okay-delayed-with-inherit.exp | 1 + tests/lang/eval-okay-delayed-with-inherit.nix | 24 +++++++++++ 7 files changed, 46 insertions(+), 38 deletions(-) create mode 100644 tests/lang/eval-okay-delayed-with-inherit.exp create mode 100644 tests/lang/eval-okay-delayed-with-inherit.nix diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 4fecd2a70..82287f627 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -521,23 +521,17 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v) environment, while the inherited attributes are evaluated in the original environment. */ unsigned int displ = 0; - foreach (AttrDefs::iterator, i, attrs) - if (i->second.inherited) { - /* !!! handle overrides? */ - Value * vAttr = state.lookupVar(&env, i->second.var); - env2.values[displ++] = vAttr; - v.attrs->push_back(Attr(i->first, vAttr, &i->second.pos)); - } else { - Value * vAttr; - if (hasOverrides) { - vAttr = state.allocValue(); - mkThunk(*vAttr, env2, i->second.e); - } else - vAttr = i->second.e->maybeThunk(state, env2); - env2.values[displ++] = vAttr; - v.attrs->push_back(Attr(i->first, vAttr, &i->second.pos)); - } - + foreach (AttrDefs::iterator, i, attrs) { + Value * vAttr; + if (hasOverrides && !i->second.inherited) { + vAttr = state.allocValue(); + mkThunk(*vAttr, env2, i->second.e); + } else + vAttr = i->second.e->maybeThunk(state, i->second.inherited ? env : env2); + env2.values[displ++] = vAttr; + v.attrs->push_back(Attr(i->first, vAttr, &i->second.pos)); + } + /* If the rec contains an attribute called `__overrides', then evaluate it, and add the attributes in that set to the rec. This allows overriding of recursive attributes, which is @@ -563,10 +557,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v) else { foreach (AttrDefs::iterator, i, attrs) - if (i->second.inherited) - v.attrs->push_back(Attr(i->first, state.lookupVar(&env, i->second.var), &i->second.pos)); - else - v.attrs->push_back(Attr(i->first, i->second.e->maybeThunk(state, env), &i->second.pos)); + v.attrs->push_back(Attr(i->first, i->second.e->maybeThunk(state, env), &i->second.pos)); } } @@ -583,10 +574,7 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v) environment. */ unsigned int displ = 0; foreach (ExprAttrs::AttrDefs::iterator, i, attrs->attrs) - if (i->second.inherited) - env2.values[displ++] = state.lookupVar(&env, i->second.var); - else - env2.values[displ++] = i->second.e->maybeThunk(state, env2); + env2.values[displ++] = i->second.e->maybeThunk(state, i->second.inherited ? env : env2); body->eval(state, env2, v); } @@ -602,7 +590,7 @@ void ExprList::eval(EvalState & state, Env & env, Value & v) void ExprVar::eval(EvalState & state, Env & env, Value & v) { - Value * v2 = state.lookupVar(&env, info); + Value * v2 = state.lookupVar(&env, info, false); state.forceValue(*v2); v = *v2; } diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 9717fb66d..b7b527bc4 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -206,7 +206,7 @@ private: void addPrimOp(const string & name, unsigned int arity, PrimOpFun primOp); - inline Value * lookupVar(Env * env, const VarRef & var, bool noEval = false); + inline Value * lookupVar(Env * env, const VarRef & var, bool noEval); friend class ExprVar; friend class ExprAttrs; diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 9b75bb644..72bc0a14b 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -230,14 +230,12 @@ void ExprAttrs::bindVars(const StaticEnv & env) newEnv.vars[i->first] = i->second.displ = displ++; foreach (AttrDefs::iterator, i, attrs) - if (i->second.inherited) i->second.var.bind(env); - else i->second.e->bindVars(newEnv); + i->second.e->bindVars(i->second.inherited ? env : newEnv); } else foreach (AttrDefs::iterator, i, attrs) - if (i->second.inherited) i->second.var.bind(env); - else i->second.e->bindVars(env); + i->second.e->bindVars(env); } void ExprList::bindVars(const StaticEnv & env) @@ -274,8 +272,7 @@ void ExprLet::bindVars(const StaticEnv & env) newEnv.vars[i->first] = i->second.displ = displ++; foreach (ExprAttrs::AttrDefs::iterator, i, attrs->attrs) - if (i->second.inherited) i->second.var.bind(env); - else i->second.e->bindVars(newEnv); + i->second.e->bindVars(i->second.inherited ? env : newEnv); body->bindVars(newEnv); } diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index f6dcb3935..69372a77b 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -159,12 +159,10 @@ struct ExprAttrs : Expr bool recursive; struct AttrDef { bool inherited; - Expr * e; // if not inherited - VarRef var; // if inherited + Expr * e; Pos pos; unsigned int displ; // displacement - AttrDef(Expr * e, const Pos & pos) : inherited(false), e(e), pos(pos) { }; - AttrDef(const Symbol & name, const Pos & pos) : inherited(true), var(name), pos(pos) { }; + AttrDef(Expr * e, const Pos & pos, bool inherited=false) : inherited(inherited), e(e), pos(pos) { }; AttrDef() { }; }; typedef std::map AttrDefs; diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 7690271a8..0e3086004 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -413,7 +413,7 @@ binds if ($$->attrs.find(*i) != $$->attrs.end()) dupAttr(*i, makeCurPos(@3, data), $$->attrs[*i].pos); Pos pos = makeCurPos(@3, data); - $$->attrs[*i] = ExprAttrs::AttrDef(*i, pos); + $$->attrs[*i] = ExprAttrs::AttrDef(new ExprVar(*i), pos, true); } } | binds INHERIT '(' expr ')' attrs ';' diff --git a/tests/lang/eval-okay-delayed-with-inherit.exp b/tests/lang/eval-okay-delayed-with-inherit.exp new file mode 100644 index 000000000..eaacb55c1 --- /dev/null +++ b/tests/lang/eval-okay-delayed-with-inherit.exp @@ -0,0 +1 @@ +"b-overridden" diff --git a/tests/lang/eval-okay-delayed-with-inherit.nix b/tests/lang/eval-okay-delayed-with-inherit.nix new file mode 100644 index 000000000..84b388c27 --- /dev/null +++ b/tests/lang/eval-okay-delayed-with-inherit.nix @@ -0,0 +1,24 @@ +let + pkgs_ = with pkgs; { + a = derivation { + name = "a"; + system = builtins.currentSystem; + builder = "/bin/sh"; + args = [ "-c" "touch $out" ]; + inherit b; + }; + + inherit b; + }; + + packageOverrides = p: { + b = derivation { + name = "b-overridden"; + system = builtins.currentSystem; + builder = "/bin/sh"; + args = [ "-c" "touch $out" ]; + }; + }; + + pkgs = pkgs_ // (packageOverrides pkgs_); +in pkgs.a.b.name