1
0
Fork 0
mirror of https://github.com/NixOS/nix synced 2025-06-24 22:11:15 +02:00

Never update values after setting the type

Thunks are now overwritten by a helper function
`Value::finishValue(newType, payload)` (where `payload` is the
original anonymous union inside `Value`). This helps to ensure we
never update a value elsewhere, since that would be incompatible with
parallel evaluation (i.e. after a value has transitioned from being a
thunk to being a non-thunk, it should be immutable).

There were two places where this happened: `Value::mkString()` and
`ExprAttrs::eval()`.

This PR also adds a bunch of accessor functions for value contents,
like `Value::integer()` to access the integer field in the union.
This commit is contained in:
Eelco Dolstra 2024-03-25 18:20:18 +01:00
parent 6d90287f5a
commit 8c0590fa32
35 changed files with 530 additions and 556 deletions

View file

@ -216,13 +216,13 @@ static void import(EvalState & state, const PosIdx pos, Value & vPath, Value * v
else {
state.forceAttrs(*vScope, pos, "while evaluating the first argument passed to builtins.scopedImport");
Env * env = &state.allocEnv(vScope->attrs->size());
Env * env = &state.allocEnv(vScope->attrs()->size());
env->up = &state.baseEnv;
auto staticEnv = std::make_shared<StaticEnv>(nullptr, state.staticBaseEnv.get(), vScope->attrs->size());
auto staticEnv = std::make_shared<StaticEnv>(nullptr, state.staticBaseEnv.get(), vScope->attrs()->size());
unsigned int displ = 0;
for (auto & attr : *vScope->attrs) {
for (auto & attr : *vScope->attrs()) {
staticEnv->vars.emplace_back(attr.name, displ);
env->values[displ++] = attr.value;
}
@ -411,7 +411,7 @@ static void prim_typeOf(EvalState & state, const PosIdx pos, Value * * args, Val
case nList: t = "list"; break;
case nFunction: t = "lambda"; break;
case nExternal:
t = args[0]->external->typeOf();
t = args[0]->external()->typeOf();
break;
case nFloat: t = "float"; break;
case nThunk: abort();
@ -575,9 +575,9 @@ struct CompareValues
{
try {
if (v1->type() == nFloat && v2->type() == nInt)
return v1->fpoint < v2->integer;
return v1->fpoint() < v2->integer();
if (v1->type() == nInt && v2->type() == nFloat)
return v1->integer < v2->fpoint;
return v1->integer() < v2->fpoint();
if (v1->type() != v2->type())
state.error<EvalError>("cannot compare %s with %s", showType(*v1), showType(*v2)).debugThrow();
// Allow selecting a subset of enum values
@ -585,16 +585,16 @@ struct CompareValues
#pragma GCC diagnostic ignored "-Wswitch-enum"
switch (v1->type()) {
case nInt:
return v1->integer < v2->integer;
return v1->integer() < v2->integer();
case nFloat:
return v1->fpoint < v2->fpoint;
return v1->fpoint() < v2->fpoint();
case nString:
return strcmp(v1->c_str(), v2->c_str()) < 0;
case nPath:
// Note: we don't take the accessor into account
// since it's not obvious how to compare them in a
// reproducible way.
return strcmp(v1->_path.path, v2->_path.path) < 0;
return strcmp(v1->payload.path.path, v2->payload.path.path) < 0;
case nList:
// Lexicographic comparison
for (size_t i = 0;; i++) {
@ -626,13 +626,13 @@ typedef std::list<Value *> ValueList;
#endif
static Bindings::iterator getAttr(
static Bindings::const_iterator getAttr(
EvalState & state,
Symbol attrSym,
Bindings * attrSet,
const Bindings * attrSet,
std::string_view errorCtx)
{
Bindings::iterator value = attrSet->find(attrSym);
auto value = attrSet->find(attrSym);
if (value == attrSet->end()) {
state.error<TypeError>("attribute '%s' missing", state.symbols[attrSym]).withTrace(noPos, errorCtx).debugThrow();
}
@ -644,7 +644,7 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value * * a
state.forceAttrs(*args[0], noPos, "while evaluating the first argument passed to builtins.genericClosure");
/* Get the start set. */
Bindings::iterator startSet = getAttr(state, state.sStartSet, args[0]->attrs, "in the attrset passed as argument to builtins.genericClosure");
auto startSet = getAttr(state, state.sStartSet, args[0]->attrs(), "in the attrset passed as argument to builtins.genericClosure");
state.forceList(*startSet->value, noPos, "while evaluating the 'startSet' attribute passed as argument to builtins.genericClosure");
@ -658,7 +658,7 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value * * a
}
/* Get the operator. */
Bindings::iterator op = getAttr(state, state.sOperator, args[0]->attrs, "in the attrset passed as argument to builtins.genericClosure");
auto op = getAttr(state, state.sOperator, args[0]->attrs(), "in the attrset passed as argument to builtins.genericClosure");
state.forceFunction(*op->value, noPos, "while evaluating the 'operator' attribute passed as argument to builtins.genericClosure");
/* Construct the closure by applying the operator to elements of
@ -675,7 +675,7 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value * * a
state.forceAttrs(*e, noPos, "while evaluating one of the elements generated by (or initially passed to) builtins.genericClosure");
Bindings::iterator key = getAttr(state, state.sKey, e->attrs, "in one of the attrsets generated by (or initially passed to) builtins.genericClosure");
auto key = getAttr(state, state.sKey, e->attrs(), "in one of the attrsets generated by (or initially passed to) builtins.genericClosure");
state.forceValue(*key->value, noPos);
if (!doneKeys.insert(key->value).second) continue;
@ -1041,7 +1041,11 @@ static void prim_second(EvalState & state, const PosIdx pos, Value * * args, Val
* Derivations
*************************************************************/
static void derivationStrictInternal(EvalState & state, const std::string & name, Bindings * attrs, Value & v);
static void derivationStrictInternal(
EvalState & state,
const std::string & name,
const Bindings * attrs,
Value & v);
/* Construct (as a unobservable side effect) a Nix derivation
expression that performs the derivation described by the argument
@ -1054,10 +1058,10 @@ static void prim_derivationStrict(EvalState & state, const PosIdx pos, Value * *
{
state.forceAttrs(*args[0], pos, "while evaluating the argument passed to builtins.derivationStrict");
Bindings * attrs = args[0]->attrs;
auto attrs = args[0]->attrs();
/* Figure out the name first (for stack backtraces). */
Bindings::iterator nameAttr = getAttr(state, state.sName, attrs, "in the attrset passed as argument to builtins.derivationStrict");
auto nameAttr = getAttr(state, state.sName, attrs, "in the attrset passed as argument to builtins.derivationStrict");
std::string drvName;
try {
@ -1096,8 +1100,11 @@ static void prim_derivationStrict(EvalState & state, const PosIdx pos, Value * *
}
}
static void derivationStrictInternal(EvalState & state, const std::string &
drvName, Bindings * attrs, Value & v)
static void derivationStrictInternal(
EvalState & state,
const std::string & drvName,
const Bindings * attrs,
Value & v)
{
/* Check whether attributes should be passed as a JSON file. */
using nlohmann::json;
@ -1670,11 +1677,11 @@ static void prim_findFile(EvalState & state, const PosIdx pos, Value * * args, V
state.forceAttrs(*v2, pos, "while evaluating an element of the list passed to builtins.findFile");
std::string prefix;
Bindings::iterator i = v2->attrs->find(state.sPrefix);
if (i != v2->attrs->end())
auto i = v2->attrs()->find(state.sPrefix);
if (i != v2->attrs()->end())
prefix = state.forceStringNoCtx(*i->value, pos, "while evaluating the `prefix` attribute of an element of the list passed to builtins.findFile");
i = getAttr(state, state.sPath, v2->attrs, "in an element of the __nixPath");
i = getAttr(state, state.sPath, v2->attrs(), "in an element of the __nixPath");
NixStringContext context;
auto path = state.coerceToString(pos, *i->value, context,
@ -2346,7 +2353,7 @@ static void prim_path(EvalState & state, const PosIdx pos, Value * * args, Value
state.forceAttrs(*args[0], pos, "while evaluating the argument passed to 'builtins.path'");
for (auto & attr : *args[0]->attrs) {
for (auto & attr : *args[0]->attrs()) {
auto n = state.symbols[attr.name];
if (n == "path")
path.emplace(state.coerceToPath(attr.pos, *attr.value, context, "while evaluating the 'path' attribute passed to 'builtins.path'"));
@ -2421,9 +2428,9 @@ static void prim_attrNames(EvalState & state, const PosIdx pos, Value * * args,
{
state.forceAttrs(*args[0], pos, "while evaluating the argument passed to builtins.attrNames");
auto list = state.buildList(args[0]->attrs->size());
auto list = state.buildList(args[0]->attrs()->size());
for (const auto & [n, i] : enumerate(*args[0]->attrs))
for (const auto & [n, i] : enumerate(*args[0]->attrs()))
(list[n] = state.allocValue())->mkString(state.symbols[i.name]);
std::sort(list.begin(), list.end(),
@ -2449,9 +2456,9 @@ static void prim_attrValues(EvalState & state, const PosIdx pos, Value * * args,
{
state.forceAttrs(*args[0], pos, "while evaluating the argument passed to builtins.attrValues");
auto list = state.buildList(args[0]->attrs->size());
auto list = state.buildList(args[0]->attrs()->size());
for (const auto & [n, i] : enumerate(*args[0]->attrs))
for (const auto & [n, i] : enumerate(*args[0]->attrs()))
list[n] = (Value *) &i;
std::sort(list.begin(), list.end(),
@ -2482,10 +2489,10 @@ void prim_getAttr(EvalState & state, const PosIdx pos, Value * * args, Value & v
{
auto attr = state.forceStringNoCtx(*args[0], pos, "while evaluating the first argument passed to builtins.getAttr");
state.forceAttrs(*args[1], pos, "while evaluating the second argument passed to builtins.getAttr");
Bindings::iterator i = getAttr(
auto i = getAttr(
state,
state.symbols.create(attr),
args[1]->attrs,
args[1]->attrs(),
"in the attribute set under consideration"
);
// !!! add to stack trace?
@ -2511,8 +2518,8 @@ static void prim_unsafeGetAttrPos(EvalState & state, const PosIdx pos, Value * *
{
auto attr = state.forceStringNoCtx(*args[0], pos, "while evaluating the first argument passed to builtins.unsafeGetAttrPos");
state.forceAttrs(*args[1], pos, "while evaluating the second argument passed to builtins.unsafeGetAttrPos");
Bindings::iterator i = args[1]->attrs->find(state.symbols.create(attr));
if (i == args[1]->attrs->end())
auto i = args[1]->attrs()->find(state.symbols.create(attr));
if (i == args[1]->attrs()->end())
v.mkNull();
else
state.mkPos(v, i->pos);
@ -2540,13 +2547,13 @@ static struct LazyPosAcessors {
PrimOp primop_lineOfPos{
.arity = 1,
.fun = [] (EvalState & state, PosIdx pos, Value * * args, Value & v) {
v.mkInt(state.positions[PosIdx(args[0]->integer)].line);
v.mkInt(state.positions[PosIdx(args[0]->integer())].line);
}
};
PrimOp primop_columnOfPos{
.arity = 1,
.fun = [] (EvalState & state, PosIdx pos, Value * * args, Value & v) {
v.mkInt(state.positions[PosIdx(args[0]->integer)].column);
v.mkInt(state.positions[PosIdx(args[0]->integer())].column);
}
};
@ -2577,7 +2584,7 @@ static void prim_hasAttr(EvalState & state, const PosIdx pos, Value * * args, Va
{
auto attr = state.forceStringNoCtx(*args[0], pos, "while evaluating the first argument passed to builtins.hasAttr");
state.forceAttrs(*args[1], pos, "while evaluating the second argument passed to builtins.hasAttr");
v.mkBool(args[1]->attrs->find(state.symbols.create(attr)) != args[1]->attrs->end());
v.mkBool(args[1]->attrs()->find(state.symbols.create(attr)) != args[1]->attrs()->end());
}
static RegisterPrimOp primop_hasAttr({
@ -2627,9 +2634,9 @@ static void prim_removeAttrs(EvalState & state, const PosIdx pos, Value * * args
/* Copy all attributes not in that set. Note that we don't need
to sort v.attrs because it's a subset of an already sorted
vector. */
auto attrs = state.buildBindings(args[0]->attrs->size());
auto attrs = state.buildBindings(args[0]->attrs()->size());
std::set_difference(
args[0]->attrs->begin(), args[0]->attrs->end(),
args[0]->attrs()->begin(), args[0]->attrs()->end(),
names.begin(), names.end(),
std::back_inserter(attrs));
v.mkAttrs(attrs.alreadySorted());
@ -2667,13 +2674,13 @@ static void prim_listToAttrs(EvalState & state, const PosIdx pos, Value * * args
for (auto v2 : args[0]->listItems()) {
state.forceAttrs(*v2, pos, "while evaluating an element of the list passed to builtins.listToAttrs");
Bindings::iterator j = getAttr(state, state.sName, v2->attrs, "in a {name=...; value=...;} pair");
auto j = getAttr(state, state.sName, v2->attrs(), "in a {name=...; value=...;} pair");
auto name = state.forceStringNoCtx(*j->value, j->pos, "while evaluating the `name` attribute of an element of the list passed to builtins.listToAttrs");
auto sym = state.symbols.create(name);
if (seen.insert(sym).second) {
Bindings::iterator j2 = getAttr(state, state.sValue, v2->attrs, "in a {name=...; value=...;} pair");
auto j2 = getAttr(state, state.sValue, v2->attrs(), "in a {name=...; value=...;} pair");
attrs.insert(sym, j2->value, j2->pos);
}
}
@ -2717,8 +2724,8 @@ static void prim_intersectAttrs(EvalState & state, const PosIdx pos, Value * * a
state.forceAttrs(*args[0], pos, "while evaluating the first argument passed to builtins.intersectAttrs");
state.forceAttrs(*args[1], pos, "while evaluating the second argument passed to builtins.intersectAttrs");
Bindings &left = *args[0]->attrs;
Bindings &right = *args[1]->attrs;
auto & left = *args[0]->attrs();
auto & right = *args[1]->attrs();
auto attrs = state.buildBindings(std::min(left.size(), right.size()));
@ -2762,14 +2769,14 @@ static void prim_intersectAttrs(EvalState & state, const PosIdx pos, Value * * a
if (left.size() < right.size()) {
for (auto & l : left) {
Bindings::iterator r = right.find(l.name);
auto r = right.find(l.name);
if (r != right.end())
attrs.insert(*r);
}
}
else {
for (auto & r : right) {
Bindings::iterator l = left.find(r.name);
auto l = left.find(r.name);
if (l != left.end())
attrs.insert(r);
}
@ -2800,8 +2807,7 @@ static void prim_catAttrs(EvalState & state, const PosIdx pos, Value * * args, V
for (auto v2 : args[1]->listItems()) {
state.forceAttrs(*v2, pos, "while evaluating an element in the list passed as second argument to builtins.catAttrs");
Bindings::iterator i = v2->attrs->find(attrName);
if (i != v2->attrs->end())
if (auto i = v2->attrs()->get(attrName))
res[found++] = i->value;
}
@ -2838,13 +2844,13 @@ static void prim_functionArgs(EvalState & state, const PosIdx pos, Value * * arg
if (!args[0]->isLambda())
state.error<TypeError>("'functionArgs' requires a function").atPos(pos).debugThrow();
if (!args[0]->lambda.fun->hasFormals()) {
if (!args[0]->payload.lambda.fun->hasFormals()) {
v.mkAttrs(&state.emptyBindings);
return;
}
auto attrs = state.buildBindings(args[0]->lambda.fun->formals->formals.size());
for (auto & i : args[0]->lambda.fun->formals->formals)
auto attrs = state.buildBindings(args[0]->payload.lambda.fun->formals->formals.size());
for (auto & i : args[0]->payload.lambda.fun->formals->formals)
attrs.insert(i.name, state.getBool(i.def), i.pos);
v.mkAttrs(attrs);
}
@ -2871,9 +2877,9 @@ static void prim_mapAttrs(EvalState & state, const PosIdx pos, Value * * args, V
{
state.forceAttrs(*args[1], pos, "while evaluating the second argument passed to builtins.mapAttrs");
auto attrs = state.buildBindings(args[1]->attrs->size());
auto attrs = state.buildBindings(args[1]->attrs()->size());
for (auto & i : *args[1]->attrs) {
for (auto & i : *args[1]->attrs()) {
Value * vName = state.allocValue();
Value * vFun2 = state.allocValue();
vName->mkString(state.symbols[i.name]);
@ -2923,7 +2929,7 @@ static void prim_zipAttrsWith(EvalState & state, const PosIdx pos, Value * * arg
for (auto & vElem : listItems) {
state.forceAttrs(*vElem, noPos, "while evaluating a value of the list passed as second argument to builtins.zipAttrsWith");
for (auto & attr : *vElem->attrs)
for (auto & attr : *vElem->attrs())
attrsSeen.try_emplace(attr.name).first->second.size++;
}
@ -2931,7 +2937,7 @@ static void prim_zipAttrsWith(EvalState & state, const PosIdx pos, Value * * arg
elem.list.emplace(state.buildList(elem.size));
for (auto & vElem : listItems) {
for (auto & attr : *vElem->attrs) {
for (auto & attr : *vElem->attrs()) {
auto & item = attrsSeen.at(attr.name);
(*item.list)[item.pos++] = attr.value;
}
@ -3375,7 +3381,7 @@ static void prim_sort(EvalState & state, const PosIdx pos, Value * * args, Value
callFunction. */
/* TODO: (layus) this is absurd. An optimisation like this
should be outside the lambda creation */
if (args[0]->isPrimOp() && args[0]->primOp->fun == prim_lessThan)
if (args[0]->isPrimOp() && args[0]->primOp()->fun == prim_lessThan)
return CompareValues(state, noPos, "while evaluating the ordering function passed to builtins.sort")(a, b);
Value * vs[] = {a, b};
@ -3866,18 +3872,17 @@ static RegisterPrimOp primop_hashString({
static void prim_convertHash(EvalState & state, const PosIdx pos, Value * * args, Value & v)
{
state.forceAttrs(*args[0], pos, "while evaluating the first argument passed to builtins.convertHash");
auto &inputAttrs = args[0]->attrs;
auto inputAttrs = args[0]->attrs();
Bindings::iterator iteratorHash = getAttr(state, state.symbols.create("hash"), inputAttrs, "while locating the attribute 'hash'");
auto iteratorHash = getAttr(state, state.symbols.create("hash"), inputAttrs, "while locating the attribute 'hash'");
auto hash = state.forceStringNoCtx(*iteratorHash->value, pos, "while evaluating the attribute 'hash'");
Bindings::iterator iteratorHashAlgo = inputAttrs->find(state.symbols.create("hashAlgo"));
auto iteratorHashAlgo = inputAttrs->get(state.symbols.create("hashAlgo"));
std::optional<HashAlgorithm> ha = std::nullopt;
if (iteratorHashAlgo != inputAttrs->end()) {
if (iteratorHashAlgo)
ha = parseHashAlgo(state.forceStringNoCtx(*iteratorHashAlgo->value, pos, "while evaluating the attribute 'hashAlgo'"));
}
Bindings::iterator iteratorToHashFormat = getAttr(state, state.symbols.create("toHashFormat"), args[0]->attrs, "while locating the attribute 'toHashFormat'");
auto iteratorToHashFormat = getAttr(state, state.symbols.create("toHashFormat"), args[0]->attrs(), "while locating the attribute 'toHashFormat'");
HashFormat hf = parseHashFormat(state.forceStringNoCtx(*iteratorToHashFormat->value, pos, "while evaluating the attribute 'toHashFormat'"));
v.mkString(Hash::parseAny(hash, ha).to_string(hf, hf == HashFormat::SRI));
@ -4623,7 +4628,7 @@ void EvalState::createBaseEnv()
/* Now that we've added all primops, sort the `builtins' set,
because attribute lookups expect it to be sorted. */
baseEnv.values[0]->attrs->sort();
baseEnv.values[0]->payload.attrs->sort();
staticBaseEnv->sort();