nrabulinski/lix
0
0
Fork 0
forked from lix-project/lix
Code Pull requests Activity

libexpr: Split ExprAttrs and ExprLet

Browse source 
ExprLet was previously inheriting from ExprAttrs for the data, while
ignoring all
set-specific operations on it. The set specific code has now been split
off so that
let doesn't inherit it anymore:

- ExprAttrs (not an Expr), containing the attributes and the related
logic
- ExprLet : Expr, ExprAttrs
- ExprSet : Expr, ExprAttrs

Co-authored-by: eldritch horrors <pennae@lix.systems>
Change-Id: I63f2fbcd1e790b3cffb56eec1e7565ee3cdbf964
This commit is contained in:
piegames 2024-11-03 13:38:53 +01:00
parent 2a9e560570
commit 9611018c27
8 changed files with 86 additions and 65 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
lix/libexpr/eval.cc
View file

@ -1071,12 +1071,13 @@ Env * ExprAttrs::buildInheritFromEnv(EvalState & state, Env & up)
return &inheritEnv;
}
void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
void ExprSet::eval(EvalState & state, Env & env, Value & v)
{
v.mkAttrs(state.ctx.buildBindings(attrs.size() + dynamicAttrs.size()).finish());
auto dynamicEnv = &env;
if (recursive) {
/* Create a new environment that contains the attributes in
this `rec'. */
Env & env2(state.ctx.mem.allocEnv(attrs.size()));
@ -1084,7 +1085,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
dynamicEnv = &env2;
Env * inheritEnv = inheritFromExprs ? buildInheritFromEnv(state, env2) : nullptr;
AttrDefs::iterator overrides = attrs.find(state.ctx.s.overrides);
ExprAttrs::AttrDefs::iterator overrides = attrs.find(state.ctx.s.overrides);
bool hasOverrides = overrides != attrs.end();
/* The recursive attributes are evaluated in the new
@ -1093,7 +1094,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
Displacement displ = 0;
for (auto & i : attrs) {
Value * vAttr;
if (hasOverrides && i.second.kind != AttrDef::Kind::Inherited) {
if (hasOverrides && i.second.kind != ExprAttrs::AttrDef::Kind::Inherited) {
vAttr = state.ctx.mem.allocValue();
vAttr->mkThunk(i.second.chooseByKind(&env2, &env, inheritEnv), *i.second.e);
state.ctx.stats.nrThunks++;
@ -1118,7 +1119,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
for (auto & i : *v.attrs)
newBnds->push_back(i);
for (auto & i : *vOverrides->attrs) {
AttrDefs::iterator j = attrs.find(i.name);
ExprAttrs::AttrDefs::iterator j = attrs.find(i.name);
if (j != attrs.end()) {
(*newBnds)[j->second.displ] = i;
env2.values[j->second.displ] = i.value;
@ -1167,16 +1168,16 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v)
{
/* Create a new environment that contains the attributes in this
`let'. */
Env & env2(state.ctx.mem.allocEnv(attrs->attrs.size()));
Env & env2(state.ctx.mem.allocEnv(attrs.size()));
env2.up = &env;
Env * inheritEnv = attrs->inheritFromExprs ? attrs->buildInheritFromEnv(state, env2) : nullptr;
Env * inheritEnv = inheritFromExprs ? buildInheritFromEnv(state, env2) : nullptr;
/* The recursive attributes are evaluated in the new environment,
while the inherited attributes are evaluated in the original
environment. */
Displacement displ = 0;
for (auto & i : attrs->attrs) {
for (auto & i : attrs) {
env2.values[displ++] = i.second.e->maybeThunk(
state,
*i.second.chooseByKind(&env2, &env, inheritEnv));

2
lix/libexpr/eval.hh
View file

@ -748,7 +748,7 @@ private:
inline Value * lookupVar(Env * env, const ExprVar & var, bool noEval);
friend struct ExprVar;
friend struct ExprAttrs;
friend struct ExprSet;
friend struct ExprLet;
/**

16
lix/libexpr/nixexpr.cc
View file

@ -139,7 +139,7 @@ void ExprAttrs::showBindings(const SymbolTable & symbols, std::ostream & str) co
}
}
void ExprAttrs::show(const SymbolTable & symbols, std::ostream & str) const
void ExprSet::show(const SymbolTable & symbols, std::ostream & str) const
{
if (recursive) str << "rec ";
str << "{ ";
@ -202,7 +202,7 @@ void ExprCall::show(const SymbolTable & symbols, std::ostream & str) const
void ExprLet::show(const SymbolTable & symbols, std::ostream & str) const
{
str << "(let ";
attrs->showBindings(symbols, str);
showBindings(symbols, str);
str << "in ";
body->show(symbols, str);
str << ")";
@ -423,7 +423,7 @@ std::shared_ptr<const StaticEnv> ExprAttrs::bindInheritSources(
return inner;
}
void ExprAttrs::bindVars(Evaluator & es, const std::shared_ptr<const StaticEnv> & env)
void ExprSet::bindVars(Evaluator & es, const std::shared_ptr<const StaticEnv> & env)
{
if (es.debug)
es.debug->exprEnvs.insert(std::make_pair(this, env));
@ -511,18 +511,18 @@ void ExprCall::bindVars(Evaluator & es, const std::shared_ptr<const StaticEnv> &
void ExprLet::bindVars(Evaluator & es, const std::shared_ptr<const StaticEnv> & env)
{
auto newEnv = [&] () -> std::shared_ptr<const StaticEnv> {
auto newEnv = std::make_shared<StaticEnv>(nullptr, env.get(), attrs->attrs.size());
auto newEnv = std::make_shared<StaticEnv>(nullptr, env.get(), attrs.size());
Displacement displ = 0;
for (auto & i : attrs->attrs)
for (auto & i : attrs)
newEnv->vars.emplace_back(i.first, i.second.displ = displ++);
return newEnv;
}();
// No need to sort newEnv since attrs->attrs is in sorted order.
// No need to sort newEnv since attrs is in sorted order.
auto inheritFromEnv = attrs->bindInheritSources(es, newEnv);
for (auto & i : attrs->attrs)
auto inheritFromEnv = bindInheritSources(es, newEnv);
for (auto & i : attrs)
i.second.e->bindVars(es, i.second.chooseByKind(newEnv, env, inheritFromEnv));
if (es.debug)

30
lix/libexpr/nixexpr.hh
View file

@ -196,10 +196,16 @@ struct ExprOpHasAttr : Expr
COMMON_METHODS
};
struct ExprAttrs : Expr
/* Helper struct to contain the data shared across lets and sets */
struct ExprAttrs
{
bool recursive;
PosIdx pos;
ExprAttrs() = default;
ExprAttrs(const ExprAttrs &) = delete;
ExprAttrs & operator=(const ExprAttrs &) = delete;
ExprAttrs(ExprAttrs &&) = default;
ExprAttrs & operator=(ExprAttrs &&) = default;
virtual ~ExprAttrs() = default;
struct AttrDef {
enum class Kind {
/** `attr = expr;` */
@ -243,10 +249,6 @@ struct ExprAttrs : Expr
};
typedef std::vector<DynamicAttrDef> DynamicAttrDefs;
DynamicAttrDefs dynamicAttrs;
ExprAttrs(const PosIdx &pos) : recursive(false), pos(pos) { };
ExprAttrs() : recursive(false) { };
PosIdx getPos() const override { return pos; }
COMMON_METHODS
std::shared_ptr<const StaticEnv> bindInheritSources(
Evaluator & es, const std::shared_ptr<const StaticEnv> & env);
@ -254,6 +256,16 @@ struct ExprAttrs : Expr
void showBindings(const SymbolTable & symbols, std::ostream & str) const;
};
struct ExprSet : Expr, ExprAttrs {
PosIdx pos;
bool recursive = false;
ExprSet(const PosIdx &pos, bool recursive = false) : pos(pos), recursive(recursive) { };
ExprSet() { };
PosIdx getPos() const override { return pos; }
COMMON_METHODS
};
struct ExprList : Expr
{
std::vector<std::unique_ptr<Expr>> elems;
@ -368,11 +380,9 @@ struct ExprCall : Expr
COMMON_METHODS
};
struct ExprLet : Expr
struct ExprLet : Expr, ExprAttrs
{
std::unique_ptr<ExprAttrs> attrs;
std::unique_ptr<Expr> body;
ExprLet(std::unique_ptr<ExprAttrs> attrs, std::unique_ptr<Expr> body) : attrs(std::move(attrs)), body(std::move(body)) { };
COMMON_METHODS
};

63
lix/libexpr/parser/parser-impl1.inc.cc
View file

@ -285,13 +285,29 @@ template<> struct BuildAST<grammar::v1::attr::string> {
template<> struct BuildAST<grammar::v1::attr::expr> : BuildAST<grammar::v1::attr::string> {};
struct BindingsState : SubexprState {
using SubexprState::SubexprState;
explicit BindingsState(ExprState & up, ExprAttrs & attrs) : SubexprState(up), attrs(attrs) {}
ExprAttrs attrs;
ExprAttrs & attrs;
PosIdx pos;
AttrPath path;
std::unique_ptr<Expr> value;
};
struct BindingsStateSet : BindingsState {
ExprSet set = {};
BindingsStateSet(ExprState & up, State & ps, auto &...) : BindingsState(up, set) { }
};
struct BindingsStateRecSet : BindingsState {
ExprSet set = { PosIdx{}, true };
BindingsStateRecSet(ExprState & up, State & ps, auto &...) : BindingsState(up, set) { }
};
struct BindingsStateLet : BindingsState {
ExprLet let = {};
BindingsStateLet(ExprState & up, State & ps, auto &...) : BindingsState(up, let) { }
};
struct InheritState : SubexprState {
using SubexprState::SubexprState;
@ -663,8 +679,8 @@ template<> struct BuildAST<grammar::v1::expr::uri> {
}
};
template<> struct BuildAST<grammar::v1::expr::ancient_let> : change_head<BindingsState> {
static void success(const auto & in, BindingsState & b, ExprState & s, State & ps) {
template<> struct BuildAST<grammar::v1::expr::ancient_let> : change_head<BindingsStateRecSet> {
static void success(const auto & in, BindingsStateRecSet & b, ExprState & s, State & ps) {
// Added 2024-09-18. Turn into an error at some point in the future.
// See the documentation on deprecated features for more details.
if (!ps.featureSettings.isEnabled(Dep::AncientLet))
@ -675,30 +691,23 @@ template<> struct BuildAST<grammar::v1::expr::ancient_let> : change_head<Binding
"--extra-deprecated-features ancient-let"
);
b.attrs.pos = ps.at(in);
b.attrs.recursive = true;
s.pushExpr<ExprSelect>(b.attrs.pos, b.attrs.pos, std::make_unique<ExprAttrs>(std::move(b.attrs)), ps.s.body);
auto pos = ps.at(in);
b.set.pos = pos;
s.pushExpr<ExprSelect>(pos, pos, std::make_unique<ExprSet>(std::move(b.set)), ps.s.body);
}
};
template<> struct BuildAST<grammar::v1::expr::rec_set> : change_head<BindingsState> {
static void success(const auto & in, BindingsState & b, ExprState & s, State & ps) {
// Before inserting new attrs, check for __override and throw an error
// (the error will initially be a warning to ease migration)
if (!featureSettings.isEnabled(Dep::RecSetOverrides) && b.attrs.attrs.contains(ps.s.overrides)) {
ps.overridesFound(ps.at(in));
}
b.attrs.pos = ps.at(in);
b.attrs.recursive = true;
s.pushExpr<ExprAttrs>(b.attrs.pos, std::move(b.attrs));
template<> struct BuildAST<grammar::v1::expr::rec_set> : change_head<BindingsStateRecSet> {
static void success(const auto & in, BindingsStateRecSet & b, ExprState & s, State & ps) {
b.set.pos = ps.at(in);
s.pushExpr<ExprSet>(ps.at(in), std::move(b.set));
}
};
template<> struct BuildAST<grammar::v1::expr::set> : change_head<BindingsState> {
static void success(const auto & in, BindingsState & b, ExprState & s, State & ps) {
b.attrs.pos = ps.at(in);
s.pushExpr<ExprAttrs>(b.attrs.pos, std::move(b.attrs));
template<> struct BuildAST<grammar::v1::expr::set> : change_head<BindingsStateSet> {
static void success(const auto & in, BindingsStateSet & b, ExprState & s, State & ps) {
b.set.pos = ps.at(in);
s.pushExpr<ExprSet>(ps.at(in), std::move(b.set));
}
};
@ -835,15 +844,15 @@ template<> struct BuildAST<grammar::v1::expr::with> {
}
};
template<> struct BuildAST<grammar::v1::expr::let> : change_head<BindingsState> {
static void success(const auto & in, BindingsState & b, ExprState & s, State & ps) {
if (!b.attrs.dynamicAttrs.empty())
template<> struct BuildAST<grammar::v1::expr::let> : change_head<BindingsStateLet> {
static void success(const auto & in, BindingsStateLet & b, ExprState & s, State & ps) {
if (!b.let.dynamicAttrs.empty())
throw ParseError({
.msg = HintFmt("dynamic attributes not allowed in let"),
.pos = ps.positions[ps.at(in)]
});
s.pushExpr<ExprLet>(ps.at(in), std::make_unique<ExprAttrs>(std::move(b.attrs)), b->popExprOnly());
b.let.body = b->popExprOnly();
s.pushExpr<ExprLet>(ps.at(in), std::move(b.let));
}
};

19
lix/libexpr/parser/state.hh
View file

@ -102,7 +102,7 @@ inline void State::addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_
ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(i->symbol);
if (j != attrs->attrs.end()) {
if (j->second.kind != ExprAttrs::AttrDef::Kind::Inherited) {
ExprAttrs * attrs2 = dynamic_cast<ExprAttrs *>(j->second.e.get());
ExprSet * attrs2 = dynamic_cast<ExprSet *>(j->second.e.get());
if (!attrs2) {
attrPath.erase(i + 1, attrPath.end());
dupAttr(attrPath, pos, j->second.pos);
@ -115,12 +115,12 @@ inline void State::addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_
} else {
auto next = attrs->attrs.emplace(std::piecewise_construct,
std::tuple(i->symbol),
std::tuple(std::make_unique<ExprAttrs>(), pos));
attrs = static_cast<ExprAttrs *>(next.first->second.e.get());
std::tuple(std::make_unique<ExprSet>(), pos));
attrs = static_cast<ExprSet *>(next.first->second.e.get());
}
} else {
auto & next = attrs->dynamicAttrs.emplace_back(std::move(i->expr), std::make_unique<ExprAttrs>(), pos);
attrs = static_cast<ExprAttrs *>(next.valueExpr.get());
auto & next = attrs->dynamicAttrs.emplace_back(std::move(i->expr), std::make_unique<ExprSet>(), pos);
attrs = static_cast<ExprSet *>(next.valueExpr.get());
}
}
// Expr insertion.
@ -132,8 +132,8 @@ inline void State::addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_
// e and the expr pointed by the attr path are two attribute sets,
// we want to merge them.
// Otherwise, throw an error.
auto * ae = dynamic_cast<ExprAttrs *>(e.get());
auto * jAttrs = dynamic_cast<ExprAttrs *>(j->second.e.get());
auto * ae = dynamic_cast<ExprSet *>(e.get());
auto * jAttrs = dynamic_cast<ExprSet *>(j->second.e.get());
if (jAttrs && ae) {
if (ae->inheritFromExprs && !jAttrs->inheritFromExprs)
jAttrs->inheritFromExprs = std::make_unique<std::vector<ref<Expr>>>();
@ -157,8 +157,9 @@ inline void State::addAttr(ExprAttrs * attrs, AttrPath && attrPath, std::unique_
} else {
// Before inserting new attrs, check for __override and throw an error
// (the error will initially be a warning to ease migration)
if (attrs->recursive && !featureSettings.isEnabled(Dep::RecSetOverrides) && i->symbol == s.overrides) {
overridesFound(pos);
if (!featureSettings.isEnabled(Dep::RecSetOverrides) && i->symbol == s.overrides) {
if (auto set = dynamic_cast<ExprSet *>(attrs); set && set->recursive)
overridesFound(pos);
}
// This attr path is not defined. Let's create it.

4
lix/libexpr/value.cc
View file

@ -55,8 +55,8 @@ bool Value::isTrivial() const
internalType != tApp
&& internalType != tPrimOpApp
&& (internalType != tThunk
|| (dynamic_cast<ExprAttrs *>(thunk.expr)
&& static_cast<ExprAttrs *>(thunk.expr)->dynamicAttrs.empty())
|| (dynamic_cast<ExprSet *>(thunk.expr)
&& static_cast<ExprSet *>(thunk.expr)->dynamicAttrs.empty())
|| dynamic_cast<ExprLambda *>(thunk.expr)
|| dynamic_cast<ExprList *>(thunk.expr));
}

2
tests/functional/lang/parse-okay-rec-set-override-warning.err.exp
View file

@ -1,2 +1,2 @@
warning: __overrides found at «stdin»:3:16. This feature is deprecated and will be removed in the future. Use --extra-deprecated-features rec-set-overrides to silence this warning.
warning: __overrides found at «stdin»:4:2. This feature is deprecated and will be removed in the future. Use --extra-deprecated-features rec-set-overrides to silence this warning.
warning: __overrides found at «stdin»:4:8. This feature is deprecated and will be removed in the future. Use --extra-deprecated-features rec-set-overrides to silence this warning.