nixos/nix
1
1
Fork 0
mirror of https://github.com/NixOS/nix.git synced 2025-11-08 11:36:03 +01:00
Code Activity

Inline only-used-once closures in ExprConcatStrings::eval

Browse source 
Refactor `ExprConcatStrings::eval` by inlining two only-called-once
closures into the call-site, so that the code is easier to reason about
locally (especially since the variables that were closed over were
mutated all over the place within this function).

Also use curly braces with each branch for consistency in the the
resulting code.

This is a pure refactor, but also arguably causes us to depend less on
the optimizer; now, we don't have to make sure that this closure is
inlined.
This commit is contained in:
Aspen Smith 2025-09-10 16:53:21 -04:00 • committed by John Ericson
parent ca9fde1b88
commit b67c2f1572
1 changed files with 19 additions and 27 deletions
Download patch file Download diff file Expand all files Collapse all files

46
src/libexpr/eval.cc
View file

@ -2029,27 +2029,6 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
bool first = !forceString; bool first = !forceString;
ValueType firstType = nString; ValueType firstType = nString;
const auto str = [&] {
std::string result;
result.reserve(sSize);
for (const auto & part : strings)
result += *part;
return result;
};
/* c_str() is not str().c_str() because we want to create a string
Value. allocating a GC'd string directly and moving it into a
Value lets us avoid an allocation and copy. */
const auto c_str = [&] {
char * result = allocString(sSize + 1);
char * tmp = result;
for (const auto & part : strings) {
memcpy(tmp, part->data(), part->size());
tmp += part->size();
}
*tmp = 0;
return result;
};
// 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();
@ -2111,19 +2090,32 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
first = false; first = false;
} }
if (firstType == nInt) if (firstType == nInt) {
v.mkInt(n); v.mkInt(n);
else if (firstType == nFloat) } else if (firstType == nFloat) {
v.mkFloat(nf); v.mkFloat(nf);
else if (firstType == nPath) { } else if (firstType == nPath) {
if (!context.empty()) if (!context.empty())
state.error<EvalError>("a string that refers to a store path cannot be appended to a path") state.error<EvalError>("a string that refers to a store path cannot be appended to a path")
.atPos(pos) .atPos(pos)
.withFrame(env, *this) .withFrame(env, *this)
.debugThrow(); .debugThrow();
v.mkPath(state.rootPath(CanonPath(str()))); std::string result_str;
} else result_str.reserve(sSize);
v.mkStringMove(c_str(), context); for (const auto & part : strings) {
result_str += *part;
}
v.mkPath(state.rootPath(CanonPath(result_str)));
} else {
char * result_str = allocString(sSize + 1);
char * tmp = result_str;
for (const auto & part : strings) {
memcpy(tmp, part->data(), part->size());
tmp += part->size();
}
*tmp = 0;
v.mkStringMove(result_str, context);
}
} }
void ExprPos::eval(EvalState & state, Env & env, Value & v) void ExprPos::eval(EvalState & state, Env & env, Value & v)