store ExprConcatStrings elements as direct vector

storing a pointer only adds an unnecessary indirection and memory allocation.

Change-Id: If06dd05effdf1ccb0df0873580f50c775608925d

src/libexpr/eval.cc

@@ -2010,10 +2010,10 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
     };
 
     // List of returned strings. References to these Values must NOT be persisted.
-    SmallTemporaryValueVector<conservativeStackReservation> values(es->size());
+    SmallTemporaryValueVector<conservativeStackReservation> values(es.size());
     Value * vTmpP = values.data();
 
-    for (auto & [i_pos, i] : *es) {
+    for (auto & [i_pos, i] : es) {
         Value & vTmp = *vTmpP++;
         i->eval(state, env, vTmp);
 
@@ -2043,7 +2043,7 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
             } else
                 state.error<EvalError>("cannot add %1% to a float", showType(vTmp)).atPos(i_pos).withFrame(env, *this).debugThrow();
         } else {
-            if (s.empty()) s.reserve(es->size());
+            if (s.empty()) s.reserve(es.size());
             /* skip canonization of first path, which would only be not
             canonized in the first place if it's coming from a ./${foo} type
             path */

src/libexpr/nixexpr.cc

@@ -230,7 +230,7 @@ void ExprConcatStrings::show(const SymbolTable & symbols, std::ostream & str) co
 {
     bool first = true;
     str << "(";
-    for (auto & i : *es) {
+    for (auto & i : es) {
         if (first) first = false; else str << " + ";
         i.second->show(symbols, str);
     }
@@ -546,7 +546,7 @@ void ExprConcatStrings::bindVars(EvalState & es, const std::shared_ptr<const Sta
     if (es.debugRepl)
         es.exprEnvs.insert(std::make_pair(this, env));
 
-    for (auto & i : *this->es)
+    for (auto & i : this->es)
         i.second->bindVars(es, env);
 }
 

src/libexpr/nixexpr.hh

@@ -296,7 +296,7 @@ struct ExprCall : Expr
     std::vector<Expr *> args;
     PosIdx pos;
     ExprCall(const PosIdx & pos, Expr * fun, std::vector<Expr *> && args)
-        : fun(fun), args(args), pos(pos)
+        : fun(fun), args(std::move(args)), pos(pos)
     { }
     PosIdx getPos() const override { return pos; }
     COMMON_METHODS
@@ -378,9 +378,9 @@ struct ExprConcatStrings : Expr
 {
     PosIdx pos;
     bool forceString;
-    std::vector<std::pair<PosIdx, Expr *>> * es;
+    std::vector<std::pair<PosIdx, Expr *>> es;
-    ExprConcatStrings(const PosIdx & pos, bool forceString, std::vector<std::pair<PosIdx, Expr *>> * es)
+    ExprConcatStrings(const PosIdx & pos, bool forceString, std::vector<std::pair<PosIdx, Expr *>> es)
-        : pos(pos), forceString(forceString), es(es) { };
+        : pos(pos), forceString(forceString), es(std::move(es)) { };
     PosIdx getPos() const override { return pos; }
     COMMON_METHODS
 };

src/libexpr/parser-state.hh

@@ -216,7 +216,7 @@ inline Expr * ParserState::stripIndentation(const PosIdx pos,
     }
 
     /* Strip spaces from each line. */
-    auto * es2 = new std::vector<std::pair<PosIdx, Expr *>>;
+    std::vector<std::pair<PosIdx, Expr *>> es2;
     atStartOfLine = true;
     size_t curDropped = 0;
     size_t n = es.size();
@@ -224,7 +224,7 @@ inline Expr * ParserState::stripIndentation(const PosIdx pos,
     const auto trimExpr = [&] (Expr * e) {
         atStartOfLine = false;
         curDropped = 0;
-        es2->emplace_back(i->first, e);
+        es2.emplace_back(i->first, e);
     };
     const auto trimString = [&] (const StringToken & t) {
         std::string s2;
@@ -256,19 +256,17 @@ inline Expr * ParserState::stripIndentation(const PosIdx pos,
                 s2 = std::string(s2, 0, p + 1);
         }
 
-        es2->emplace_back(i->first, new ExprString(std::move(s2)));
+        es2.emplace_back(i->first, new ExprString(std::move(s2)));
     };
     for (; i != es.end(); ++i, --n) {
         std::visit(overloaded { trimExpr, trimString }, i->second);
     }
 
     /* If this is a single string, then don't do a concatenation. */
-    if (es2->size() == 1 && dynamic_cast<ExprString *>((*es2)[0].second)) {
+    if (es2.size() == 1 && dynamic_cast<ExprString *>(es2[0].second)) {
-        auto *const result = (*es2)[0].second;
+        return es2[0].second;
-        delete es2;
-        return result;
     }
-    return new ExprConcatStrings(pos, true, es2);
+    return new ExprConcatStrings(pos, true, std::move(es2));
 }
 
 inline PosIdx ParserState::at(const ParserLocation & loc)

src/libexpr/parser.y

@@ -209,7 +209,7 @@ expr_op
   | expr_op UPDATE expr_op { $$ = new ExprOpUpdate(state->at(@2), $1, $3); }
   | expr_op '?' attrpath { $$ = new ExprOpHasAttr($1, std::move(*$3)); delete $3; }
   | expr_op '+' expr_op
-    { $$ = new ExprConcatStrings(state->at(@2), false, new std::vector<std::pair<PosIdx, Expr *> >({{state->at(@1), $1}, {state->at(@3), $3}})); }
+    { $$ = new ExprConcatStrings(state->at(@2), false, {{state->at(@1), $1}, {state->at(@3), $3}}); }
   | expr_op '-' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->s.sub), {$1, $3}); }
   | expr_op '*' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->s.mul), {$1, $3}); }
   | expr_op '/' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->s.div), {$1, $3}); }
@@ -258,7 +258,8 @@ expr_simple
   | path_start PATH_END
   | path_start string_parts_interpolated PATH_END {
       $2->insert($2->begin(), {state->at(@1), $1});
-      $$ = new ExprConcatStrings(CUR_POS, false, $2);
+      $$ = new ExprConcatStrings(CUR_POS, false, std::move(*$2));
+      delete $2;
   }
   | SPATH {
       std::string path($1.p + 1, $1.l - 2);
@@ -290,7 +291,10 @@ expr_simple
 
 string_parts
   : STR { $$ = new ExprString(std::string($1)); }
-  | string_parts_interpolated { $$ = new ExprConcatStrings(CUR_POS, true, $1); }
+  | string_parts_interpolated
+    { $$ = new ExprConcatStrings(CUR_POS, true, std::move(*$1));
+      delete $1;
+    }
   | { $$ = new ExprString(""); }
   ;