From 0bc468f195e37a8a5f4f8b36ae6c92459e8ca652 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Thu, 22 Apr 2010 15:08:09 +0000
Subject: [PATCH] * Simplify the implementation of `with'.  This gives a 7%
 speedup in   evaluating the NixOS system configuration.

---
 src/libexpr/eval.cc    | 34 +++++++++-------------------------
 src/libexpr/eval.hh    |  1 +
 src/libexpr/nixexpr.cc |  8 ++++----
 src/libexpr/nixexpr.hh |  2 +-
 4 files changed, 15 insertions(+), 30 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 9acd423108..b4f12c8a9c 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -246,10 +246,14 @@ Value * EvalState::lookupVar(Env * env, const VarRef & var)
     for (unsigned int l = var.level; l; --l, env = env->up) ;
     
     if (var.fromWith) {
-        Bindings::iterator j = env->values[0].attrs->find(var.name);
-        if (j == env->values[0].attrs->end())
-            throwEvalError("undefined variable `%1%'", var.name);
-        return &j->second;
+        while (1) {
+            Bindings::iterator j = env->values[0].attrs->find(var.name);
+            if (j != env->values[0].attrs->end())
+                return &j->second;
+            if (env->prevWith == 0)
+                throwEvalError("undefined variable `%1%'", var.name);
+            for (unsigned int l = env->prevWith; l; --l, env = env->up) ;
+        }
     } else
         return &env->values[var.displ];
 }
@@ -656,30 +660,10 @@ void ExprWith::eval(EvalState & state, Env & env, Value & v)
 {
     Env & env2(state.allocEnv(1));
     env2.up = &env;
+    env2.prevWith = prevWith;
 
     state.evalAttrs(env, attrs, env2.values[0]);
 
-    /* If there is an enclosing `with', copy all attributes that don't
-       appear in this `with'. */
-    if (prevWith != -1) {
-        Env * env3 = &env;
-        for (unsigned int l = prevWith; l; --l, env3 = env3->up) ;
-
-        /* Because the first `with' may be a shallow copy of another
-           attribute set (through a tCopy node), we need to clone its
-           `attrs' before modifying them. */
-        Bindings * old(env2.values[0].attrs);
-        state.mkAttrs(env2.values[0]);
-        foreach (Bindings::iterator, i, *old)
-            mkCopy((*env2.values[0].attrs)[i->first], i->second);
-
-        foreach (Bindings::iterator, i, *env3->values[0].attrs) {
-            Bindings::iterator j = env2.values[0].attrs->find(i->first);
-            if (j == env2.values[0].attrs->end())
-                mkCopy((*env2.values[0].attrs)[i->first], i->second);
-        }
-    }
-
     state.eval(env2, body, v);
 }
 
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index 2726fd971d..6912e22887 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -106,6 +106,7 @@ struct Value
 struct Env
 {
     Env * up;
+    unsigned int prevWith; // nr of levels up to next `with' environment
     Value values[0];
 };
 
diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc
index 3dfbddf5b8..a9c83108e9 100644
--- a/src/libexpr/nixexpr.cc
+++ b/src/libexpr/nixexpr.cc
@@ -278,12 +278,12 @@ void ExprLet::bindVars(const StaticEnv & env)
 void ExprWith::bindVars(const StaticEnv & env)
 {
     /* Does this `with' have an enclosing `with'?  If so, record its
-       level so that we can copy the attributes of the enclosing
-       `with'. */
+       level so that `lookupVar' can look up variables in the previous
+       `with' if this one doesn't contain the desired attribute. */
     const StaticEnv * curEnv;
     unsigned int level;
-    prevWith = -1;
-    for (curEnv = &env, level = 0; curEnv; curEnv = curEnv->up, level++)
+    prevWith = 0;
+    for (curEnv = &env, level = 1; curEnv; curEnv = curEnv->up, level++)
         if (curEnv->isWith) {
             prevWith = level;
             break;
diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh
index 4da77ee586..2d328d382c 100644
--- a/src/libexpr/nixexpr.hh
+++ b/src/libexpr/nixexpr.hh
@@ -184,7 +184,7 @@ struct ExprWith : Expr
 {
     Pos pos;
     Expr * attrs, * body;
-    int prevWith;
+    unsigned int prevWith;
     ExprWith(const Pos & pos, Expr * attrs, Expr * body) : pos(pos), attrs(attrs), body(body) { };
     COMMON_METHODS
 };