Work on Values instead of Exprs

This prevents some duplicate evaluation in nix-env and
nix-instantiate.

Also, when traversing ~/.nix-defexpr, only read regular files with the
extension .nix.  Previously it was reading files like
.../channels/binary-caches/<name>.  The only reason this didn't cause
problems is pure luck (namely, <name> shadows an actual Nix
expression, the binary-caches files happen to be syntactically valid
Nix expressions, and we iterate over the directory contents in just
the right order).

4 files changed, 62 insertions, 50 deletions

diff --git a/src/libexpr/attr-path.cc b/src/libexpr/attr-path.cc
index 109001e6015d..d834dcae7ac4 100644
--- a/src/libexpr/attr-path.cc
+++ b/src/libexpr/attr-path.cc
@@ -6,9 +6,8 @@
 namespace nix {
 
 
-// !!! Shouldn't we return a pointer to a Value?
-void findAlongAttrPath(EvalState & state, const string & attrPath,
-    Bindings & autoArgs, Expr * e, Value & v)
+Value * findAlongAttrPath(EvalState & state, const string & attrPath,
+    Bindings & autoArgs, Value & vIn)
 {
     Strings tokens = tokenizeString<Strings>(attrPath, ".");
 
@@ -17,7 +16,7 @@ void findAlongAttrPath(EvalState & state, const string & attrPath,
 
     string curPath;
 
-    state.mkThunk_(v, e);
+    Value * v = &vIn;
 
     foreach (Strings::iterator, i, tokens) {
 
@@ -31,10 +30,10 @@ void findAlongAttrPath(EvalState & state, const string & attrPath,
         if (string2Int(attr, attrIndex)) apType = apIndex;
 
         /* Evaluate the expression. */
-        Value vTmp;
-        state.autoCallFunction(autoArgs, v, vTmp);
-        v = vTmp;
-        state.forceValue(v);
+        Value * vNew = state.allocValue();
+        state.autoCallFunction(autoArgs, *v, *vNew);
+        v = vNew;
+        state.forceValue(*v);
 
         /* It should evaluate to either an attribute set or an
            expression, according to what is specified in the
@@ -42,31 +41,33 @@ void findAlongAttrPath(EvalState & state, const string & attrPath,
 
         if (apType == apAttr) {
 
-            if (v.type != tAttrs)
+            if (v->type != tAttrs)
                 throw TypeError(
                     format("the expression selected by the selection path `%1%' should be an attribute set but is %2%")
-                    % curPath % showType(v));
+                    % curPath % showType(*v));
 
-            Bindings::iterator a = v.attrs->find(state.symbols.create(attr));
-            if (a == v.attrs->end())
+            Bindings::iterator a = v->attrs->find(state.symbols.create(attr));
+            if (a == v->attrs->end())
                 throw Error(format("attribute `%1%' in selection path `%2%' not found") % attr % curPath);
-            v = *a->value;
+            v = &*a->value;
         }
 
         else if (apType == apIndex) {
 
-            if (v.type != tList)
+            if (v->type != tList)
                 throw TypeError(
                     format("the expression selected by the selection path `%1%' should be a list but is %2%")
-                    % curPath % showType(v));
+                    % curPath % showType(*v));
 
-            if (attrIndex >= v.list.length)
+            if (attrIndex >= v->list.length)
                 throw Error(format("list index %1% in selection path `%2%' is out of range") % attrIndex % curPath);
 
-            v = *v.list.elems[attrIndex];
+            v = v->list.elems[attrIndex];
         }
 
     }
+
+    return v;
 }
 
 
diff --git a/src/libexpr/attr-path.hh b/src/libexpr/attr-path.hh
index d3aad746116a..46a341950939 100644
--- a/src/libexpr/attr-path.hh
+++ b/src/libexpr/attr-path.hh
@@ -7,7 +7,7 @@
 
 namespace nix {
 
-void findAlongAttrPath(EvalState & state, const string & attrPath,
-    Bindings & autoArgs, Expr * e, Value & v);
+Value * findAlongAttrPath(EvalState & state, const string & attrPath,
+    Bindings & autoArgs, Value & vIn);
 
 }
diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc
index 1cb1ec483339..1e1e4d00d589 100644
--- a/src/nix-env/nix-env.cc
+++ b/src/nix-env/nix-env.cc
@@ -94,18 +94,14 @@ static bool parseInstallSourceOptions(Globals & globals,
 }
 
 
-static bool isNixExpr(const Path & path)
+static bool isNixExpr(const Path & path, struct stat & st)
 {
-    struct stat st;
-    if (stat(path.c_str(), &st) == -1)
-        throw SysError(format("getting information about `%1%'") % path);
-
-    return !S_ISDIR(st.st_mode) || pathExists(path + "/default.nix");
+    return S_ISREG(st.st_mode) || (S_ISDIR(st.st_mode) && pathExists(path + "/default.nix"));
 }
 
 
 static void getAllExprs(EvalState & state,
-    const Path & path, ExprAttrs & attrs)
+    const Path & path, Value & v)
 {
     Strings names = readDirectory(path);
     StringSet namesSorted(names.begin(), names.end());
@@ -122,7 +118,7 @@ static void getAllExprs(EvalState & state,
         if (stat(path2.c_str(), &st) == -1)
             continue; // ignore dangling symlinks in ~/.nix-defexpr
 
-        if (isNixExpr(path2)) {
+        if (isNixExpr(path2, st) && (!S_ISREG(st.st_mode) || hasSuffix(path2, ".nix"))) {
             /* Strip off the `.nix' filename suffix (if applicable),
                otherwise the attribute cannot be selected with the
                `-A' option.  Useful if you want to stick a Nix
@@ -130,20 +126,28 @@ static void getAllExprs(EvalState & state,
             string attrName = *i;
             if (hasSuffix(attrName, ".nix"))
                 attrName = string(attrName, 0, attrName.size() - 4);
-            attrs.attrs[state.symbols.create(attrName)] =
-                ExprAttrs::AttrDef(state.parseExprFromFile(resolveExprPath(absPath(path2))), noPos);
+            // FIXME: make loading lazy.
+            Value & v2(*state.allocAttr(v, state.symbols.create(attrName)));
+            state.evalFile(path2, v2);
         }
-        else
+        else if (S_ISDIR(st.st_mode))
             /* `path2' is a directory (with no default.nix in it);
                recurse into it. */
-            getAllExprs(state, path2, attrs);
+            getAllExprs(state, path2, v);
     }
 }
 
 
-static Expr * loadSourceExpr(EvalState & state, const Path & path)
+static void loadSourceExpr(EvalState & state, const Path & path, Value & v)
 {
-    if (isNixExpr(path)) return state.parseExprFromFile(resolveExprPath(absPath(path)));
+    struct stat st;
+    if (stat(path.c_str(), &st) == -1)
+        throw SysError(format("getting information about `%1%'") % path);
+
+    if (isNixExpr(path, st)) {
+        state.evalFile(path, v);
+        return;
+    }
 
     /* The path is a directory.  Put the Nix expressions in the
        directory in an attribute set, with the file name of each
@@ -151,11 +155,12 @@ static Expr * loadSourceExpr(EvalState & state, const Path & path)
        (but keep the attribute set flat, not nested, to make it easier
        for a user to have a ~/.nix-defexpr directory that includes
        some system-wide directory). */
-    ExprAttrs * attrs = new ExprAttrs;
-    attrs->attrs[state.symbols.create("_combineChannels")] =
-        ExprAttrs::AttrDef(new ExprList(), noPos);
-    getAllExprs(state, path, *attrs);
-    return attrs;
+    if (S_ISDIR(st.st_mode)) {
+        state.mkAttrs(v, 16);
+        state.mkList(*state.allocAttr(v, state.symbols.create("_combineChannels")), 0);
+        getAllExprs(state, path, v);
+        v.attrs->sort();
+    }
 }
 
 
@@ -163,8 +168,10 @@ static void loadDerivations(EvalState & state, Path nixExprPath,
     string systemFilter, Bindings & autoArgs,
     const string & pathPrefix, DrvInfos & elems)
 {
-    Value v;
-    findAlongAttrPath(state, pathPrefix, autoArgs, loadSourceExpr(state, nixExprPath), v);
+    Value vRoot;
+    loadSourceExpr(state, nixExprPath, vRoot);
+
+    Value & v(*findAlongAttrPath(state, pathPrefix, autoArgs, vRoot));
 
     getDerivations(state, v, pathPrefix, autoArgs, elems, true);
 
@@ -356,13 +363,15 @@ static void queryInstSources(EvalState & state,
            (import ./foo.nix)' = `(import ./foo.nix).bar'. */
         case srcNixExprs: {
 
-            Expr * e1 = loadSourceExpr(state, instSource.nixExprPath);
+            Value vArg;
+            loadSourceExpr(state, instSource.nixExprPath, vArg);
 
             foreach (Strings::const_iterator, i, args) {
-                Expr * e2 = state.parseExprFromString(*i, absPath("."));
-                Expr * call = new ExprApp(e2, e1);
-                Value v; state.eval(call, v);
-                getDerivations(state, v, "", instSource.autoArgs, elems, true);
+                Expr * eFun = state.parseExprFromString(*i, absPath("."));
+                Value vFun, vTmp;
+                state.eval(eFun, vFun);
+                mkApp(vTmp, vFun, vArg);
+                getDerivations(state, vTmp, "", instSource.autoArgs, elems, true);
             }
 
             break;
@@ -413,10 +422,10 @@ static void queryInstSources(EvalState & state,
         }
 
         case srcAttrPath: {
+            Value vRoot;
+            loadSourceExpr(state, instSource.nixExprPath, vRoot);
             foreach (Strings::const_iterator, i, args) {
-                Value v;
-                findAlongAttrPath(state, *i, instSource.autoArgs,
-                    loadSourceExpr(state, instSource.nixExprPath), v);
+                Value & v(*findAlongAttrPath(state, *i, instSource.autoArgs, vRoot));
                 getDerivations(state, v, "", instSource.autoArgs, elems, true);
             }
             break;
diff --git a/src/nix-instantiate/nix-instantiate.cc b/src/nix-instantiate/nix-instantiate.cc
index 0f0710d680cb..fd2c04eae444 100644
--- a/src/nix-instantiate/nix-instantiate.cc
+++ b/src/nix-instantiate/nix-instantiate.cc
@@ -44,9 +44,11 @@ void processExpr(EvalState & state, const Strings & attrPaths,
         return;
     }
 
+    Value vRoot;
+    state.eval(e, vRoot);
+
     foreach (Strings::const_iterator, i, attrPaths) {
-        Value v;
-        findAlongAttrPath(state, *i, autoArgs, e, v);
+        Value & v(*findAlongAttrPath(state, *i, autoArgs, vRoot));
         state.forceValue(v);
 
         PathSet context;