245/376: Get rid of some unnecessary ExprConcatStrings nodes in dynamic attrs

[Ludovic Courtès]

civodul pushed a commit to tag 1.8
in repository guix.

commit b6809608cc467925db44b1eb435095c37e433255
Author: Eelco Dolstra <address@hidden>
Date:   Sun Oct 5 01:04:58 2014 +0200

    Get rid of some unnecessary ExprConcatStrings nodes in dynamic attrs
    
    This gives a ~18% speedup in NixOS evaluation (after converting
    most calls to hasAttr/getAttr to dynamic attrs).
---
 src/libexpr/eval.cc    |    9 +++------
 src/libexpr/nixexpr.hh |   14 +++++++-------
 src/libexpr/parser.y   |   20 ++++++++------------
 3 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 1eeb809..5122bd1 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -701,13 +701,10 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value 
& v)
     /* Dynamic attrs apply *after* rec and __overrides. */
     foreach (DynamicAttrDefs::iterator, i, dynamicAttrs) {
         Value nameVal;
-        if (i->nameExpr->es->size() == 1) {
-            i->nameExpr->es->front()->eval(state, *dynamicEnv, nameVal);
-            state.forceValue(nameVal);
-            if (nameVal.type == tNull)
-                continue;
-        }
         i->nameExpr->eval(state, *dynamicEnv, nameVal);
+        state.forceValue(nameVal);
+        if (nameVal.type == tNull)
+            continue;
         state.forceStringNoCtx(nameVal);
         Symbol nameSym = state.symbols.create(nameVal.string.s);
         Bindings::iterator j = v.attrs->find(nameSym);
diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh
index 2619a70..0eaa362 100644
--- a/src/libexpr/nixexpr.hh
+++ b/src/libexpr/nixexpr.hh
@@ -53,16 +53,15 @@ struct Env;
 struct Value;
 class EvalState;
 struct StaticEnv;
-struct ExprConcatStrings;
 
 
 /* An attribute path is a sequence of attribute names. */
 struct AttrName
 {
     Symbol symbol;
-    ExprConcatStrings * expr;
+    Expr * expr;
     AttrName(const Symbol & s) : symbol(s) {};
-    AttrName(ExprConcatStrings * e) : expr(e) {};
+    AttrName(Expr * e) : expr(e) {};
 };
 
 typedef std::vector<AttrName> AttrPath;
@@ -173,16 +172,17 @@ struct ExprAttrs : Expr
         Expr * e;
         Pos pos;
         unsigned int displ; // displacement
-        AttrDef(Expr * e, const Pos & pos, bool inherited=false) : 
inherited(inherited), e(e), pos(pos) { };
+        AttrDef(Expr * e, const Pos & pos, bool inherited=false)
+            : inherited(inherited), e(e), pos(pos) { };
         AttrDef() { };
     };
     typedef std::map<Symbol, AttrDef> AttrDefs;
     AttrDefs attrs;
     struct DynamicAttrDef {
-        ExprConcatStrings * nameExpr;
-        Expr * valueExpr;
+        Expr * nameExpr, * valueExpr;
         Pos pos;
-        DynamicAttrDef(ExprConcatStrings * nameExpr, Expr * valueExpr, const 
Pos & pos) : nameExpr(nameExpr), valueExpr(valueExpr), pos(pos) { };
+        DynamicAttrDef(Expr * nameExpr, Expr * valueExpr, const Pos & pos)
+            : nameExpr(nameExpr), valueExpr(valueExpr), pos(pos) { };
     };
     typedef std::vector<DynamicAttrDef> DynamicAttrDefs;
     DynamicAttrDefs dynamicAttrs;
diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y
index 60a87ca..dcb270b 100644
--- a/src/libexpr/parser.y
+++ b/src/libexpr/parser.y
@@ -341,11 +341,7 @@ expr_op
   | expr_op UPDATE expr_op { $$ = new ExprOpUpdate(CUR_POS, $1, $3); }
   | expr_op '?' attrpath { $$ = new ExprOpHasAttr($1, *$3); }
   | expr_op '+' expr_op
-    { vector<Expr *> * l = new vector<Expr *>;
-      l->push_back($1);
-      l->push_back($3);
-      $$ = new ExprConcatStrings(CUR_POS, false, l);
-    }
+    { $$ = new ExprConcatStrings(CUR_POS, false, new vector<Expr *>({$1, 
$3})); }
   | expr_op '-' expr_op { $$ = new ExprApp(CUR_POS, new ExprApp(new 
ExprVar(data->symbols.create("__sub")), $1), $3); }
   | expr_op '*' expr_op { $$ = new ExprApp(CUR_POS, new ExprApp(new 
ExprVar(data->symbols.create("__mul")), $1), $3); }
   | expr_op '/' expr_op { $$ = new ExprApp(CUR_POS, new ExprApp(new 
ExprVar(data->symbols.create("__div")), $1), $3); }
@@ -461,13 +457,13 @@ attrs
   : attrs attr { $$ = $1; $1->push_back(AttrName(data->symbols.create($2))); }
   | attrs string_attr
     { $$ = $1;
-      ExprString *str = dynamic_cast<ExprString *>($2);
+      ExprString * str = dynamic_cast<ExprString *>($2);
       if (str) {
           $$->push_back(AttrName(str->s));
           delete str;
       } else
-        throw ParseError(format("dynamic attributes not allowed in inherit at 
%1%")
-            % makeCurPos(@2, data));
+          throw ParseError(format("dynamic attributes not allowed in inherit 
at %1%")
+              % makeCurPos(@2, data));
     }
   | { $$ = new AttrPath; }
   ;
@@ -476,12 +472,12 @@ attrpath
   : attrpath '.' attr { $$ = $1; 
$1->push_back(AttrName(data->symbols.create($3))); }
   | attrpath '.' string_attr
     { $$ = $1;
-      ExprString *str = dynamic_cast<ExprString *>($3);
+      ExprString * str = dynamic_cast<ExprString *>($3);
       if (str) {
           $$->push_back(AttrName(str->s));
           delete str;
       } else
-          $$->push_back(AttrName(static_cast<ExprConcatStrings *>($3)));
+          $$->push_back(AttrName($3));
     }
   | attr { $$ = new vector<AttrName>; 
$$->push_back(AttrName(data->symbols.create($1))); }
   | string_attr
@@ -491,7 +487,7 @@ attrpath
           $$->push_back(AttrName(str->s));
           delete str;
       } else
-          $$->push_back(AttrName(static_cast<ExprConcatStrings *>($1)));
+          $$->push_back(AttrName($1));
     }
   ;
 
@@ -502,7 +498,7 @@ attr
 
 string_attr
   : '"' string_parts '"' { $$ = $2; }
-  | DOLLAR_CURLY expr '}' { $$ = new ExprConcatStrings(CUR_POS, true, new 
vector<Expr*>(1, $2)); }
+  | DOLLAR_CURLY expr '}' { $$ = $2; }
   ;
 
 expr_list