store ExprConcatStrings elements as direct vector

storing a pointer only adds an unnecessary indirection and memory allocation.
This commit is contained in:
pennae 2024-02-11 12:09:07 +01:00
parent 6bcdd2c39a
commit aa4c8695ea
5 changed files with 22 additions and 20 deletions

View file

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

View file

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

View file

@ -297,7 +297,7 @@ struct ExprCall : Expr
std::vector<Expr *> args; std::vector<Expr *> args;
PosIdx pos; PosIdx pos;
ExprCall(const PosIdx & pos, Expr * fun, std::vector<Expr *> && args) 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; } PosIdx getPos() const override { return pos; }
COMMON_METHODS COMMON_METHODS
@ -379,9 +379,9 @@ struct ExprConcatStrings : Expr
{ {
PosIdx pos; PosIdx pos;
bool forceString; 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; } PosIdx getPos() const override { return pos; }
COMMON_METHODS COMMON_METHODS
}; };

View file

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

View file

@ -211,7 +211,7 @@ expr_op
| expr_op UPDATE expr_op { $$ = new ExprOpUpdate(state->at(@2), $1, $3); } | 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 '?' attrpath { $$ = new ExprOpHasAttr($1, std::move(*$3)); delete $3; }
| expr_op '+' expr_op | 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.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.mul), {$1, $3}); }
| expr_op '/' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->s.div), {$1, $3}); } | expr_op '/' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->s.div), {$1, $3}); }
@ -260,7 +260,8 @@ expr_simple
| path_start PATH_END | path_start PATH_END
| path_start string_parts_interpolated PATH_END { | path_start string_parts_interpolated PATH_END {
$2->insert($2->begin(), {state->at(@1), $1}); $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 { | SPATH {
std::string path($1.p + 1, $1.l - 2); std::string path($1.p + 1, $1.l - 2);
@ -292,7 +293,10 @@ expr_simple
string_parts string_parts
: STR { $$ = new ExprString(std::string($1)); } : 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(""); } | { $$ = new ExprString(""); }
; ;