* Use a linked list of substitutions. This reduces the amount of

  copying.

4 files changed, 54 insertions, 19 deletions

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 384d87a26..1ee9da3b9 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -71,7 +71,7 @@ static Expr substArgs(Expr body, ATermList formals, Expr arg)
             throw Error(format("required function argument `%1%' missing")
                 % aterm2String(*i));
     
-    return substitute(subs, body);
+    return substitute(Substitution(0, &subs), body);
 }
 
 
@@ -97,11 +97,13 @@ ATerm expandRec(ATerm e, ATermList rbnds, ATermList nrbnds)
         subs.set(name, e2);
     }
 
+    Substitution subs_(0, &subs);
+
     /* Create the non-recursive set. */
     ATermMap as;
     for (ATermIterator i(rbnds); i; ++i) {
         if (!matchBind(*i, name, e2, pos)) abort(); /* can't happen */
-        as.set(name, makeAttrRHS(substitute(subs, e2), pos));
+        as.set(name, makeAttrRHS(substitute(subs_, e2), pos));
     }
 
     /* Copy the non-recursive bindings.  !!! inefficient */
@@ -344,7 +346,7 @@ Expr evalExpr2(EvalState & state, Expr e)
             try {
                 ATermMap subs;
                 subs.set(name, e2);
-                return evalExpr(state, substitute(subs, e4));
+                return evalExpr(state, substitute(Substitution(0, &subs), e4));
             } catch (Error & e) {
                 e.addPrefix(format("while evaluating the function at %1%:\n")
                     % showPos(pos));
@@ -402,7 +404,7 @@ Expr evalExpr2(EvalState & state, Expr e)
             throw;
         }
         try {
-            e2 = substitute(attrs, e2);
+            e2 = substitute(Substitution(0, &attrs), e2);
             checkVarDefs(state.primOps, e2);
             return evalExpr(state, e2);
         } catch (Error & e) {
@@ -534,4 +536,5 @@ void printEvalStats(EvalState & state)
         % state.nrEvaluated % state.nrCached
         % ((float) state.nrCached / (float) state.nrEvaluated * 100)
         % AT_calcAllocatedSize());
+    sleep(100);
 }
diff --git a/src/libexpr/nixexpr-ast.def b/src/libexpr/nixexpr-ast.def
index 3f6473e94..a4565ab1f 100644
--- a/src/libexpr/nixexpr-ast.def
+++ b/src/libexpr/nixexpr-ast.def
@@ -30,6 +30,7 @@ Uri | string | Expr |
 List | ATermList | Expr |
 BlackHole | | Expr |
 Undefined | | Expr |
+Removed | | Expr |
 PrimOp | int ATermBlob ATermList | Expr |
 Attrs | ATermList | Expr |
 Closed | Expr | Expr |
diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc
index 3adba1e2e..bfa7a9ed7 100644
--- a/src/libexpr/nixexpr.cc
+++ b/src/libexpr/nixexpr.cc
@@ -222,7 +222,7 @@ Expr makeAttrs(const ATermMap & attrs)
 }
 
 
-Expr substitute(const ATermMap & subs, Expr e)
+Expr substitute(const Substitution & subs, Expr e)
 {
     checkInterrupt();
 
@@ -235,7 +235,8 @@ Expr substitute(const ATermMap & subs, Expr e)
     if (matchClosed(e, e2)) return e;
 
     if (matchVar(e, name)) {
-        Expr sub = subs.get(name);
+        Expr sub = subs.lookup(name);
+        if (sub == makeRemoved()) sub = 0;
         Expr wrapped;
         /* Add a "closed" wrapper around terms that aren't already
            closed.  The check is necessary to prevent repeated
@@ -249,36 +250,37 @@ Expr substitute(const ATermMap & subs, Expr e)
     ATermList formals;
     ATerm body, def;
     if (matchFunction(e, formals, body, pos)) {
-        ATermMap subs2(subs);
+        ATermMap map;
         for (ATermIterator i(formals); i; ++i) {
             if (!matchNoDefFormal(*i, name) &&
                 !matchDefFormal(*i, name, def))
                 abort();
-            subs2.remove(name);
+            map.set(name, makeRemoved());
         }
+        Substitution subs2(&subs, &map);
         return makeFunction(
             (ATermList) substitute(subs2, (ATerm) formals),
             substitute(subs2, body), pos);
     }
 
     if (matchFunction1(e, name, body, pos)) {
-        ATermMap subs2(subs);
-        subs2.remove(name);
-        return makeFunction1(name, substitute(subs2, body), pos);
+        ATermMap map;
+        map.set(name, makeRemoved());
+        return makeFunction1(name, substitute(Substitution(&subs, &map), body), pos);
     }
         
     /* Idem for a mutually recursive attribute set. */
     ATermList rbnds, nrbnds;
     if (matchRec(e, rbnds, nrbnds)) {
-        ATermMap subs2(subs);
+        ATermMap map;
         for (ATermIterator i(rbnds); i; ++i)
-            if (matchBind(*i, name, e2, pos)) subs2.remove(name);
+            if (matchBind(*i, name, e2, pos)) map.set(name, makeRemoved());
             else abort(); /* can't happen */
         for (ATermIterator i(nrbnds); i; ++i)
-            if (matchBind(*i, name, e2, pos)) subs2.remove(name);
+            if (matchBind(*i, name, e2, pos)) map.set(name, makeRemoved());
             else abort(); /* can't happen */
         return makeRec(
-            (ATermList) substitute(subs2, (ATerm) rbnds),
+            (ATermList) substitute(Substitution(&subs, &map), (ATerm) rbnds),
             (ATermList) substitute(subs, (ATerm) nrbnds));
     }
 
@@ -286,11 +288,15 @@ Expr substitute(const ATermMap & subs, Expr e)
         AFun fun = ATgetAFun(e);
         int arity = ATgetArity(fun);
         ATerm args[arity];
+        bool changed = false;
 
-        for (int i = 0; i < arity; ++i)
-            args[i] = substitute(subs, ATgetArgument(e, i));
+        for (int i = 0; i < arity; ++i) {
+            ATerm arg = ATgetArgument(e, i);
+            args[i] = substitute(subs, arg);
+            if (args[i] != arg) changed = true;
+        }
         
-        return (ATerm) ATmakeApplArray(fun, args);
+        return changed ? (ATerm) ATmakeApplArray(fun, args) : e;
     }
 
     if (ATgetType(e) == AT_LIST) {
diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh
index d66b0819f..298a9f0b9 100644
--- a/src/libexpr/nixexpr.hh
+++ b/src/libexpr/nixexpr.hh
@@ -60,6 +60,31 @@ private:
 typedef vector<ATerm> ATermVector;
 
 
+/* A substitution is a linked list of ATermMaps that map names to
+   identifiers.  We use a list of ATermMaps rather than a single to
+   make it easy to grow or shrink a substitution when entering a
+   scope. */
+struct Substitution
+{
+    ATermMap * map;
+    const Substitution * prev;
+
+    Substitution(const Substitution * prev, ATermMap * map)
+    {
+        this->prev = prev;
+        this->map = map;
+    }
+    
+    Expr lookup(Expr name) const
+    {
+        Expr x;
+        for (const Substitution * s(this); s; s = s->prev)
+            if (x = s->map->get(name)) return x;
+        return 0;
+    }
+};
+
+
 /* Show a position. */
 string showPos(ATerm pos);
 
@@ -85,7 +110,7 @@ Expr queryAttr(Expr e, const string & name, ATerm & pos);
 Expr makeAttrs(const ATermMap & attrs);
 
 /* Perform a set of substitutions on an expression. */
-Expr substitute(const ATermMap & subs, Expr e);
+Expr substitute(const Substitution & subs, Expr e);
 
 /* Check whether all variables are defined in the given expression.
    Throw an exception if this isn't the case. */