From ac1e8f40d4a5c380d68bb6f1c7cef6f1e7987c1a Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <e.dolstra@tudelft.nl>
Date: Tue, 13 Apr 2010 12:25:42 +0000
Subject: [PATCH] * Use a symbol table to represent identifiers and attribute
 names   efficiently.  The symbol table ensures that there is only one copy  
 of each symbol, thus allowing symbols to be compared efficiently   using a
 pointer equality test.

---
 src/libexpr/Makefile.am                |  2 +-
 src/libexpr/attr-path.cc               |  2 +-
 src/libexpr/common-opts.cc             |  4 +-
 src/libexpr/eval-test.cc               | 25 ++++++++-
 src/libexpr/eval.cc                    | 45 +++++++++-------
 src/libexpr/eval.hh                    | 16 ++++--
 src/libexpr/get-drvs.cc                | 16 +++---
 src/libexpr/nixexpr.cc                 |  6 +--
 src/libexpr/nixexpr.hh                 | 26 ++++-----
 src/libexpr/parser.hh                  |  4 +-
 src/libexpr/parser.y                   | 69 ++++++++++++++----------
 src/libexpr/primops.cc                 | 27 +++++-----
 src/libexpr/symbol-table.hh            | 75 ++++++++++++++++++++++++++
 src/libexpr/value-to-xml.cc            |  8 +--
 src/nix-instantiate/nix-instantiate.cc |  4 +-
 15 files changed, 228 insertions(+), 101 deletions(-)
 create mode 100644 src/libexpr/symbol-table.hh

diff --git a/src/libexpr/Makefile.am b/src/libexpr/Makefile.am
index 60b1815d9..39423394a 100644
--- a/src/libexpr/Makefile.am
+++ b/src/libexpr/Makefile.am
@@ -8,7 +8,7 @@ libexpr_la_SOURCES = \
 pkginclude_HEADERS = \
  nixexpr.hh eval.hh parser.hh lexer-tab.hh parser-tab.hh \
  get-drvs.hh attr-path.hh value-to-xml.hh common-opts.hh \
- names.hh
+ names.hh symbol-table.hh
 
 libexpr_la_LIBADD = ../libutil/libutil.la ../libstore/libstore.la \
  ../boost/format/libformat.la
diff --git a/src/libexpr/attr-path.cc b/src/libexpr/attr-path.cc
index aa421ab43..b53837781 100644
--- a/src/libexpr/attr-path.cc
+++ b/src/libexpr/attr-path.cc
@@ -45,7 +45,7 @@ void findAlongAttrPath(EvalState & state, const string & attrPath,
                     format("the expression selected by the selection path `%1%' should be an attribute set but is %2%")
                     % curPath % showType(v));
 
-            Bindings::iterator a = v.attrs->find(attr);
+            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->second;
diff --git a/src/libexpr/common-opts.cc b/src/libexpr/common-opts.cc
index 6171de0f0..a25317de1 100644
--- a/src/libexpr/common-opts.cc
+++ b/src/libexpr/common-opts.cc
@@ -20,10 +20,10 @@ bool parseOptionArg(const string & arg, Strings::iterator & i,
     if (i == argsEnd) throw error;
     string value = *i++;
 
-    Value & v(autoArgs[name]);
+    Value & v(autoArgs[state.symbols.create(name)]);
 
     if (arg == "--arg")
-        state.mkThunk_(v, parseExprFromString(value, absPath(".")));
+        state.mkThunk_( v, parseExprFromString(state, value, absPath(".")));
     else
         mkString(v, value);
     
diff --git a/src/libexpr/eval-test.cc b/src/libexpr/eval-test.cc
index ffadd41a7..d03d3bdee 100644
--- a/src/libexpr/eval-test.cc
+++ b/src/libexpr/eval-test.cc
@@ -12,7 +12,7 @@ using namespace nix;
 
 void doTest(EvalState & state, string s)
 {
-    Expr * e = parseExprFromString(s, absPath("."));
+    Expr * e = parseExprFromString(state, s, absPath("."));
     std::cerr << ">>>>> " << *e << std::endl;
     Value v;
     state.eval(e, v);
@@ -23,6 +23,29 @@ void doTest(EvalState & state, string s)
 
 void run(Strings args)
 {
+    SymbolTable t;
+
+    printMsg(lvlError, format("size of symbol: %1% bytes") % sizeof(Symbol));
+    
+    Symbol s1 = t.create("foo");
+    Symbol s2 = t.create("foo");
+    Symbol s3 = t.create("bar");
+    Symbol s4 = t.create("foo");
+
+    assert(s1 == s2);
+    assert(s1 == s4);
+    assert(s1 != s3);
+
+    std::map<Symbol, int> m;
+
+    m[s1] = 123;
+    m[s3] = 456;
+
+    std::cout << m[s1] << std::endl;
+    std::cout << m[s2] << std::endl;
+    std::cout << m[s3] << std::endl;
+    std::cout << m[s4] << std::endl;
+
     EvalState state;
 
     printMsg(lvlError, format("size of value: %1% bytes") % sizeof(Value));
diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 99149fd7f..ffeae8d73 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -43,8 +43,8 @@ std::ostream & operator << (std::ostream & str, Value & v)
         break;
     case tAttrs:
         str << "{ ";
-        foreach (Bindings::iterator, i, *v.attrs)
-            str << i->first << " = " << i->second << "; ";
+        foreach (Bindings::iterator, i, *v.attrs) 
+            str << (string) i->first << " = " << i->second << "; ";
         str << "}";
         break;
     case tList:
@@ -91,7 +91,14 @@ string showType(Value & v)
 }
 
 
-EvalState::EvalState() : baseEnv(allocEnv())
+EvalState::EvalState()
+    : sWith(symbols.create("<with>"))
+    , sOutPath(symbols.create("outPath"))
+    , sDrvPath(symbols.create("drvPath"))
+    , sType(symbols.create("type"))
+    , sMeta(symbols.create("meta"))
+    , sName(symbols.create("name"))
+    , baseEnv(allocEnv())
 {
     nrValues = nrEnvs = nrEvaluated = recursionDepth = maxRecursionDepth = 0;
     deepestStack = (char *) -1;
@@ -110,9 +117,9 @@ EvalState::~EvalState()
 
 void EvalState::addConstant(const string & name, Value & v)
 {
-    baseEnv.bindings[name] = v;
+    baseEnv.bindings[symbols.create(name)] = v;
     string name2 = string(name, 0, 2) == "__" ? string(name, 2) : name;
-    (*baseEnv.bindings["builtins"].attrs)[name2] = v;
+    (*baseEnv.bindings[symbols.create("builtins")].attrs)[symbols.create(name2)] = v;
     nrValues += 2;
 }
 
@@ -124,9 +131,9 @@ void EvalState::addPrimOp(const string & name,
     v.type = tPrimOp;
     v.primOp.arity = arity;
     v.primOp.fun = primOp;
-    baseEnv.bindings[name] = v;
+    baseEnv.bindings[symbols.create(name)] = v;
     string name2 = string(name, 0, 2) == "__" ? string(name, 2) : name;
-    (*baseEnv.bindings["builtins"].attrs)[name2] = v;
+    (*baseEnv.bindings[symbols.create("builtins")].attrs)[symbols.create(name2)] = v;
     nrValues += 2;
 }
 
@@ -225,12 +232,12 @@ void mkPath(Value & v, const char * s)
 }
 
 
-static Value * lookupWith(Env * env, const Sym & name)
+Value * EvalState::lookupWith(Env * env, const Symbol & name)
 {
     if (!env) return 0;
     Value * v = lookupWith(env->up, name);
     if (v) return v;
-    Bindings::iterator i = env->bindings.find("<with>");
+    Bindings::iterator i = env->bindings.find(sWith);
     if (i == env->bindings.end()) return 0;
     Bindings::iterator j = i->second.attrs->find(name);
     if (j != i->second.attrs->end()) return &j->second;
@@ -238,7 +245,7 @@ static Value * lookupWith(Env * env, const Sym & name)
 }
 
 
-static Value * lookupVar(Env * env, const Sym & name)
+Value * EvalState::lookupVar(Env * env, const Symbol & name)
 {
     /* First look for a regular variable binding for `name'. */
     for (Env * env2 = env; env2; env2 = env2->up) {
@@ -318,7 +325,7 @@ void EvalState::evalFile(const Path & path, Value & v)
     Expr * e = parseTrees[path];
 
     if (!e) {
-        e = parseExprFromFile(path);
+        e = parseExprFromFile(*this, path);
         parseTrees[path] = e;
     }
     
@@ -428,9 +435,9 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
 
         /* The inherited attributes, on the other hand, are
            evaluated in the original environment. */
-        foreach (list<string>::iterator, i, inherited) {
+        foreach (list<Symbol>::iterator, i, inherited) {
             Value & v2 = env2.bindings[*i];
-            mkCopy(v2, *lookupVar(&env, *i));
+            mkCopy(v2, *state.lookupVar(&env, *i));
         }
     }
 
@@ -441,9 +448,9 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
             mkThunk(v2, env, i->second);
         }
 
-        foreach (list<string>::iterator, i, inherited) {
+        foreach (list<Symbol>::iterator, i, inherited) {
             Value & v2 = (*v.attrs)[*i];
-            mkCopy(v2, *lookupVar(&env, *i));
+            mkCopy(v2, *state.lookupVar(&env, *i));
         }
     }
 }
@@ -459,7 +466,7 @@ void ExprList::eval(EvalState & state, Env & env, Value & v)
 
 void ExprVar::eval(EvalState & state, Env & env, Value & v)
 {
-    Value * v2 = lookupVar(&env, name);
+    Value * v2 = state.lookupVar(&env, name);
     state.forceValue(*v2);
     v = *v2;
 }
@@ -631,7 +638,7 @@ void ExprWith::eval(EvalState & state, Env & env, Value & v)
     Env & env2(state.allocEnv());
     env2.up = &env;
 
-    Value & vAttrs = env2.bindings["<with>"];
+    Value & vAttrs = env2.bindings[state.sWith];
     state.eval(env, attrs, vAttrs);
     state.forceAttrs(vAttrs);
         
@@ -871,7 +878,7 @@ string EvalState::forceStringNoCtx(Value & v)
 bool EvalState::isDerivation(Value & v)
 {
     if (v.type != tAttrs) return false;
-    Bindings::iterator i = v.attrs->find("type");
+    Bindings::iterator i = v.attrs->find(sType);
     return i != v.attrs->end() && forceStringNoCtx(i->second) == "derivation";
 }
 
@@ -915,7 +922,7 @@ string EvalState::coerceToString(Value & v, PathSet & context,
     }
 
     if (v.type == tAttrs) {
-        Bindings::iterator i = v.attrs->find("outPath");
+        Bindings::iterator i = v.attrs->find(sOutPath);
         if (i == v.attrs->end())
             throwTypeError("cannot coerce an attribute set (except a derivation) to a string");
         return coerceToString(i->second, context, coerceMore, copyToStore);
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index cbdd09085..fe91db2ef 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -4,6 +4,7 @@
 #include <map>
 
 #include "nixexpr.hh"
+#include "symbol-table.hh"
 
 
 namespace nix {
@@ -14,9 +15,7 @@ class EvalState;
 struct Env;
 struct Value;
 
-typedef string Sym;
-
-typedef std::map<Sym, Value> Bindings;
+typedef std::map<Symbol, Value> Bindings;
 
 
 struct Env
@@ -161,6 +160,10 @@ class EvalState
 public:
     DrvHashes drvHashes; /* normalised derivation hashes */
 
+    SymbolTable symbols;
+
+    const Symbol sWith, sOutPath, sDrvPath, sType, sMeta, sName;
+
 private:
     SrcToStore srcToStore; 
 
@@ -235,6 +238,13 @@ private:
     void addPrimOp(const string & name,
         unsigned int arity, PrimOp primOp);
 
+    Value * lookupVar(Env * env, const Symbol & name);
+    
+    Value * lookupWith(Env * env, const Symbol & name);
+
+    friend class ExprVar;
+    friend class ExprAttrs;
+
 public:
     
     /* Do a deep equality test between two values.  That is, list
diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc
index 95938d5c1..399470192 100644
--- a/src/libexpr/get-drvs.cc
+++ b/src/libexpr/get-drvs.cc
@@ -8,7 +8,7 @@ namespace nix {
 string DrvInfo::queryDrvPath(EvalState & state) const
 {
     if (drvPath == "") {
-        Bindings::iterator i = attrs->find("drvPath");
+        Bindings::iterator i = attrs->find(state.sDrvPath);
         PathSet context;
         (string &) drvPath = i != attrs->end() ? state.coerceToPath(i->second, context) : "";
     }
@@ -19,7 +19,7 @@ string DrvInfo::queryDrvPath(EvalState & state) const
 string DrvInfo::queryOutPath(EvalState & state) const
 {
     if (outPath == "") {
-        Bindings::iterator i = attrs->find("outPath");
+        Bindings::iterator i = attrs->find(state.sOutPath);
         PathSet context;
         (string &) outPath = i != attrs->end() ? state.coerceToPath(i->second, context) : "";
     }
@@ -31,7 +31,7 @@ MetaInfo DrvInfo::queryMetaInfo(EvalState & state) const
 {
     MetaInfo meta;
     
-    Bindings::iterator a = attrs->find("meta");
+    Bindings::iterator a = attrs->find(state.sMeta);
     if (a == attrs->end()) return meta; /* fine, empty meta information */
 
     state.forceAttrs(a->second);
@@ -113,12 +113,12 @@ static bool getDerivation(EvalState & state, Value & v,
 
         DrvInfo drv;
     
-        Bindings::iterator i = v.attrs->find("name");
+        Bindings::iterator i = v.attrs->find(state.sName);
         /* !!! We really would like to have a decent back trace here. */
         if (i == v.attrs->end()) throw TypeError("derivation name missing");
         drv.name = state.forceStringNoCtx(i->second);
 
-        i = v.attrs->find("system");
+        i = v.attrs->find(state.symbols.create("system"));
         if (i == v.attrs->end())
             drv.system = "unknown";
         else
@@ -170,7 +170,7 @@ static void getDerivations(EvalState & state, Value & vIn,
 
         /* !!! undocumented hackery to support combining channels in
            nix-env.cc. */
-        bool combineChannels = v.attrs->find("_combineChannels") != v.attrs->end();
+        bool combineChannels = v.attrs->find(state.symbols.create("_combineChannels")) != v.attrs->end();
 
         /* Consider the attributes in sorted order to get more
            deterministic behaviour in nix-env operations (e.g. when
@@ -184,7 +184,7 @@ static void getDerivations(EvalState & state, Value & vIn,
         foreach (StringSet::iterator, i, attrs) {
             startNest(nest, lvlDebug, format("evaluating attribute `%1%'") % *i);
             string pathPrefix2 = addToPath(pathPrefix, *i);
-            Value & v2((*v.attrs)[*i]);
+            Value & v2((*v.attrs)[state.symbols.create(*i)]);
             if (combineChannels)
                 getDerivations(state, v2, pathPrefix2, autoArgs, drvs, done);
             else if (getDerivation(state, v2, pathPrefix2, drvs, done)) {
@@ -193,7 +193,7 @@ static void getDerivations(EvalState & state, Value & vIn,
                    if it has a `recurseForDerivations = true'
                    attribute. */
                 if (v2.type == tAttrs) {
-                    Bindings::iterator j = v2.attrs->find("recurseForDerivations");
+                    Bindings::iterator j = v2.attrs->find(state.symbols.create("recurseForDerivations"));
                     if (j != v2.attrs->end() && state.forceBool(j->second))
                         getDerivations(state, v2, pathPrefix2, autoArgs, drvs, done);
                 }
diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc
index 1902e23b0..922066c23 100644
--- a/src/libexpr/nixexpr.cc
+++ b/src/libexpr/nixexpr.cc
@@ -54,7 +54,7 @@ void ExprAttrs::show(std::ostream & str)
 {
     if (recursive) str << "rec ";
     str << "{ ";
-    foreach (list<string>::iterator, i, inherited)
+    foreach (list<Symbol>::iterator, i, inherited)
         str << "inherit " << *i << "; ";
     foreach (Attrs::iterator, i, attrs)
         str << i->first << " = " << *i->second << "; ";
@@ -81,9 +81,9 @@ void ExprLambda::show(std::ostream & str)
             if (i->def) str << " ? " << *i->def;
         }
         str << " }";
-        if (arg != "") str << " @ ";
+        if (!arg.empty()) str << " @ ";
     }
-    if (arg != "") str << arg;
+    if (!arg.empty()) str << arg;
     str << ": " << *body << ")";
 }
 
diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh
index 725f9fe88..f0c05d435 100644
--- a/src/libexpr/nixexpr.hh
+++ b/src/libexpr/nixexpr.hh
@@ -3,7 +3,7 @@
 
 #include <map>
 
-#include "types.hh"
+#include "symbol-table.hh"
 
 
 namespace nix {
@@ -75,33 +75,33 @@ struct ExprPath : Expr
 
 struct ExprVar : Expr
 {
-    string name;
-    ExprVar(const string & name) : name(name) { };
+    Symbol name;
+    ExprVar(const Symbol & name) : name(name) { };
     COMMON_METHODS
 };
 
 struct ExprSelect : Expr
 {
     Expr * e;
-    string name;
-    ExprSelect(Expr * e, const string & name) : e(e), name(name) { };
+    Symbol name;
+    ExprSelect(Expr * e, const Symbol & name) : e(e), name(name) { };
     COMMON_METHODS
 };
 
 struct ExprOpHasAttr : Expr
 {
     Expr * e;
-    string name;
-    ExprOpHasAttr(Expr * e, const string & name) : e(e), name(name) { };
+    Symbol name;
+    ExprOpHasAttr(Expr * e, const Symbol & name) : e(e), name(name) { };
     COMMON_METHODS
 };
 
 struct ExprAttrs : Expr
 {
     bool recursive;
-    typedef std::map<string, Expr *> Attrs;
+    typedef std::map<Symbol, Expr *> Attrs;
     Attrs attrs;
-    list<string> inherited;
+    list<Symbol> inherited;
     ExprAttrs() : recursive(false) { };
     COMMON_METHODS
 };
@@ -115,9 +115,9 @@ struct ExprList : Expr
 
 struct Formal
 {
-    string name;
+    Symbol name;
     Expr * def;
-    Formal(const string & name, Expr * def) : name(name), def(def) { };
+    Formal(const Symbol & name, Expr * def) : name(name), def(def) { };
 };
 
 struct Formals
@@ -130,11 +130,11 @@ struct Formals
 struct ExprLambda : Expr
 {
     Pos pos;
-    string arg;
+    Symbol arg;
     bool matchAttrs;
     Formals * formals;
     Expr * body;
-    ExprLambda(const Pos & pos, const string & arg, bool matchAttrs, Formals * formals, Expr * body)
+    ExprLambda(const Pos & pos, const Symbol & arg, bool matchAttrs, Formals * formals, Expr * body)
         : pos(pos), arg(arg), matchAttrs(matchAttrs), formals(formals), body(body) { };
     COMMON_METHODS
 };
diff --git a/src/libexpr/parser.hh b/src/libexpr/parser.hh
index d1e531ca2..c8c8ad809 100644
--- a/src/libexpr/parser.hh
+++ b/src/libexpr/parser.hh
@@ -9,10 +9,10 @@ namespace nix {
 
 /* Parse a Nix expression from the specified file.  If `path' refers
    to a directory, then "/default.nix" is appended. */
-Expr * parseExprFromFile(Path path);
+Expr * parseExprFromFile(EvalState & state, Path path);
 
 /* Parse a Nix expression from the specified string. */
-Expr * parseExprFromString(const string & s, const Path & basePath);
+Expr * parseExprFromString(EvalState & state, const string & s, const Path & basePath);
 
 
 }
diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y
index 07bf56a1c..c1c17e2b2 100644
--- a/src/libexpr/parser.y
+++ b/src/libexpr/parser.y
@@ -37,17 +37,23 @@ namespace nix {
     
 struct ParseData 
 {
+    SymbolTable & symbols;
     Expr * result;
     Path basePath;
     Path path;
     string error;
+    Symbol sLetBody;
+    ParseData(SymbolTable & symbols)
+        : symbols(symbols)
+        , sLetBody(symbols.create("<let-body>"))
+    { };
 };
 
 
-static string showAttrPath(const vector<string> & attrPath)
+static string showAttrPath(const vector<Symbol> & attrPath)
 {
     string s;
-    foreach (vector<string>::const_iterator, i, attrPath) {
+    foreach (vector<Symbol>::const_iterator, i, attrPath) {
         if (!s.empty()) s += '.';
         s += *i;
     }
@@ -55,10 +61,11 @@ static string showAttrPath(const vector<string> & attrPath)
 }
  
 
-static void addAttr(ExprAttrs * attrs, const vector<string> & attrPath, Expr * e, const Pos & pos)
+static void addAttr(ExprAttrs * attrs, const vector<Symbol> & attrPath,
+    Expr * e, const Pos & pos)
 {
     unsigned int n = 0;
-    foreach (vector<string>::const_iterator, i, attrPath) {
+    foreach (vector<Symbol>::const_iterator, i, attrPath) {
         n++;
         if (attrs->attrs[*i]) {
             ExprAttrs * attrs2 = dynamic_cast<ExprAttrs *>(attrs->attrs[*i]);
@@ -243,10 +250,10 @@ void yyerror(YYLTYPE * loc, yyscan_t scanner, ParseData * data, const char * err
   nix::Formals * formals;
   nix::Formal * formal;
   int n;
-  char * id;
+  char * id; // !!! -> Symbol
   char * path;
   char * uri;
-  std::vector<std::string> * ids;
+  std::vector<nix::Symbol> * ids;
   std::vector<nix::Expr *> * string_parts;
 }
 
@@ -287,19 +294,19 @@ expr: expr_function;
 
 expr_function
   : ID ':' expr_function
-    { $$ = new ExprLambda(CUR_POS, $1, false, 0, $3); /* checkPatternVars(CUR_POS, $1); $$ = makeFunction($1, $3, CUR_POS); */ }
+    { $$ = new ExprLambda(CUR_POS, data->symbols.create($1), false, 0, $3); /* checkPatternVars(CUR_POS, $1); */ }
   | '{' formals '}' ':' expr_function
-    { $$ = new ExprLambda(CUR_POS, "", true, $2, $5); }
+    { $$ = new ExprLambda(CUR_POS, data->symbols.create(""), true, $2, $5); }
   | '{' formals '}' '@' ID ':' expr_function
-    { $$ = new ExprLambda(CUR_POS, $5, true, $2, $7); }
+    { $$ = new ExprLambda(CUR_POS, data->symbols.create($5), true, $2, $7); }
   | ID '@' '{' formals '}' ':' expr_function
-    { $$ = new ExprLambda(CUR_POS, $1, true, $4, $7); }
+    { $$ = new ExprLambda(CUR_POS, data->symbols.create($1), true, $4, $7); }
   | ASSERT expr ';' expr_function
     { $$ = new ExprAssert(CUR_POS, $2, $4); }
   | WITH expr ';' expr_function
     { $$ = new ExprWith(CUR_POS, $2, $4); }
   | LET binds IN expr_function
-    { $2->attrs["<let-body>"] = $4; $2->recursive = true; $$ = new ExprSelect($2, "<let-body>"); }
+    { $2->attrs[data->sLetBody] = $4; $2->recursive = true; $$ = new ExprSelect($2, data->sLetBody); }
   | expr_if
   ;
 
@@ -316,7 +323,7 @@ expr_op
   | expr_op OR expr_op { $$ = new ExprOpOr($1, $3); }
   | expr_op IMPL expr_op { $$ = new ExprOpImpl($1, $3); }
   | expr_op UPDATE expr_op { $$ = new ExprOpUpdate($1, $3); }
-  | expr_op '?' ID { $$ = new ExprOpHasAttr($1, $3); }
+  | expr_op '?' ID { $$ = new ExprOpHasAttr($1, data->symbols.create($3)); }
   | expr_op '+' expr_op
     { vector<Expr *> * l = new vector<Expr *>;
       l->push_back($1);
@@ -335,12 +342,12 @@ expr_app
 
 expr_select
   : expr_select '.' ID
-    { $$ = new ExprSelect($1, $3); }
+    { $$ = new ExprSelect($1, data->symbols.create($3)); }
   | expr_simple { $$ = $1; }
   ;
 
 expr_simple
-  : ID { $$ = new ExprVar($1); }
+  : ID { $$ = new ExprVar(data->symbols.create($1)); }
   | INT { $$ = new ExprInt($1); }
   | '"' string_parts '"' {
       /* For efficiency, and to simplify parse trees a bit. */
@@ -357,7 +364,7 @@ expr_simple
   /* Let expressions `let {..., body = ...}' are just desugared
      into `(rec {..., body = ...}).body'. */
   | LET '{' binds '}'
-    { $3->recursive = true; $$ = new ExprSelect($3, "body"); }
+    { $3->recursive = true; $$ = new ExprSelect($3, data->symbols.create("body")); }
   | REC '{' binds '}'
     { $3->recursive = true; $$ = $3; }
   | '{' binds '}'
@@ -381,26 +388,26 @@ binds
   : binds attrpath '=' expr ';' { $$ = $1; addAttr($$, *$2, $4, CUR_POS); }
   | binds INHERIT ids ';'
     { $$ = $1;
-      foreach (vector<string>::iterator, i, *$3)
+      foreach (vector<Symbol>::iterator, i, *$3)
         $$->inherited.push_back(*i);
     }
   | binds INHERIT '(' expr ')' ids ';'
     { $$ = $1;
       /* !!! Should ensure sharing of the expression in $4. */
-      foreach (vector<string>::iterator, i, *$6)
+      foreach (vector<Symbol>::iterator, i, *$6)
         $$->attrs[*i] = new ExprSelect($4, *i);
     }
   | { $$ = new ExprAttrs; }
   ;
 
 ids
-  : ids ID { $$ = $1; $1->push_back($2); /* !!! dangerous */ }
-  | { $$ = new vector<string>; }
+  : ids ID { $$ = $1; $1->push_back(data->symbols.create($2)); /* !!! dangerous */ }
+  | { $$ = new vector<Symbol>; }
   ;
 
 attrpath
-  : attrpath '.' ID { $$ = $1; $1->push_back($3); }
-  | ID { $$ = new vector<string>; $$->push_back($1); }
+  : attrpath '.' ID { $$ = $1; $1->push_back(data->symbols.create($3)); }
+  | ID { $$ = new vector<Symbol>; $$->push_back(data->symbols.create($1)); }
   ;
 
 expr_list
@@ -420,8 +427,8 @@ formals
   ;
 
 formal
-  : ID { $$ = new Formal($1, 0); }
-  | ID '?' expr { $$ = new Formal($1, $3); }
+  : ID { $$ = new Formal(data->symbols.create($1), 0); }
+  | ID '?' expr { $$ = new Formal(data->symbols.create($1), $3); }
   ;
   
 %%
@@ -432,14 +439,17 @@ formal
 #include <fcntl.h>
 #include <unistd.h>
 
+#include <eval.hh>
+
 
 namespace nix {
       
 
-static Expr * parse(const char * text, const Path & path, const Path & basePath)
+static Expr * parse(EvalState & state, const char * text,
+    const Path & path, const Path & basePath)
 {
     yyscan_t scanner;
-    ParseData data;
+    ParseData data(state.symbols);
     data.basePath = basePath;
     data.path = path;
 
@@ -460,7 +470,7 @@ static Expr * parse(const char * text, const Path & path, const Path & basePath)
 }
 
 
-Expr * parseExprFromFile(Path path)
+Expr * parseExprFromFile(EvalState & state, Path path)
 {
     assert(path[0] == '/');
 
@@ -481,13 +491,14 @@ Expr * parseExprFromFile(Path path)
         path = canonPath(path + "/default.nix");
 
     /* Read and parse the input file. */
-    return parse(readFile(path).c_str(), path, dirOf(path));
+    return parse(state, readFile(path).c_str(), path, dirOf(path));
 }
 
 
-Expr * parseExprFromString(const string & s, const Path & basePath)
+Expr * parseExprFromString(EvalState & state,
+    const string & s, const Path & basePath)
 {
-    return parse(s.c_str(), "(string)", basePath);
+    return parse(state, s.c_str(), "(string)", basePath);
 }
 
  
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index e50034a04..257dcd2e9 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -280,7 +280,7 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v)
     state.forceAttrs(*args[0]);
 
     /* Figure out the name first (for stack backtraces). */
-    Bindings::iterator attr = args[0]->attrs->find("name");
+    Bindings::iterator attr = args[0]->attrs->find(state.sName);
     if (attr == args[0]->attrs->end())
         throw EvalError("required attribute `name' missing");
     string drvName;
@@ -448,8 +448,8 @@ static void prim_derivationStrict(EvalState & state, Value * * args, Value & v)
 
     /* !!! assumes a single output */
     state.mkAttrs(v);
-    mkString((*v.attrs)["outPath"], outPath, singleton<PathSet>(drvPath));
-    mkString((*v.attrs)["drvPath"], drvPath, singleton<PathSet>("=" + drvPath));
+    mkString((*v.attrs)[state.sOutPath], outPath, singleton<PathSet>(drvPath));
+    mkString((*v.attrs)[state.sDrvPath], drvPath, singleton<PathSet>("=" + drvPath));
 }
 
 
@@ -667,7 +667,8 @@ static void prim_getAttr(EvalState & state, Value * * args, Value & v)
 {
     string attr = state.forceStringNoCtx(*args[0]);
     state.forceAttrs(*args[1]);
-    Bindings::iterator i = args[1]->attrs->find(attr);
+    // !!! Should we create a symbol here or just do a lookup?
+    Bindings::iterator i = args[1]->attrs->find(state.symbols.create(attr));
     if (i == args[1]->attrs->end())
         throw EvalError(format("attribute `%1%' missing") % attr);
     state.forceValue(i->second);
@@ -680,7 +681,7 @@ static void prim_hasAttr(EvalState & state, Value * * args, Value & v)
 {
     string attr = state.forceStringNoCtx(*args[0]);
     state.forceAttrs(*args[1]);
-    mkBool(v, args[1]->attrs->find(attr) != args[1]->attrs->end());
+    mkBool(v, args[1]->attrs->find(state.symbols.create(attr)) != args[1]->attrs->end());
 }
 
 
@@ -701,7 +702,7 @@ static void prim_removeAttrs(EvalState & state, Value * * args, Value & v)
 
     for (unsigned int i = 0; i < args[1]->list.length; ++i) {
         state.forceStringNoCtx(args[1]->list.elems[i]);
-        v.attrs->erase(args[1]->list.elems[i].string.s);
+        v.attrs->erase(state.symbols.create(args[1]->list.elems[i].string.s));
     }
 }
 
@@ -720,16 +721,16 @@ static void prim_listToAttrs(EvalState & state, Value * * args, Value & v)
         Value & v2(args[0]->list.elems[i]);
         state.forceAttrs(v2);
         
-        Bindings::iterator j = v2.attrs->find("name");
+        Bindings::iterator j = v2.attrs->find(state.sName);
         if (j == v2.attrs->end())
             throw TypeError("`name' attribute missing in a call to `listToAttrs'");
         string name = state.forceStringNoCtx(j->second);
         
-        j = v2.attrs->find("value");
+        j = v2.attrs->find(state.symbols.create("value"));
         if (j == v2.attrs->end())
             throw TypeError("`value' attribute missing in a call to `listToAttrs'");
 
-        (*v.attrs)[name] = j->second; // !!! sharing?
+        (*v.attrs)[state.symbols.create(name)] = j->second; // !!! sharing?
     }
 }
 
@@ -976,8 +977,8 @@ static void prim_parseDrvName(EvalState & state, Value * * args, Value & v)
     string name = state.forceStringNoCtx(*args[0]);
     DrvName parsed(name);
     state.mkAttrs(v);
-    mkString((*v.attrs)["name"], parsed.name);
-    mkString((*v.attrs)["version"], parsed.version);
+    mkString((*v.attrs)[state.sName], parsed.name);
+    mkString((*v.attrs)[state.symbols.create("version")], parsed.version);
 }
 
 
@@ -998,7 +999,7 @@ void EvalState::createBaseEnv()
 {
     baseEnv.up = 0;
 
-    Value & builtins = baseEnv.bindings["builtins"];
+    Value & builtins = baseEnv.bindings[symbols.create("builtins")];
     builtins.type = tAttrs;
     builtins.attrs = new Bindings;
 
@@ -1023,7 +1024,7 @@ void EvalState::createBaseEnv()
     /* Add a wrapper around the derivation primop that computes the
        `drvPath' and `outPath' attributes lazily. */
     string s = "attrs: let res = derivationStrict attrs; in attrs // { drvPath = res.drvPath; outPath = res.outPath; type = \"derivation\"; }";
-    mkThunk(v, baseEnv, parseExprFromString(s, "/"));
+    mkThunk(v, baseEnv, parseExprFromString(*this, s, "/"));
     addConstant("derivation", v);
 
     // Miscellaneous
diff --git a/src/libexpr/symbol-table.hh b/src/libexpr/symbol-table.hh
new file mode 100644
index 000000000..ae0751d11
--- /dev/null
+++ b/src/libexpr/symbol-table.hh
@@ -0,0 +1,75 @@
+#ifndef __SYMBOL_TABLE_H
+#define __SYMBOL_TABLE_H
+
+#include <map>
+
+#include "types.hh"
+
+namespace nix {
+
+/* Symbol table used by the parser and evaluator to represent and look
+   up identifiers and attribute sets efficiently.
+   SymbolTable::create() converts a string into a symbol.  Symbols
+   have the property that they can be compared efficiently (using a
+   pointer equality test), because the symbol table stores only one
+   copy of each string. */
+
+class Symbol
+{
+private:
+    const string * s; // pointer into SymbolTable
+    Symbol(const string * s) : s(s) { };
+    friend class SymbolTable;
+
+public:
+    bool operator == (const Symbol & s2) const
+    {
+        return s == s2.s;
+    }
+    
+    bool operator != (const Symbol & s2) const
+    {
+        return s != s2.s;
+    }
+    
+    bool operator < (const Symbol & s2) const
+    {
+        return s < s2.s;
+    }
+
+    operator const string & () const
+    {
+        return *s;
+    }
+
+    bool empty() const
+    {
+        return s->empty();
+    }
+
+    friend std::ostream & operator << (std::ostream & str, const Symbol & sym);
+};
+
+inline std::ostream & operator << (std::ostream & str, const Symbol & sym)
+{
+    str << *sym.s;
+    return str;
+}
+
+class SymbolTable
+{
+private:
+    typedef std::set<string> Symbols;
+    Symbols symbols;
+
+public:
+    Symbol create(const string & s)
+    {
+        std::pair<Symbols::iterator, bool> res = symbols.insert(s);
+        return Symbol(&*res.first);
+    }
+};
+
+}
+
+#endif /* !__SYMBOL_TABLE_H */
diff --git a/src/libexpr/value-to-xml.cc b/src/libexpr/value-to-xml.cc
index c8a067aac..0c8fc143c 100644
--- a/src/libexpr/value-to-xml.cc
+++ b/src/libexpr/value-to-xml.cc
@@ -28,7 +28,7 @@ static void showAttrs(EvalState & state, bool strict, Bindings & attrs,
         names.insert(i->first);
     foreach (StringSet::iterator, i, names) {
         XMLOpenElement _(doc, "attr", singletonAttrs("name", *i));
-        printValueAsXML(state, strict, attrs[*i], doc, context, drvsSeen);
+        printValueAsXML(state, strict, attrs[state.symbols.create(*i)], doc, context, drvsSeen);
     }
 }
 
@@ -67,14 +67,14 @@ static void printValueAsXML(EvalState & state, bool strict, Value & v,
             if (state.isDerivation(v)) {
                 XMLAttrs xmlAttrs;
             
-                Bindings::iterator a = v.attrs->find("derivation");
+                Bindings::iterator a = v.attrs->find(state.symbols.create("derivation"));
 
                 Path drvPath;
-                a = v.attrs->find("drvPath");
+                a = v.attrs->find(state.sDrvPath);
                 if (a != v.attrs->end() && a->second.type == tString)
                     xmlAttrs["drvPath"] = drvPath = a->second.string.s;
         
-                a = v.attrs->find("outPath");
+                a = v.attrs->find(state.sOutPath);
                 if (a != v.attrs->end() && a->second.type == tString)
                     xmlAttrs["outPath"] = a->second.string.s;
 
diff --git a/src/nix-instantiate/nix-instantiate.cc b/src/nix-instantiate/nix-instantiate.cc
index ba34fc63b..c61cf89c9 100644
--- a/src/nix-instantiate/nix-instantiate.cc
+++ b/src/nix-instantiate/nix-instantiate.cc
@@ -28,7 +28,7 @@ static Expr * parseStdin(EvalState & state)
     startNest(nest, lvlTalkative, format("parsing standard input"));
     string s, s2;
     while (getline(std::cin, s2)) s += s2 + "\n";
-    return parseExprFromString(s, absPath("."));
+    return parseExprFromString(state, s, absPath("."));
 }
 
 
@@ -136,7 +136,7 @@ void run(Strings args)
 
     foreach (Strings::iterator, i, files) {
         Path path = absPath(*i);
-        Expr * e = parseExprFromFile(path);
+        Expr * e = parseExprFromFile(state, path);
         processExpr(state, attrPaths, parseOnly, strict, autoArgs,
             evalOnly, xmlOutput, e);
     }