Skip to content

Commit 4b97f11

Browse files
RadvendiiRadvendii
committed
libexpr: ExprAttrs::attrs and ExprAttrs::dynamicAttrs -> std::optional
without this, there is no way to swap them out for structures using a different allocator. This should be reverted as part of redesiging ExprAttrs to use an ExprAttrsBuilder
1 parent 614e143 commit 4b97f11

File tree

5 files changed

+61
-50
lines changed

5 files changed

+61
-50
lines changed

‎src/libexpr/eval.cc‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ bool Value::isTrivial() const
205205
{
206206
return !isa<tApp, tPrimOpApp>()
207207
&& (!isa<tThunk>()
208-
|| (dynamic_cast<ExprAttrs *>(thunk().expr) && ((ExprAttrs *) thunk().expr)->dynamicAttrs.empty())
208+
|| (dynamic_cast<ExprAttrs *>(thunk().expr) && ((ExprAttrs *) thunk().expr)->dynamicAttrs->empty())
209209
|| dynamic_cast<ExprLambda *>(thunk().expr) || dynamic_cast<ExprList *>(thunk().expr));
210210
}
211211

@@ -1226,26 +1226,26 @@ Env * ExprAttrs::buildInheritFromEnv(EvalState & state, Env & up)
12261226

12271227
void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
12281228
{
1229-
auto bindings = state.buildBindings(attrs.size() + dynamicAttrs.size());
1229+
auto bindings = state.buildBindings(attrs->size() + dynamicAttrs->size());
12301230
auto dynamicEnv = &env;
12311231
bool sort = false;
12321232

12331233
if (recursive) {
12341234
/* Create a new environment that contains the attributes in
12351235
this `rec'. */
1236-
Env & env2(state.mem.allocEnv(attrs.size()));
1236+
Env & env2(state.mem.allocEnv(attrs->size()));
12371237
env2.up = &env;
12381238
dynamicEnv = &env2;
12391239
Env * inheritEnv = inheritFromExprs ? buildInheritFromEnv(state, env2) : nullptr;
12401240

1241-
AttrDefs::iterator overrides = attrs.find(state.s.overrides);
1242-
bool hasOverrides = overrides != attrs.end();
1241+
AttrDefs::iterator overrides = attrs->find(state.s.overrides);
1242+
bool hasOverrides = overrides != attrs->end();
12431243

12441244
/* The recursive attributes are evaluated in the new
12451245
environment, while the inherited attributes are evaluated
12461246
in the original environment. */
12471247
Displacement displ = 0;
1248-
for (auto & i : attrs) {
1248+
for (auto & i : *attrs) {
12491249
Value * vAttr;
12501250
if (hasOverrides && i.second.kind != AttrDef::Kind::Inherited) {
12511251
vAttr = state.allocValue();
@@ -1272,8 +1272,8 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
12721272
"while evaluating the `__overrides` attribute");
12731273
bindings.grow(state.buildBindings(bindings.capacity() + vOverrides->attrs()->size()));
12741274
for (auto & i : *vOverrides->attrs()) {
1275-
AttrDefs::iterator j = attrs.find(i.name);
1276-
if (j != attrs.end()) {
1275+
AttrDefs::iterator j = attrs->find(i.name);
1276+
if (j != attrs->end()) {
12771277
(*bindings.bindings)[j->second.displ] = i;
12781278
env2.values[j->second.displ] = i.value;
12791279
} else
@@ -1285,13 +1285,13 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
12851285

12861286
else {
12871287
Env * inheritEnv = inheritFromExprs ? buildInheritFromEnv(state, env) : nullptr;
1288-
for (auto & i : attrs)
1288+
for (auto & i : *attrs)
12891289
bindings.insert(
12901290
i.first, i.second.e->maybeThunk(state, *i.second.chooseByKind(&env, &env, inheritEnv)), i.second.pos);
12911291
}
12921292

12931293
/* Dynamic attrs apply *after* rec and __overrides. */
1294-
for (auto & i : dynamicAttrs) {
1294+
for (auto & i : *dynamicAttrs) {
12951295
Value nameVal;
12961296
i.nameExpr->eval(state, *dynamicEnv, nameVal);
12971297
state.forceValue(nameVal, i.pos);
@@ -1325,7 +1325,7 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v)
13251325
{
13261326
/* Create a new environment that contains the attributes in this
13271327
`let'. */
1328-
Env & env2(state.mem.allocEnv(attrs->attrs.size()));
1328+
Env & env2(state.mem.allocEnv(attrs->attrs->size()));
13291329
env2.up = &env;
13301330

13311331
Env * inheritEnv = attrs->inheritFromExprs ? attrs->buildInheritFromEnv(state, env2) : nullptr;
@@ -1334,7 +1334,7 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v)
13341334
while the inherited attributes are evaluated in the original
13351335
environment. */
13361336
Displacement displ = 0;
1337-
for (auto & i : attrs->attrs) {
1337+
for (auto & i : *attrs->attrs) {
13381338
env2.values[displ++] = i.second.e->maybeThunk(state, *i.second.chooseByKind(&env2, &env, inheritEnv));
13391339
}
13401340

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,11 @@ struct ExprAttrs : Expr
396396
};
397397

398398
typedef std::pmr::map<Symbol, AttrDef> AttrDefs;
399-
AttrDefs attrs;
399+
/**
400+
* attrs will never be null. we use std::optional so that we can call emplace() to re-initialize the value with a
401+
* new pmr::map using a different allocator (move assignment will copy into the old allocator)
402+
*/
403+
std::optional<AttrDefs> attrs;
400404
std::unique_ptr<std::pmr::vector<Expr *>> inheritFromExprs;
401405

402406
struct DynamicAttrDef
@@ -410,12 +414,19 @@ struct ExprAttrs : Expr
410414
};
411415

412416
typedef std::pmr::vector<DynamicAttrDef> DynamicAttrDefs;
413-
DynamicAttrDefs dynamicAttrs;
417+
/**
418+
* dynamicAttrs will never be null. See comment on AttrDefs above.
419+
*/
420+
std::optional<DynamicAttrDefs> dynamicAttrs;
414421
ExprAttrs(const PosIdx & pos)
415422
: recursive(false)
416-
, pos(pos) {};
423+
, pos(pos)
424+
, attrs(AttrDefs{})
425+
, dynamicAttrs(DynamicAttrDefs{}) {};
417426
ExprAttrs()
418-
: recursive(false) {};
427+
: recursive(false)
428+
, attrs(AttrDefs{})
429+
, dynamicAttrs(DynamicAttrDefs{}) {};
419430

420431
PosIdx getPos() const override
421432
{

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

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -126,20 +126,20 @@ inline void ParserState::addAttr(
126126
for (i = attrPath.begin(); i + 1 < attrPath.end(); i++) {
127127
ExprAttrs * nested;
128128
if (i->symbol) {
129-
ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(i->symbol);
130-
if (j != attrs->attrs.end()) {
129+
ExprAttrs::AttrDefs::iterator j = attrs->attrs->find(i->symbol);
130+
if (j != attrs->attrs->end()) {
131131
nested = dynamic_cast<ExprAttrs *>(j->second.e);
132132
if (!nested) {
133133
attrPath.erase(i + 1, attrPath.end());
134134
dupAttr(attrPath, pos, j->second.pos);
135135
}
136136
} else {
137137
nested = exprs.add<ExprAttrs>();
138-
attrs->attrs[i->symbol] = ExprAttrs::AttrDef(nested, pos);
138+
(*attrs->attrs)[i->symbol] = ExprAttrs::AttrDef(nested, pos);
139139
}
140140
} else {
141141
nested = exprs.add<ExprAttrs>();
142-
attrs->dynamicAttrs.push_back(ExprAttrs::DynamicAttrDef(i->expr, nested, pos));
142+
attrs->dynamicAttrs->push_back(ExprAttrs::DynamicAttrDef(i->expr, nested, pos));
143143
}
144144
attrs = nested;
145145
}
@@ -148,7 +148,7 @@ inline void ParserState::addAttr(
148148
if (i->symbol) {
149149
addAttr(attrs, attrPath, i->symbol, ExprAttrs::AttrDef(e, pos));
150150
} else {
151-
attrs->dynamicAttrs.push_back(ExprAttrs::DynamicAttrDef(i->expr, e, pos));
151+
attrs->dynamicAttrs->push_back(ExprAttrs::DynamicAttrDef(i->expr, e, pos));
152152
}
153153

154154
auto it = lexerState.positionToDocComment.find(pos);
@@ -165,8 +165,8 @@ inline void ParserState::addAttr(
165165
inline void
166166
ParserState::addAttr(ExprAttrs * attrs, AttrPath & attrPath, const Symbol & symbol, ExprAttrs::AttrDef && def)
167167
{
168-
ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(symbol);
169-
if (j != attrs->attrs.end()) {
168+
ExprAttrs::AttrDefs::iterator j = attrs->attrs->find(symbol);
169+
if (j != attrs->attrs->end()) {
170170
// This attr path is already defined. However, if both
171171
// e and the expr pointed by the attr path are two attribute sets,
172172
// we want to merge them.
@@ -182,7 +182,7 @@ ParserState::addAttr(ExprAttrs * attrs, AttrPath & attrPath, const Symbol & symb
182182
if (jAttrs && ae) {
183183
if (ae->inheritFromExprs && !jAttrs->inheritFromExprs)
184184
jAttrs->inheritFromExprs = std::make_unique<std::pmr::vector<Expr *>>();
185-
for (auto & ad : ae->attrs) {
185+
for (auto & ad : *ae->attrs) {
186186
if (ad.second.kind == ExprAttrs::AttrDef::Kind::InheritedFrom) {
187187
auto & sel = dynamic_cast<ExprSelect &>(*ad.second.e);
188188
auto & from = dynamic_cast<ExprInheritFrom &>(*sel.e);
@@ -192,12 +192,12 @@ ParserState::addAttr(ExprAttrs * attrs, AttrPath & attrPath, const Symbol & symb
192192
addAttr(jAttrs, attrPath, ad.first, std::move(ad.second));
193193
attrPath.pop_back();
194194
}
195-
ae->attrs.clear();
196-
jAttrs->dynamicAttrs.insert(
197-
jAttrs->dynamicAttrs.end(),
198-
std::make_move_iterator(ae->dynamicAttrs.begin()),
199-
std::make_move_iterator(ae->dynamicAttrs.end()));
200-
ae->dynamicAttrs.clear();
195+
ae->attrs->clear();
196+
jAttrs->dynamicAttrs->insert(
197+
jAttrs->dynamicAttrs->end(),
198+
std::make_move_iterator(ae->dynamicAttrs->begin()),
199+
std::make_move_iterator(ae->dynamicAttrs->end()));
200+
ae->dynamicAttrs->clear();
201201
if (ae->inheritFromExprs) {
202202
jAttrs->inheritFromExprs->insert(
203203
jAttrs->inheritFromExprs->end(),
@@ -210,7 +210,7 @@ ParserState::addAttr(ExprAttrs * attrs, AttrPath & attrPath, const Symbol & symb
210210
}
211211
} else {
212212
// This attr path is not defined. Let's create it.
213-
attrs->attrs.emplace(symbol, def);
213+
attrs->attrs->emplace(symbol, def);
214214
def.e->setName(symbol);
215215
}
216216
}

‎src/libexpr/nixexpr.cc‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,9 @@ void ExprOpHasAttr::show(const SymbolTable & symbols, std::ostream & str) const
7474

7575
void ExprAttrs::showBindings(const SymbolTable & symbols, std::ostream & str) const
7676
{
77-
typedef const decltype(attrs)::value_type * Attr;
77+
typedef const AttrDefs::value_type * Attr;
7878
std::vector<Attr> sorted;
79-
for (auto & i : attrs)
79+
for (auto & i : *attrs)
8080
sorted.push_back(&i);
8181
std::sort(sorted.begin(), sorted.end(), [&](Attr a, Attr b) {
8282
std::string_view sa = symbols[a->first], sb = symbols[b->first];
@@ -122,7 +122,7 @@ void ExprAttrs::showBindings(const SymbolTable & symbols, std::ostream & str) co
122122
str << "; ";
123123
}
124124
}
125-
for (auto & i : dynamicAttrs) {
125+
for (auto & i : *dynamicAttrs) {
126126
str << "\"${";
127127
i.nameExpr->show(symbols, str);
128128
str << "}\" = ";
@@ -406,31 +406,31 @@ void ExprAttrs::bindVars(EvalState & es, const std::shared_ptr<const StaticEnv>
406406

407407
if (recursive) {
408408
auto newEnv = [&]() -> std::shared_ptr<const StaticEnv> {
409-
auto newEnv = std::make_shared<StaticEnv>(nullptr, env, attrs.size());
409+
auto newEnv = std::make_shared<StaticEnv>(nullptr, env, attrs->size());
410410

411411
Displacement displ = 0;
412-
for (auto & i : attrs)
412+
for (auto & i : *attrs)
413413
newEnv->vars.emplace_back(i.first, i.second.displ = displ++);
414414
return newEnv;
415415
}();
416416

417417
// No need to sort newEnv since attrs is in sorted order.
418418

419419
auto inheritFromEnv = bindInheritSources(es, newEnv);
420-
for (auto & i : attrs)
420+
for (auto & i : *attrs)
421421
i.second.e->bindVars(es, i.second.chooseByKind(newEnv, env, inheritFromEnv));
422422

423-
for (auto & i : dynamicAttrs) {
423+
for (auto & i : *dynamicAttrs) {
424424
i.nameExpr->bindVars(es, newEnv);
425425
i.valueExpr->bindVars(es, newEnv);
426426
}
427427
} else {
428428
auto inheritFromEnv = bindInheritSources(es, env);
429429

430-
for (auto & i : attrs)
430+
for (auto & i : *attrs)
431431
i.second.e->bindVars(es, i.second.chooseByKind(env, env, inheritFromEnv));
432432

433-
for (auto & i : dynamicAttrs) {
433+
for (auto & i : *dynamicAttrs) {
434434
i.nameExpr->bindVars(es, env);
435435
i.valueExpr->bindVars(es, env);
436436
}
@@ -486,18 +486,18 @@ void ExprCall::bindVars(EvalState & es, const std::shared_ptr<const StaticEnv> &
486486
void ExprLet::bindVars(EvalState & es, const std::shared_ptr<const StaticEnv> & env)
487487
{
488488
auto newEnv = [&]() -> std::shared_ptr<const StaticEnv> {
489-
auto newEnv = std::make_shared<StaticEnv>(nullptr, env, attrs->attrs.size());
489+
auto newEnv = std::make_shared<StaticEnv>(nullptr, env, attrs->attrs->size());
490490

491491
Displacement displ = 0;
492-
for (auto & i : attrs->attrs)
492+
for (auto & i : *attrs->attrs)
493493
newEnv->vars.emplace_back(i.first, i.second.displ = displ++);
494494
return newEnv;
495495
}();
496496

497497
// No need to sort newEnv since attrs->attrs is in sorted order.
498498

499499
auto inheritFromEnv = attrs->bindInheritSources(es, newEnv);
500-
for (auto & i : attrs->attrs)
500+
for (auto & i : *attrs->attrs)
501501
i.second.e->bindVars(es, i.second.chooseByKind(newEnv, env, inheritFromEnv));
502502

503503
if (es.debugRepl)

‎src/libexpr/parser.y‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ expr_function
211211
| WITH expr ';' expr_function
212212
{ $$ = state->exprs.add<ExprWith>(CUR_POS, $2, $4); }
213213
| LET binds IN_KW expr_function
214-
{ if (!$2->dynamicAttrs.empty())
214+
{ if (!$2->dynamicAttrs->empty())
215215
throw ParseError({
216216
.msg = HintFmt("dynamic attributes not allowed in let"),
217217
.pos = state->positions[CUR_POS]
@@ -413,9 +413,9 @@ binds1
413413
| binds[accum] INHERIT attrs ';'
414414
{ $$ = $accum;
415415
for (auto & [i, iPos] : $attrs) {
416-
if ($accum->attrs.find(i.symbol) != $accum->attrs.end())
417-
state->dupAttr(i.symbol, iPos, $accum->attrs[i.symbol].pos);
418-
$accum->attrs.emplace(
416+
if ($accum->attrs->find(i.symbol) != $accum->attrs->end())
417+
state->dupAttr(i.symbol, iPos, (*$accum->attrs)[i.symbol].pos);
418+
$accum->attrs->emplace(
419419
i.symbol,
420420
ExprAttrs::AttrDef(state->exprs.add<ExprVar>(iPos, i.symbol), iPos, ExprAttrs::AttrDef::Kind::Inherited));
421421
}
@@ -427,9 +427,9 @@ binds1
427427
$accum->inheritFromExprs->push_back($expr);
428428
auto from = state->exprs.add<ExprInheritFrom>(state->at(@expr), $accum->inheritFromExprs->size() - 1);
429429
for (auto & [i, iPos] : $attrs) {
430-
if ($accum->attrs.find(i.symbol) != $accum->attrs.end())
431-
state->dupAttr(i.symbol, iPos, $accum->attrs[i.symbol].pos);
432-
$accum->attrs.emplace(
430+
if ($accum->attrs->find(i.symbol) != $accum->attrs->end())
431+
state->dupAttr(i.symbol, iPos, (*$accum->attrs)[i.symbol].pos);
432+
$accum->attrs->emplace(
433433
i.symbol,
434434
ExprAttrs::AttrDef(
435435
state->exprs.add<ExprSelect>(state->exprs.alloc, iPos, from, i.symbol),

0 commit comments

Comments
 (0)