From f7ea650142eb796eb3f2827c805cc0bc563e2183 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Tue, 22 Dec 2020 21:51:50 +0100 Subject: 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 --- third_party/nix/src/libexpr/attr-set.cc | 11 ------ third_party/nix/src/libexpr/attr-set.hh | 7 ---- third_party/nix/src/libexpr/eval.cc | 6 +-- third_party/nix/src/libexpr/get-drvs.cc | 20 +++++----- third_party/nix/src/libexpr/primops.cc | 68 ++++++++++++++++----------------- 5 files changed, 47 insertions(+), 65 deletions(-) diff --git a/third_party/nix/src/libexpr/attr-set.cc b/third_party/nix/src/libexpr/attr-set.cc index 808ffce176ee..b1617c981f51 100644 --- a/third_party/nix/src/libexpr/attr-set.cc +++ b/third_party/nix/src/libexpr/attr-set.cc @@ -32,17 +32,6 @@ size_t Bindings::size() const { return attributes_.size(); } bool Bindings::empty() { return attributes_.empty(); } -std::vector Bindings::SortedByKeys() { - std::vector res; - res.reserve(attributes_.size()); - - for (const auto& [key, value] : attributes_) { - res.emplace_back(&value); - } - - return res; -} - Bindings::iterator Bindings::find(const Symbol& name) { return attributes_.find(name); } diff --git a/third_party/nix/src/libexpr/attr-set.hh b/third_party/nix/src/libexpr/attr-set.hh index c02f3930fda5..5d77e0907cd6 100644 --- a/third_party/nix/src/libexpr/attr-set.hh +++ b/third_party/nix/src/libexpr/attr-set.hh @@ -59,13 +59,6 @@ class Bindings { iterator end(); 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 SortedByKeys(); - // oh no friend class EvalState; diff --git a/third_party/nix/src/libexpr/eval.cc b/third_party/nix/src/libexpr/eval.cc index f7b745a7b480..682ea6483213 100644 --- a/third_party/nix/src/libexpr/eval.cc +++ b/third_party/nix/src/libexpr/eval.cc @@ -102,9 +102,9 @@ static void printValue(std::ostream& str, std::set& active, break; case tAttrs: { str << "{ "; - for (auto& i : v.attrs->SortedByKeys()) { - str << i->name << " = "; - printValue(str, active, *i->value); + for (const auto& [key, value] : *v.attrs) { + str << key << " = "; + printValue(str, active, *value.value); str << "; "; } str << "}"; diff --git a/third_party/nix/src/libexpr/get-drvs.cc b/third_party/nix/src/libexpr/get-drvs.cc index d81fb5dfadc8..164c1e54f38e 100644 --- a/third_party/nix/src/libexpr/get-drvs.cc +++ b/third_party/nix/src/libexpr/get-drvs.cc @@ -389,26 +389,26 @@ static void getDerivations(EvalState& state, Value& vIn, there are names clashes between derivations, the derivation bound to the attribute with the "lower" name should take precedence). */ - for (auto& i : v.attrs->SortedByKeys()) { - DLOG(INFO) << "evaluating attribute '" << i->name << "'"; - if (!std::regex_match(std::string(i->name), attrRegex)) { + for (auto& [_, i] : *v.attrs) { + DLOG(INFO) << "evaluating attribute '" << i.name << "'"; + if (!std::regex_match(std::string(i.name), attrRegex)) { continue; } - std::string pathPrefix2 = addToPath(pathPrefix, i->name); + std::string pathPrefix2 = addToPath(pathPrefix, i.name); if (combineChannels) { - getDerivations(state, *i->value, pathPrefix2, autoArgs, drvs, done, + getDerivations(state, *i.value, pathPrefix2, autoArgs, drvs, done, ignoreAssertionFailures); - } else if (getDerivation(state, *i->value, pathPrefix2, drvs, done, + } else if (getDerivation(state, *i.value, pathPrefix2, drvs, done, ignoreAssertionFailures)) { /* If the value of this attribute is itself a set, should we recurse into it? => Only if it has a `recurseForDerivations = true' attribute. */ - if (i->value->type == tAttrs) { - Bindings::iterator j = i->value->attrs->find( + if (i.value->type == tAttrs) { + Bindings::iterator j = i.value->attrs->find( state.symbols.Create("recurseForDerivations")); - if (j != i->value->attrs->end() && + if (j != i.value->attrs->end() && 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); } } diff --git a/third_party/nix/src/libexpr/primops.cc b/third_party/nix/src/libexpr/primops.cc index 0bac7793c7e1..49c43556fe9e 100644 --- a/third_party/nix/src/libexpr/primops.cc +++ b/third_party/nix/src/libexpr/primops.cc @@ -510,11 +510,11 @@ static void prim_derivationStrict(EvalState& state, const Pos& pos, StringSet outputs; outputs.insert("out"); - for (auto& i : args[0]->attrs->SortedByKeys()) { - if (i->name == state.sIgnoreNulls) { + for (auto& [_, i] : *args[0]->attrs) { + if (i.name == state.sIgnoreNulls) { continue; } - const std::string& key = i->name; + const std::string& key = i.name; auto handleHashMode = [&](const std::string& s) { if (s == "recursive") { @@ -556,19 +556,19 @@ static void prim_derivationStrict(EvalState& state, const Pos& pos, try { if (ignoreNulls) { - state.forceValue(*i->value); - if (i->value->type == tNull) { + state.forceValue(*i.value); + if (i.value->type == tNull) { continue; } } /* The `args' attribute is special: it supplies the command-line arguments to the builder. */ - if (i->name == state.sArgs) { - state.forceList(*i->value, pos); - for (unsigned int n = 0; n < i->value->listSize(); ++n) { - std::string s = state.coerceToString( - posDrvName, *(*i->value->list)[n], context, true); + if (i.name == state.sArgs) { + state.forceList(*i.value, pos); + for (unsigned int n = 0; n < i.value->listSize(); ++n) { + std::string s = state.coerceToString(posDrvName, *(*i.value->list)[n], + context, true); drv.args.push_back(s); } } @@ -577,48 +577,48 @@ static void prim_derivationStrict(EvalState& state, const Pos& pos, the environment. */ else { if (jsonObject) { - if (i->name == state.sStructuredAttrs) { + if (i.name == state.sStructuredAttrs) { continue; } auto placeholder(jsonObject->placeholder(key)); - printValueAsJSON(state, true, *i->value, placeholder, context); - - if (i->name == state.sBuilder) { - drv.builder = state.forceString(*i->value, context, posDrvName); - } else if (i->name == state.sSystem) { - drv.platform = state.forceStringNoCtx(*i->value, posDrvName); - } else if (i->name == state.sOutputHash) { - outputHash = state.forceStringNoCtx(*i->value, posDrvName); - } else if (i->name == state.sOutputHashAlgo) { - outputHashAlgo = state.forceStringNoCtx(*i->value, posDrvName); - } else if (i->name == state.sOutputHashMode) { - handleHashMode(state.forceStringNoCtx(*i->value, posDrvName)); - } else if (i->name == state.sOutputs) { + printValueAsJSON(state, true, *i.value, placeholder, context); + + if (i.name == state.sBuilder) { + drv.builder = state.forceString(*i.value, context, posDrvName); + } else if (i.name == state.sSystem) { + drv.platform = state.forceStringNoCtx(*i.value, posDrvName); + } else if (i.name == state.sOutputHash) { + outputHash = state.forceStringNoCtx(*i.value, posDrvName); + } else if (i.name == state.sOutputHashAlgo) { + outputHashAlgo = state.forceStringNoCtx(*i.value, posDrvName); + } else if (i.name == state.sOutputHashMode) { + handleHashMode(state.forceStringNoCtx(*i.value, posDrvName)); + } else if (i.name == state.sOutputs) { /* Require ‘outputs’ to be a list of strings. */ - state.forceList(*i->value, posDrvName); + state.forceList(*i.value, posDrvName); 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( - state.forceStringNoCtx(*(*i->value->list)[n], posDrvName)); + state.forceStringNoCtx(*(*i.value->list)[n], posDrvName)); } handleOutputs(ss); } } else { - auto s = state.coerceToString(posDrvName, *i->value, context, true); + auto s = state.coerceToString(posDrvName, *i.value, context, true); drv.env.emplace(key, s); - if (i->name == state.sBuilder) { + if (i.name == state.sBuilder) { drv.builder = s; - } else if (i->name == state.sSystem) { + } else if (i.name == state.sSystem) { drv.platform = s; - } else if (i->name == state.sOutputHash) { + } else if (i.name == state.sOutputHash) { outputHash = s; - } else if (i->name == state.sOutputHashAlgo) { + } else if (i.name == state.sOutputHashAlgo) { outputHashAlgo = s; - } else if (i->name == state.sOutputHashMode) { + } else if (i.name == state.sOutputHashMode) { handleHashMode(s); - } else if (i->name == state.sOutputs) { + } else if (i.name == state.sOutputs) { handleOutputs(absl::StrSplit(s, absl::ByAnyChar(" \t\n\r"), absl::SkipEmpty())); } -- cgit 1.4.1