refactor(tvix/libexpr): Remove Bindings::SortedByKeys()

Since we don't have a Bindings implementation with unstable order this
function is not required, as its callers can just iterate over the
attributes instead.

Change-Id: I01b35277b5a2dde69d684bc881dbd7c0701bcbb3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/2291
Tested-by: BuildkiteCI
Reviewed-by: glittershark <grfn@gws.fyi>
This commit is contained in:
Vincent Ambo 2020-12-22 21:51:50 +01:00 committed by tazjin
parent 0f9a7b3f86
commit f7ea650142
5 changed files with 46 additions and 64 deletions

View file

@ -32,17 +32,6 @@ size_t Bindings::size() const { return attributes_.size(); }
bool Bindings::empty() { return attributes_.empty(); } bool Bindings::empty() { return attributes_.empty(); }
std::vector<const Attr*> Bindings::SortedByKeys() {
std::vector<const Attr*> res;
res.reserve(attributes_.size());
for (const auto& [key, value] : attributes_) {
res.emplace_back(&value);
}
return res;
}
Bindings::iterator Bindings::find(const Symbol& name) { Bindings::iterator Bindings::find(const Symbol& name) {
return attributes_.find(name); return attributes_.find(name);
} }

View file

@ -59,13 +59,6 @@ class Bindings {
iterator end(); iterator end();
const_iterator cend() const; const_iterator cend() const;
// Returns the elements of the attribute set as a vector, sorted
// lexicographically by keys.
//
// This is used primarily for builtins that have guaranteed
// ordering, such as `attrNames` or `attrValues`.
std::vector<const Attr*> SortedByKeys();
// oh no // oh no
friend class EvalState; friend class EvalState;

View file

@ -102,9 +102,9 @@ static void printValue(std::ostream& str, std::set<const Value*>& active,
break; break;
case tAttrs: { case tAttrs: {
str << "{ "; str << "{ ";
for (auto& i : v.attrs->SortedByKeys()) { for (const auto& [key, value] : *v.attrs) {
str << i->name << " = "; str << key << " = ";
printValue(str, active, *i->value); printValue(str, active, *value.value);
str << "; "; str << "; ";
} }
str << "}"; str << "}";

View file

@ -389,26 +389,26 @@ static void getDerivations(EvalState& state, Value& vIn,
there are names clashes between derivations, the derivation there are names clashes between derivations, the derivation
bound to the attribute with the "lower" name should take bound to the attribute with the "lower" name should take
precedence). */ precedence). */
for (auto& i : v.attrs->SortedByKeys()) { for (auto& [_, i] : *v.attrs) {
DLOG(INFO) << "evaluating attribute '" << i->name << "'"; DLOG(INFO) << "evaluating attribute '" << i.name << "'";
if (!std::regex_match(std::string(i->name), attrRegex)) { if (!std::regex_match(std::string(i.name), attrRegex)) {
continue; continue;
} }
std::string pathPrefix2 = addToPath(pathPrefix, i->name); std::string pathPrefix2 = addToPath(pathPrefix, i.name);
if (combineChannels) { if (combineChannels) {
getDerivations(state, *i->value, pathPrefix2, autoArgs, drvs, done, getDerivations(state, *i.value, pathPrefix2, autoArgs, drvs, done,
ignoreAssertionFailures); ignoreAssertionFailures);
} else if (getDerivation(state, *i->value, pathPrefix2, drvs, done, } else if (getDerivation(state, *i.value, pathPrefix2, drvs, done,
ignoreAssertionFailures)) { ignoreAssertionFailures)) {
/* If the value of this attribute is itself a set, /* If the value of this attribute is itself a set,
should we recurse into it? => Only if it has a should we recurse into it? => Only if it has a
`recurseForDerivations = true' attribute. */ `recurseForDerivations = true' attribute. */
if (i->value->type == tAttrs) { if (i.value->type == tAttrs) {
Bindings::iterator j = i->value->attrs->find( Bindings::iterator j = i.value->attrs->find(
state.symbols.Create("recurseForDerivations")); state.symbols.Create("recurseForDerivations"));
if (j != i->value->attrs->end() && if (j != i.value->attrs->end() &&
state.forceBool(*j->second.value, *j->second.pos)) { state.forceBool(*j->second.value, *j->second.pos)) {
getDerivations(state, *i->value, pathPrefix2, autoArgs, drvs, done, getDerivations(state, *i.value, pathPrefix2, autoArgs, drvs, done,
ignoreAssertionFailures); ignoreAssertionFailures);
} }
} }

View file

@ -510,11 +510,11 @@ static void prim_derivationStrict(EvalState& state, const Pos& pos,
StringSet outputs; StringSet outputs;
outputs.insert("out"); outputs.insert("out");
for (auto& i : args[0]->attrs->SortedByKeys()) { for (auto& [_, i] : *args[0]->attrs) {
if (i->name == state.sIgnoreNulls) { if (i.name == state.sIgnoreNulls) {
continue; continue;
} }
const std::string& key = i->name; const std::string& key = i.name;
auto handleHashMode = [&](const std::string& s) { auto handleHashMode = [&](const std::string& s) {
if (s == "recursive") { if (s == "recursive") {
@ -556,19 +556,19 @@ static void prim_derivationStrict(EvalState& state, const Pos& pos,
try { try {
if (ignoreNulls) { if (ignoreNulls) {
state.forceValue(*i->value); state.forceValue(*i.value);
if (i->value->type == tNull) { if (i.value->type == tNull) {
continue; continue;
} }
} }
/* The `args' attribute is special: it supplies the /* The `args' attribute is special: it supplies the
command-line arguments to the builder. */ command-line arguments to the builder. */
if (i->name == state.sArgs) { if (i.name == state.sArgs) {
state.forceList(*i->value, pos); state.forceList(*i.value, pos);
for (unsigned int n = 0; n < i->value->listSize(); ++n) { for (unsigned int n = 0; n < i.value->listSize(); ++n) {
std::string s = state.coerceToString( std::string s = state.coerceToString(posDrvName, *(*i.value->list)[n],
posDrvName, *(*i->value->list)[n], context, true); context, true);
drv.args.push_back(s); drv.args.push_back(s);
} }
} }
@ -577,48 +577,48 @@ static void prim_derivationStrict(EvalState& state, const Pos& pos,
the environment. */ the environment. */
else { else {
if (jsonObject) { if (jsonObject) {
if (i->name == state.sStructuredAttrs) { if (i.name == state.sStructuredAttrs) {
continue; continue;
} }
auto placeholder(jsonObject->placeholder(key)); auto placeholder(jsonObject->placeholder(key));
printValueAsJSON(state, true, *i->value, placeholder, context); printValueAsJSON(state, true, *i.value, placeholder, context);
if (i->name == state.sBuilder) { if (i.name == state.sBuilder) {
drv.builder = state.forceString(*i->value, context, posDrvName); drv.builder = state.forceString(*i.value, context, posDrvName);
} else if (i->name == state.sSystem) { } else if (i.name == state.sSystem) {
drv.platform = state.forceStringNoCtx(*i->value, posDrvName); drv.platform = state.forceStringNoCtx(*i.value, posDrvName);
} else if (i->name == state.sOutputHash) { } else if (i.name == state.sOutputHash) {
outputHash = state.forceStringNoCtx(*i->value, posDrvName); outputHash = state.forceStringNoCtx(*i.value, posDrvName);
} else if (i->name == state.sOutputHashAlgo) { } else if (i.name == state.sOutputHashAlgo) {
outputHashAlgo = state.forceStringNoCtx(*i->value, posDrvName); outputHashAlgo = state.forceStringNoCtx(*i.value, posDrvName);
} else if (i->name == state.sOutputHashMode) { } else if (i.name == state.sOutputHashMode) {
handleHashMode(state.forceStringNoCtx(*i->value, posDrvName)); handleHashMode(state.forceStringNoCtx(*i.value, posDrvName));
} else if (i->name == state.sOutputs) { } else if (i.name == state.sOutputs) {
/* Require outputs to be a list of strings. */ /* Require outputs to be a list of strings. */
state.forceList(*i->value, posDrvName); state.forceList(*i.value, posDrvName);
Strings ss; Strings ss;
for (unsigned int n = 0; n < i->value->listSize(); ++n) { for (unsigned int n = 0; n < i.value->listSize(); ++n) {
ss.emplace_back( ss.emplace_back(
state.forceStringNoCtx(*(*i->value->list)[n], posDrvName)); state.forceStringNoCtx(*(*i.value->list)[n], posDrvName));
} }
handleOutputs(ss); handleOutputs(ss);
} }
} else { } else {
auto s = state.coerceToString(posDrvName, *i->value, context, true); auto s = state.coerceToString(posDrvName, *i.value, context, true);
drv.env.emplace(key, s); drv.env.emplace(key, s);
if (i->name == state.sBuilder) { if (i.name == state.sBuilder) {
drv.builder = s; drv.builder = s;
} else if (i->name == state.sSystem) { } else if (i.name == state.sSystem) {
drv.platform = s; drv.platform = s;
} else if (i->name == state.sOutputHash) { } else if (i.name == state.sOutputHash) {
outputHash = s; outputHash = s;
} else if (i->name == state.sOutputHashAlgo) { } else if (i.name == state.sOutputHashAlgo) {
outputHashAlgo = s; outputHashAlgo = s;
} else if (i->name == state.sOutputHashMode) { } else if (i.name == state.sOutputHashMode) {
handleHashMode(s); handleHashMode(s);
} else if (i->name == state.sOutputs) { } else if (i.name == state.sOutputs) {
handleOutputs(absl::StrSplit(s, absl::ByAnyChar(" \t\n\r"), handleOutputs(absl::StrSplit(s, absl::ByAnyChar(" \t\n\r"),
absl::SkipEmpty())); absl::SkipEmpty()));
} }