Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

forceValue: make pos mandatory #5676

Merged
merged 5 commits into from
Feb 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions src/libcmd/installables.cc
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,9 @@ void SourceExprCommand::completeInstallable(std::string_view prefix)
prefix_ = "";
}

Value &v1(*findAlongAttrPath(*state, prefix_, *autoArgs, root).first);
state->forceValue(v1);
auto [v, pos] = findAlongAttrPath(*state, prefix_, *autoArgs, root);
Value &v1(*v);
state->forceValue(v1, pos);
Value v2;
state->autoCallFunction(*autoArgs, v1, v2);

Expand Down Expand Up @@ -446,7 +447,7 @@ struct InstallableAttrPath : InstallableValue
std::pair<Value *, Pos> toValue(EvalState & state) override
{
auto [vRes, pos] = findAlongAttrPath(state, attrPath, *cmd.getAutoArgs(state), **v);
state.forceValue(*vRes);
state.forceValue(*vRes, pos);
return {vRes, pos};
}

Expand Down Expand Up @@ -496,7 +497,7 @@ Value * InstallableFlake::getFlakeOutputs(EvalState & state, const flake::Locked
auto aOutputs = vFlake->attrs->get(state.symbols.create("outputs"));
assert(aOutputs);

state.forceValue(*aOutputs->value);
state.forceValue(*aOutputs->value, aOutputs->value->determinePos(noPos));

return aOutputs->value;
}
Expand All @@ -521,7 +522,7 @@ ref<eval_cache::EvalCache> openEvalCache(
auto vFlake = state.allocValue();
flake::callFlake(state, *lockedFlake, *vFlake);

state.forceAttrs(*vFlake);
state.forceAttrs(*vFlake, noPos);

auto aOutputs = vFlake->attrs->get(state.symbols.create("outputs"));
assert(aOutputs);
Expand Down Expand Up @@ -608,7 +609,7 @@ std::pair<Value *, Pos> InstallableFlake::toValue(EvalState & state)
for (auto & attrPath : getActualAttrPaths()) {
try {
auto [v, pos] = findAlongAttrPath(state, attrPath, *emptyArgs, *vOutputs);
state.forceValue(*v);
state.forceValue(*v, pos);
return {v, pos};
} catch (AttrPathNotFound & e) {
}
Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/attr-path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ std::pair<Value *, Pos> findAlongAttrPath(EvalState & state, const string & attr
Value * vNew = state.allocValue();
state.autoCallFunction(autoArgs, *v, *vNew);
v = vNew;
state.forceValue(*v);
state.forceValue(*v, noPos);

/* It should evaluate to either a set or an expression,
according to what is specified in the attrPath. */
Expand Down
4 changes: 2 additions & 2 deletions src/libexpr/eval-cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ Value & AttrCursor::getValue()
if (!_value) {
if (parent) {
auto & vParent = parent->first->getValue();
root->state.forceAttrs(vParent);
root->state.forceAttrs(vParent, noPos);
auto attr = vParent.attrs->get(parent->second);
if (!attr)
throw Error("attribute '%s' is unexpectedly missing", getAttrPathStr());
Expand Down Expand Up @@ -381,7 +381,7 @@ Value & AttrCursor::forceValue()
auto & v = getValue();

try {
root->state.forceValue(v);
root->state.forceValue(v, noPos);
} catch (EvalError &) {
debug("setting '%s' to failed", getAttrPathStr());
if (root->db)
Expand Down
16 changes: 0 additions & 16 deletions src/libexpr/eval-inline.hh
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,6 @@ void EvalState::forceValue(Value & v, const Pos & pos)
}


inline void EvalState::forceAttrs(Value & v)
{
forceValue(v);
if (v.type() != nAttrs)
throwTypeError("value is %1% while a set was expected", v);
}


inline void EvalState::forceAttrs(Value & v, const Pos & pos)
{
forceValue(v, pos);
Expand All @@ -67,14 +59,6 @@ inline void EvalState::forceAttrs(Value & v, const Pos & pos)
}


inline void EvalState::forceList(Value & v)
{
forceValue(v);
if (!v.isList())
throwTypeError("value is %1% while a list was expected", v);
}


inline void EvalState::forceList(Value & v, const Pos & pos)
{
forceValue(v, pos);
Expand Down
19 changes: 10 additions & 9 deletions src/libexpr/eval.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1132,7 +1132,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
Hence we need __overrides.) */
if (hasOverrides) {
Value * vOverrides = (*v.attrs)[overrides->second.displ].value;
state.forceAttrs(*vOverrides);
state.forceAttrs(*vOverrides, vOverrides->determinePos(noPos));
Bindings * newBnds = state.allocBindings(v.attrs->capacity() + vOverrides->attrs->size());
for (auto & i : *v.attrs)
newBnds->push_back(i);
Expand Down Expand Up @@ -1280,7 +1280,7 @@ void ExprOpHasAttr::eval(EvalState & state, Env & env, Value & v)
e->eval(state, env, vTmp);

for (auto & i : attrPath) {
state.forceValue(*vAttrs);
state.forceValue(*vAttrs, noPos);
Bindings::iterator j;
Symbol name = getName(i, state, env);
if (vAttrs->type() != nAttrs ||
Expand Down Expand Up @@ -1494,14 +1494,15 @@ void EvalState::incrFunctionCall(ExprLambda * fun)

void EvalState::autoCallFunction(Bindings & args, Value & fun, Value & res)
{
forceValue(fun);
Pos pos = fun.determinePos(noPos);
forceValue(fun, pos);

if (fun.type() == nAttrs) {
auto found = fun.attrs->find(sFunctor);
if (found != fun.attrs->end()) {
Value * v = allocValue();
callFunction(*found->value, fun, *v, noPos);
forceValue(*v);
callFunction(*found->value, fun, *v, pos);
forceValue(*v, pos);
return autoCallFunction(args, *v, res);
}
}
Expand Down Expand Up @@ -1787,7 +1788,7 @@ void EvalState::forceValueDeep(Value & v)
recurse = [&](Value & v) {
if (!seen.insert(&v).second) return;

forceValue(v);
forceValue(v, v.determinePos(noPos));

if (v.type() == nAttrs) {
for (auto & i : *v.attrs)
Expand Down Expand Up @@ -1924,7 +1925,7 @@ bool EvalState::isDerivation(Value & v)
if (v.type() != nAttrs) return false;
Bindings::iterator i = v.attrs->find(sType);
if (i == v.attrs->end()) return false;
forceValue(*i->value);
forceValue(*i->value, *i->pos);
if (i->value->type() != nString) return false;
return strcmp(i->value->string.s, "derivation") == 0;
}
Expand Down Expand Up @@ -2036,8 +2037,8 @@ Path EvalState::coerceToPath(const Pos & pos, Value & v, PathSet & context)

bool EvalState::eqValues(Value & v1, Value & v2)
{
forceValue(v1);
forceValue(v2);
forceValue(v1, noPos);
forceValue(v2, noPos);

/* !!! Hack to support some old broken code that relies on pointer
equality tests between sets. (Specifically, builderDefs calls
Expand Down
4 changes: 1 addition & 3 deletions src/libexpr/eval.hh
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ public:
of the evaluation of the thunk. If `v' is a delayed function
application, call the function and overwrite `v' with the
result. Otherwise, this is a no-op. */
inline void forceValue(Value & v, const Pos & pos = noPos);
inline void forceValue(Value & v, const Pos & pos);

/* Force a value, then recursively force list elements and
attributes. */
Expand All @@ -231,9 +231,7 @@ public:
NixInt forceInt(Value & v, const Pos & pos);
NixFloat forceFloat(Value & v, const Pos & pos);
bool forceBool(Value & v, const Pos & pos);
inline void forceAttrs(Value & v);
inline void forceAttrs(Value & v, const Pos & pos);
inline void forceList(Value & v);
inline void forceList(Value & v, const Pos & pos);
void forceFunction(Value & v, const Pos & pos); // either lambda or primop
string forceString(Value & v, const Pos & pos = noPos);
Expand Down
6 changes: 3 additions & 3 deletions src/libexpr/get-drvs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ DrvInfo::Outputs DrvInfo::queryOutputs(bool onlyOutputsToInstall)
string name = state->forceStringNoCtx(*elem, *i->pos);
Bindings::iterator out = attrs->find(state->symbols.create(name));
if (out == attrs->end()) continue; // FIXME: throw error?
state->forceAttrs(*out->value);
state->forceAttrs(*out->value, *i->pos);

/* And evaluate its ‘outPath’ attribute. */
Bindings::iterator outPath = out->value->attrs->find(state->sOutPath);
Expand Down Expand Up @@ -172,7 +172,7 @@ StringSet DrvInfo::queryMetaNames()

bool DrvInfo::checkMeta(Value & v)
{
state->forceValue(v);
state->forceValue(v, v.determinePos(noPos));
if (v.type() == nList) {
for (auto elem : v.listItems())
if (!checkMeta(*elem)) return false;
Expand Down Expand Up @@ -278,7 +278,7 @@ static bool getDerivation(EvalState & state, Value & v,
bool ignoreAssertionFailures)
{
try {
state.forceValue(v);
state.forceValue(v, v.determinePos(noPos));
if (!state.isDerivation(v)) return true;

/* Remove spurious duplicates (e.g., a set like `rec { x =
Expand Down
4 changes: 2 additions & 2 deletions src/libexpr/primops.cc
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ static void import(EvalState & state, const Pos & pos, Value & vPath, Value * vS
if (!vScope)
state.evalFile(path, v);
else {
state.forceAttrs(*vScope);
state.forceAttrs(*vScope, pos);

Env * env = &state.allocEnv(vScope->attrs->size());
env->up = &state.baseEnv;
Expand Down Expand Up @@ -2485,7 +2485,7 @@ static void prim_zipAttrsWith(EvalState & state, const Pos & pos, Value * * args
for (unsigned int n = 0; n < listSize; ++n) {
Value * vElem = listElems[n];
try {
state.forceAttrs(*vElem);
state.forceAttrs(*vElem, noPos);
for (auto & attr : *vElem->attrs)
attrsSeen[attr.name].first++;
} catch (TypeError & e) {
Expand Down
2 changes: 1 addition & 1 deletion src/nix-build/nix-build.cc
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ static void main_nix_build(int argc, char * * argv)

for (auto & i : attrPaths) {
Value & v(*findAlongAttrPath(*state, i, *autoArgs, vRoot).first);
state->forceValue(v);
state->forceValue(v, v.determinePos(noPos));
getDerivations(*state, v, "", *autoArgs, drvs, false);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/nix-env/user-env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,

/* Evaluate it. */
debug("evaluating user environment builder");
state.forceValue(topLevel);
state.forceValue(topLevel, topLevel.determinePos(noPos));
PathSet context;
Attr & aDrvPath(*topLevel.attrs->find(state.sDrvPath));
auto topLevelDrv = state.store->parseStorePath(state.coerceToPath(*aDrvPath.pos, *aDrvPath.value, context));
Expand Down
2 changes: 1 addition & 1 deletion src/nix-instantiate/nix-instantiate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ void processExpr(EvalState & state, const Strings & attrPaths,

for (auto & i : attrPaths) {
Value & v(*findAlongAttrPath(state, i, autoArgs, vRoot).first);
state.forceValue(v);
state.forceValue(v, v.determinePos(noPos));

PathSet context;
if (evalOnly) {
Expand Down
2 changes: 1 addition & 1 deletion src/nix/eval.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ struct CmdEval : MixJSON, InstallableCommand

recurse = [&](Value & v, const Pos & pos, const Path & path)
{
state->forceValue(v);
state->forceValue(v, pos);
if (v.type() == nString)
// FIXME: disallow strings with contexts?
writeFile(path, v.string.s);
Expand Down
5 changes: 3 additions & 2 deletions src/nix/flake.cc
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,13 @@ struct CmdFlakeLock : FlakeCommand
static void enumerateOutputs(EvalState & state, Value & vFlake,
std::function<void(const std::string & name, Value & vProvide, const Pos & pos)> callback)
{
state.forceAttrs(vFlake);
auto pos = vFlake.determinePos(noPos);
state.forceAttrs(vFlake, pos);

auto aOutputs = vFlake.attrs->get(state.symbols.create("outputs"));
assert(aOutputs);

state.forceAttrs(*aOutputs->value);
state.forceAttrs(*aOutputs->value, pos);

auto sHydraJobs = state.symbols.create("hydraJobs");

Expand Down
8 changes: 4 additions & 4 deletions src/nix/prefetch.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ string resolveMirrorUrl(EvalState & state, string url)
Value vMirrors;
// FIXME: use nixpkgs flake
state.eval(state.parseExprFromString("import <nixpkgs/pkgs/build-support/fetchurl/mirrors.nix>", "."), vMirrors);
state.forceAttrs(vMirrors);
state.forceAttrs(vMirrors, noPos);

auto mirrorList = vMirrors.attrs->find(state.symbols.create(mirrorName));
if (mirrorList == vMirrors.attrs->end())
throw Error("unknown mirror name '%s'", mirrorName);
state.forceList(*mirrorList->value);
state.forceList(*mirrorList->value, noPos);

if (mirrorList->value->listSize() < 1)
throw Error("mirror URL '%s' did not expand to anything", url);
Expand Down Expand Up @@ -196,11 +196,11 @@ static int main_nix_prefetch_url(int argc, char * * argv)
Value vRoot;
state->evalFile(path, vRoot);
Value & v(*findAlongAttrPath(*state, attrPath, autoArgs, vRoot).first);
state->forceAttrs(v);
state->forceAttrs(v, noPos);

/* Extract the URL. */
auto & attr = v.attrs->need(state->symbols.create("urls"));
state->forceList(*attr.value);
state->forceList(*attr.value, noPos);
if (attr.value->listSize() < 1)
throw Error("'urls' list is empty");
url = state->forceString(*attr.value->listElems()[0]);
Expand Down
8 changes: 4 additions & 4 deletions src/nix/repl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ StringSet NixRepl::completePrefix(string prefix)
Expr * e = parseString(expr);
Value v;
e->eval(*state, *env, v);
state->forceAttrs(v);
state->forceAttrs(v, noPos);

for (auto & i : *v.attrs) {
string name = i.name;
Expand Down Expand Up @@ -673,7 +673,7 @@ void NixRepl::reloadFiles()

void NixRepl::addAttrsToScope(Value & attrs)
{
state->forceAttrs(attrs);
state->forceAttrs(attrs, attrs.determinePos(noPos));
if (displ + attrs.attrs->size() >= envSize)
throw Error("environment full; cannot add more variables");

Expand Down Expand Up @@ -712,7 +712,7 @@ void NixRepl::evalString(string s, Value & v)
{
Expr * e = parseString(s);
e->eval(*state, *env, v);
state->forceValue(v);
state->forceValue(v, v.determinePos(noPos));
}


Expand Down Expand Up @@ -742,7 +742,7 @@ std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int m
str.flush();
checkInterrupt();

state->forceValue(v);
state->forceValue(v, v.determinePos(noPos));

switch (v.type()) {

Expand Down