Skip to content

Commit 9e79e83

Browse files
Ericson2314Ericson2314
authored
Merge pull request #14384 from Radvendii/exprlambda-alloc
libexpr: store ExprLambda data in Expr::alloc
2 parents 937a6df + 67be2df commit 9e79e83

File tree

14 files changed

+145
-84
lines changed

14 files changed

+145
-84
lines changed

‎src/libexpr-tests/primops.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,7 @@ TEST_F(PrimOpTest, derivation)
771771
ASSERT_EQ(v.type(), nFunction);
772772
ASSERT_TRUE(v.isLambda());
773773
ASSERT_NE(v.lambda().fun, nullptr);
774-
ASSERT_TRUE(v.lambda().fun->hasFormals());
774+
ASSERT_TRUE(v.lambda().fun->getFormals());
775775
}
776776

777777
TEST_F(PrimOpTest, currentTime)

‎src/libexpr-tests/value/print.cc‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,8 @@ TEST_F(ValuePrintingTests, vLambda)
110110
PosTable::Origin origin = state.positions.addOrigin(std::monostate(), 1);
111111
auto posIdx = state.positions.add(origin, 0);
112112
auto body = ExprInt(0);
113-
auto formals = Formals{};
114113

115-
ExprLambda eLambda(posIdx, createSymbol("a"), &formals, &body);
114+
ExprLambda eLambda(posIdx, createSymbol("a"), &body);
116115

117116
Value vLambda;
118117
vLambda.mkLambda(&env, &eLambda);
@@ -500,9 +499,8 @@ TEST_F(ValuePrintingTests, ansiColorsLambda)
500499
PosTable::Origin origin = state.positions.addOrigin(std::monostate(), 1);
501500
auto posIdx = state.positions.add(origin, 0);
502501
auto body = ExprInt(0);
503-
auto formals = Formals{};
504502

505-
ExprLambda eLambda(posIdx, createSymbol("a"), &formals, &body);
503+
ExprLambda eLambda(posIdx, createSymbol("a"), &body);
506504

507505
Value vLambda;
508506
vLambda.mkLambda(&env, &eLambda);

‎src/libexpr/eval.cc‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1496,15 +1496,13 @@ void EvalState::callFunction(Value & fun, std::span<Value *> args, Value & vRes,
14961496

14971497
ExprLambda & lambda(*vCur.lambda().fun);
14981498

1499-
auto size = (!lambda.arg ? 0 : 1) + (lambda.hasFormals() ? lambda.formals->formals.size() : 0);
1499+
auto size = (!lambda.arg ? 0 : 1) + (lambda.getFormals() ? lambda.getFormals()->formals.size() : 0);
15001500
Env & env2(mem.allocEnv(size));
15011501
env2.up = vCur.lambda().env;
15021502

15031503
Displacement displ = 0;
15041504

1505-
if (!lambda.hasFormals())
1506-
env2.values[displ++] = args[0];
1507-
else {
1505+
if (auto formals = lambda.getFormals()) {
15081506
try {
15091507
forceAttrs(*args[0], lambda.pos, "while evaluating the value passed for the lambda argument");
15101508
} catch (Error & e) {
@@ -1520,7 +1518,7 @@ void EvalState::callFunction(Value & fun, std::span<Value *> args, Value & vRes,
15201518
there is no matching actual argument but the formal
15211519
argument has a default, use the default. */
15221520
size_t attrsUsed = 0;
1523-
for (auto & i : lambda.formals->formals) {
1521+
for (auto & i : formals->formals) {
15241522
auto j = args[0]->attrs()->get(i.name);
15251523
if (!j) {
15261524
if (!i.def) {
@@ -1542,13 +1540,13 @@ void EvalState::callFunction(Value & fun, std::span<Value *> args, Value & vRes,
15421540

15431541
/* Check that each actual argument is listed as a formal
15441542
argument (unless the attribute match specifies a `...'). */
1545-
if (!lambda.formals->ellipsis && attrsUsed != args[0]->attrs()->size()) {
1543+
if (!formals->ellipsis && attrsUsed != args[0]->attrs()->size()) {
15461544
/* Nope, so show the first unexpected argument to the
15471545
user. */
15481546
for (auto & i : *args[0]->attrs())
1549-
if (!lambda.formals->has(i.name)) {
1547+
if (!formals->has(i.name)) {
15501548
StringSet formalNames;
1551-
for (auto & formal : lambda.formals->formals)
1549+
for (auto & formal : formals->formals)
15521550
formalNames.insert(std::string(symbols[formal.name]));
15531551
auto suggestions = Suggestions::bestMatches(formalNames, symbols[i.name]);
15541552
error<TypeError>(
@@ -1563,6 +1561,8 @@ void EvalState::callFunction(Value & fun, std::span<Value *> args, Value & vRes,
15631561
}
15641562
unreachable();
15651563
}
1564+
} else {
1565+
env2.values[displ++] = args[0];
15661566
}
15671567

15681568
nrFunctionCalls++;
@@ -1747,22 +1747,23 @@ void EvalState::autoCallFunction(const Bindings & args, Value & fun, Value & res
17471747
}
17481748
}
17491749

1750-
if (!fun.isLambda() || !fun.lambda().fun->hasFormals()) {
1750+
if (!fun.isLambda() || !fun.lambda().fun->getFormals()) {
17511751
res = fun;
17521752
return;
17531753
}
1754+
auto formals = fun.lambda().fun->getFormals();
17541755

1755-
auto attrs = buildBindings(std::max(static_cast<uint32_t>(fun.lambda().fun->formals->formals.size()), args.size()));
1756+
auto attrs = buildBindings(std::max(static_cast<uint32_t>(formals->formals.size()), args.size()));
17561757

1757-
if (fun.lambda().fun->formals->ellipsis) {
1758+
if (formals->ellipsis) {
17581759
// If the formals have an ellipsis (eg the function accepts extra args) pass
17591760
// all available automatic arguments (which includes arguments specified on
17601761
// the command line via --arg/--argstr)
17611762
for (auto & v : args)
17621763
attrs.insert(v);
17631764
} else {
17641765
// Otherwise, only pass the arguments that the function accepts
1765-
for (auto & i : fun.lambda().fun->formals->formals) {
1766+
for (auto & i : formals->formals) {
17661767
auto j = args.get(i.name);
17671768
if (j) {
17681769
attrs.insert(*j);

‎src/libexpr/include/nix/expr/nixexpr.hh‎

Lines changed: 58 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ struct Formal
466466
Expr * def;
467467
};
468468

469-
struct Formals
469+
struct FormalsBuilder
470470
{
471471
typedef std::vector<Formal> Formals_;
472472
/**
@@ -481,6 +481,23 @@ struct Formals
481481
formals.begin(), formals.end(), arg, [](const Formal & f, const Symbol & sym) { return f.name < sym; });
482482
return it != formals.end() && it->name == arg;
483483
}
484+
};
485+
486+
struct Formals
487+
{
488+
std::span<Formal> formals;
489+
bool ellipsis;
490+
491+
Formals(std::span<Formal> formals, bool ellipsis)
492+
: formals(formals)
493+
, ellipsis(ellipsis) {};
494+
495+
bool has(Symbol arg) const
496+
{
497+
auto it = std::lower_bound(
498+
formals.begin(), formals.end(), arg, [](const Formal & f, const Symbol & sym) { return f.name < sym; });
499+
return it != formals.end() && it->name == arg;
500+
}
484501

485502
std::vector<Formal> lexicographicOrder(const SymbolTable & symbols) const
486503
{
@@ -498,31 +515,57 @@ struct ExprLambda : Expr
498515
PosIdx pos;
499516
Symbol name;
500517
Symbol arg;
501-
Formals * formals;
518+
519+
private:
520+
bool hasFormals;
521+
bool ellipsis;
522+
uint16_t nFormals;
523+
Formal * formalsStart;
524+
public:
525+
526+
std::optional<Formals> getFormals() const
527+
{
528+
if (hasFormals)
529+
return Formals{{formalsStart, nFormals}, ellipsis};
530+
else
531+
return std::nullopt;
532+
}
533+
502534
Expr * body;
503535
DocComment docComment;
504536

505-
ExprLambda(PosIdx pos, Symbol arg, Formals * formals, Expr * body)
537+
ExprLambda(
538+
std::pmr::polymorphic_allocator<char> & alloc,
539+
PosIdx pos,
540+
Symbol arg,
541+
const FormalsBuilder & formals,
542+
Expr * body)
506543
: pos(pos)
507544
, arg(arg)
508-
, formals(formals)
509-
, body(body) {};
510-
511-
ExprLambda(PosIdx pos, Formals * formals, Expr * body)
512-
: pos(pos)
513-
, formals(formals)
545+
, hasFormals(true)
546+
, ellipsis(formals.ellipsis)
547+
, nFormals(formals.formals.size())
548+
, formalsStart(alloc.allocate_object<Formal>(nFormals))
514549
, body(body)
515550
{
516-
}
551+
std::ranges::copy(formals.formals, formalsStart);
552+
};
553+
554+
ExprLambda(PosIdx pos, Symbol arg, Expr * body)
555+
: pos(pos)
556+
, arg(arg)
557+
, hasFormals(false)
558+
, ellipsis(false)
559+
, nFormals(0)
560+
, formalsStart(nullptr)
561+
, body(body) {};
562+
563+
ExprLambda(std::pmr::polymorphic_allocator<char> & alloc, PosIdx pos, FormalsBuilder formals, Expr * body)
564+
: ExprLambda(alloc, pos, Symbol(), formals, body) {};
517565

518566
void setName(Symbol name) override;
519567
std::string showNamePos(const EvalState & state) const;
520568

521-
inline bool hasFormals() const
522-
{
523-
return formals != nullptr;
524-
}
525-
526569
PosIdx getPos() const override
527570
{
528571
return pos;

‎src/libexpr/include/nix/expr/parser-state.hh‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ struct ParserState
9393
void addAttr(
9494
ExprAttrs * attrs, AttrPath && attrPath, const ParserLocation & loc, Expr * e, const ParserLocation & exprLoc);
9595
void addAttr(ExprAttrs * attrs, AttrPath & attrPath, const Symbol & symbol, ExprAttrs::AttrDef && def);
96-
Formals * validateFormals(Formals * formals, PosIdx pos = noPos, Symbol arg = {});
96+
void validateFormals(FormalsBuilder & formals, PosIdx pos = noPos, Symbol arg = {});
9797
Expr * stripIndentation(const PosIdx pos, std::vector<std::pair<PosIdx, std::variant<Expr *, StringToken>>> && es);
9898
PosIdx at(const ParserLocation & loc);
9999
};
@@ -213,29 +213,27 @@ ParserState::addAttr(ExprAttrs * attrs, AttrPath & attrPath, const Symbol & symb
213213
}
214214
}
215215

216-
inline Formals * ParserState::validateFormals(Formals * formals, PosIdx pos, Symbol arg)
216+
inline void ParserState::validateFormals(FormalsBuilder & formals, PosIdx pos, Symbol arg)
217217
{
218-
std::sort(formals->formals.begin(), formals->formals.end(), [](const auto & a, const auto & b) {
218+
std::sort(formals.formals.begin(), formals.formals.end(), [](const auto & a, const auto & b) {
219219
return std::tie(a.name, a.pos) < std::tie(b.name, b.pos);
220220
});
221221

222222
std::optional<std::pair<Symbol, PosIdx>> duplicate;
223-
for (size_t i = 0; i + 1 < formals->formals.size(); i++) {
224-
if (formals->formals[i].name != formals->formals[i + 1].name)
223+
for (size_t i = 0; i + 1 < formals.formals.size(); i++) {
224+
if (formals.formals[i].name != formals.formals[i + 1].name)
225225
continue;
226-
std::pair thisDup{formals->formals[i].name, formals->formals[i + 1].pos};
226+
std::pair thisDup{formals.formals[i].name, formals.formals[i + 1].pos};
227227
duplicate = std::min(thisDup, duplicate.value_or(thisDup));
228228
}
229229
if (duplicate)
230230
throw ParseError(
231231
{.msg = HintFmt("duplicate formal function argument '%1%'", symbols[duplicate->first]),
232232
.pos = positions[duplicate->second]});
233233

234-
if (arg && formals->has(arg))
234+
if (arg && formals.has(arg))
235235
throw ParseError(
236236
{.msg = HintFmt("duplicate formal function argument '%1%'", symbols[arg]), .pos = positions[pos]});
237-
238-
return formals;
239237
}
240238

241239
inline Expr *

‎src/libexpr/nixexpr.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ void ExprList::show(const SymbolTable & symbols, std::ostream & str) const
154154
void ExprLambda::show(const SymbolTable & symbols, std::ostream & str) const
155155
{
156156
str << "(";
157-
if (hasFormals()) {
157+
if (auto formals = getFormals()) {
158158
str << "{ ";
159159
bool first = true;
160160
// the natural Symbol ordering is by creation time, which can lead to the
@@ -171,7 +171,7 @@ void ExprLambda::show(const SymbolTable & symbols, std::ostream & str) const
171171
i.def->show(symbols, str);
172172
}
173173
}
174-
if (formals->ellipsis) {
174+
if (ellipsis) {
175175
if (!first)
176176
str << ", ";
177177
str << "...";
@@ -452,14 +452,14 @@ void ExprLambda::bindVars(EvalState & es, const std::shared_ptr<const StaticEnv>
452452
es.exprEnvs.insert(std::make_pair(this, env));
453453

454454
auto newEnv =
455-
std::make_shared<StaticEnv>(nullptr, env, (hasFormals() ? formals->formals.size() : 0) + (!arg ? 0 : 1));
455+
std::make_shared<StaticEnv>(nullptr, env, (getFormals() ? getFormals()->formals.size() : 0) + (!arg ? 0 : 1));
456456

457457
Displacement displ = 0;
458458

459459
if (arg)
460460
newEnv->vars.emplace_back(arg, displ++);
461461

462-
if (hasFormals()) {
462+
if (auto formals = getFormals()) {
463463
for (auto & i : formals->formals)
464464
newEnv->vars.emplace_back(i.name, displ++);
465465

‎src/libexpr/parser.y‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ static Expr * makeCall(PosIdx pos, Expr * fn, Expr * arg) {
131131
%type <nix::Expr *> expr_pipe_from expr_pipe_into
132132
%type <std::vector<Expr *>> list
133133
%type <nix::ExprAttrs *> binds binds1
134-
%type <nix::Formals *> formals formal_set
134+
%type <nix::FormalsBuilder> formals formal_set
135135
%type <nix::Formal> formal
136136
%type <std::vector<nix::AttrName>> attrpath
137137
%type <std::vector<std::pair<nix::AttrName, nix::PosIdx>>> attrs
@@ -179,26 +179,30 @@ expr: expr_function;
179179

180180
expr_function
181181
: ID ':' expr_function
182-
{ auto me = new ExprLambda(CUR_POS, state->symbols.create($1), 0, $3);
182+
{ auto me = new ExprLambda(CUR_POS, state->symbols.create($1), $3);
183183
$$ = me;
184184
SET_DOC_POS(me, @1);
185185
}
186186
| formal_set ':' expr_function[body]
187-
{ auto me = new ExprLambda(CUR_POS, state->validateFormals($formal_set), $body);
187+
{
188+
state->validateFormals($formal_set);
189+
auto me = new ExprLambda(state->alloc, CUR_POS, std::move($formal_set), $body);
188190
$$ = me;
189191
SET_DOC_POS(me, @1);
190192
}
191193
| formal_set '@' ID ':' expr_function[body]
192194
{
193195
auto arg = state->symbols.create($ID);
194-
auto me = new ExprLambda(CUR_POS, arg, state->validateFormals($formal_set, CUR_POS, arg), $body);
196+
state->validateFormals($formal_set, CUR_POS, arg);
197+
auto me = new ExprLambda(state->alloc, CUR_POS, arg, std::move($formal_set), $body);
195198
$$ = me;
196199
SET_DOC_POS(me, @1);
197200
}
198201
| ID '@' formal_set ':' expr_function[body]
199202
{
200203
auto arg = state->symbols.create($ID);
201-
auto me = new ExprLambda(CUR_POS, arg, state->validateFormals($formal_set, CUR_POS, arg), $body);
204+
state->validateFormals($formal_set, CUR_POS, arg);
205+
auto me = new ExprLambda(state->alloc, CUR_POS, arg, std::move($formal_set), $body);
202206
$$ = me;
203207
SET_DOC_POS(me, @1);
204208
}
@@ -490,18 +494,18 @@ list
490494
;
491495

492496
formal_set
493-
: '{' formals ',' ELLIPSIS '}' { $$ = $formals; $$->ellipsis = true; }
494-
| '{' ELLIPSIS '}' { $$ = new Formals; $$->ellipsis = true; }
495-
| '{' formals ',' '}' { $$ = $formals; $$->ellipsis = false; }
496-
| '{' formals '}' { $$ = $formals; $$->ellipsis = false; }
497-
| '{' '}' { $$ = new Formals; $$->ellipsis = false; }
497+
: '{' formals ',' ELLIPSIS '}' { $$ = std::move($formals); $$.ellipsis = true; }
498+
| '{' ELLIPSIS '}' { $$.ellipsis = true; }
499+
| '{' formals ',' '}' { $$ = std::move($formals); $$.ellipsis = false; }
500+
| '{' formals '}' { $$ = std::move($formals); $$.ellipsis = false; }
501+
| '{' '}' { $$.ellipsis = false; }
498502
;
499503

500504
formals
501505
: formals[accum] ',' formal
502-
{ $$ = $accum; $$->formals.emplace_back(std::move($formal)); }
506+
{ $$ = std::move($accum); $$.formals.emplace_back(std::move($formal)); }
503507
| formal
504-
{ $$ = new Formals; $$->formals.emplace_back(std::move($formal)); }
508+
{ $$.formals.emplace_back(std::move($formal)); }
505509
;
506510

507511
formal

‎src/libexpr/primops.cc‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3363,21 +3363,20 @@ static void prim_functionArgs(EvalState & state, const PosIdx pos, Value ** args
33633363
if (!args[0]->isLambda())
33643364
state.error<TypeError>("'functionArgs' requires a function").atPos(pos).debugThrow();
33653365

3366-
if (!args[0]->lambda().fun->hasFormals()) {
3366+
if (const auto & formals = args[0]->lambda().fun->getFormals()) {
3367+
auto attrs = state.buildBindings(formals->formals.size());
3368+
for (auto & i : formals->formals)
3369+
attrs.insert(i.name, state.getBool(i.def), i.pos);
3370+
/* Optimization: avoid sorting bindings. `formals` must already be sorted according to
3371+
(std::tie(a.name, a.pos) < std::tie(b.name, b.pos)) predicate, so the following assertion
3372+
always holds:
3373+
assert(std::is_sorted(attrs.alreadySorted()->begin(), attrs.alreadySorted()->end()));
3374+
.*/
3375+
v.mkAttrs(attrs.alreadySorted());
3376+
} else {
33673377
v.mkAttrs(&Bindings::emptyBindings);
33683378
return;
33693379
}
3370-
3371-
const auto & formals = args[0]->lambda().fun->formals->formals;
3372-
auto attrs = state.buildBindings(formals.size());
3373-
for (auto & i : formals)
3374-
attrs.insert(i.name, state.getBool(i.def), i.pos);
3375-
/* Optimization: avoid sorting bindings. `formals` must already be sorted according to
3376-
(std::tie(a.name, a.pos) < std::tie(b.name, b.pos)) predicate, so the following assertion
3377-
always holds:
3378-
assert(std::is_sorted(attrs.alreadySorted()->begin(), attrs.alreadySorted()->end()));
3379-
.*/
3380-
v.mkAttrs(attrs.alreadySorted());
33813380
}
33823381

33833382
static RegisterPrimOp primop_functionArgs({

0 commit comments

Comments
 (0)