From 474695975dde60f582ca0b2fb72c17f664e22876 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 26 Apr 2022 14:01:21 +0200 Subject: [PATCH] EvalCache: Revert to using symbols in getAttr() --- src/libcmd/installables.cc | 2 +- src/libexpr/eval-cache.cc | 38 +++++++++++++++++++++++++------------- src/libexpr/eval-cache.hh | 8 ++++++-- src/nix/app.cc | 6 +++--- src/nix/flake.cc | 2 +- src/nix/search.cc | 2 +- 6 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 693b5e2a2..e3210d18d 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -600,7 +600,7 @@ std::tuple InstallableF auto drvInfo = DerivationInfo { std::move(drvPath), - attr->getAttr("outputName")->getString() + attr->getAttr(state->sOutputName)->getString() }; return {attrPath, getLockedFlake()->flake.lockedRef, std::move(drvInfo)}; diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index 00e80ae3b..e90c9bff5 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -420,8 +420,10 @@ Suggestions AttrCursor::getSuggestionsForAttr(Symbol name) return Suggestions::bestMatches(strAttrNames, root->state.symbols[name]); } -std::shared_ptr AttrCursor::maybeGetAttr(std::string_view name, bool forceErrors) +std::shared_ptr AttrCursor::maybeGetAttr(Symbol name, bool forceErrors) { + auto nameS = root->state.symbols[name]; + if (root->db) { if (!cachedValue) cachedValue = root->db->getAttr(getKey(), root->state.symbols); @@ -429,11 +431,11 @@ std::shared_ptr AttrCursor::maybeGetAttr(std::string_view name, bool if (cachedValue) { if (auto attrs = std::get_if>(&cachedValue->second)) { for (auto & attr : *attrs) - if (root->state.symbols[attr] == name) + if (attr == name) return std::make_shared(root, std::make_pair(shared_from_this(), attr)); return nullptr; } else if (std::get_if(&cachedValue->second)) { - auto attr = root->db->getAttr({cachedValue->first, std::string(name)}, root->state.symbols); + auto attr = root->db->getAttr({cachedValue->first, nameS}, root->state.symbols); if (attr) { if (std::get_if(&attr->second)) return nullptr; @@ -441,10 +443,10 @@ std::shared_ptr AttrCursor::maybeGetAttr(std::string_view name, bool if (forceErrors) debug("reevaluating failed cached attribute '%s'"); else - throw CachedEvalError("cached failure of attribute '%s'", getAttrPathStr(root->state.symbols.create(name))); + throw CachedEvalError("cached failure of attribute '%s'", getAttrPathStr(name)); } else return std::make_shared(root, - std::make_pair(shared_from_this(), root->state.symbols.create(name)), nullptr, std::move(attr)); + std::make_pair(shared_from_this(), name), nullptr, std::move(attr)); } // Incomplete attrset, so need to fall thru and // evaluate to see whether 'name' exists @@ -465,13 +467,13 @@ std::shared_ptr AttrCursor::maybeGetAttr(std::string_view name, bool root->db->setPlaceholder({cachedValue->first, root->state.symbols[attr.name]}); } - auto attr = v.attrs->get(root->state.symbols.create(name)); + auto attr = v.attrs->get(name); if (!attr) { if (root->db) { if (!cachedValue) cachedValue = {root->db->setPlaceholder(getKey()), placeholder_t()}; - root->db->setMissing({cachedValue->first, std::string(name)}); + root->db->setMissing({cachedValue->first, nameS}); } return nullptr; } @@ -480,26 +482,36 @@ std::shared_ptr AttrCursor::maybeGetAttr(std::string_view name, bool if (root->db) { if (!cachedValue) cachedValue = {root->db->setPlaceholder(getKey()), placeholder_t()}; - cachedValue2 = {root->db->setPlaceholder({cachedValue->first, std::string(name)}), placeholder_t()}; + cachedValue2 = {root->db->setPlaceholder({cachedValue->first, nameS}), placeholder_t()}; } return make_ref( - root, std::make_pair(shared_from_this(), root->state.symbols.create(name)), attr->value, std::move(cachedValue2)); + root, std::make_pair(shared_from_this(), name), attr->value, std::move(cachedValue2)); } -ref AttrCursor::getAttr(std::string_view name, bool forceErrors) +std::shared_ptr AttrCursor::maybeGetAttr(std::string_view name) +{ + return maybeGetAttr(root->state.symbols.create(name)); +} + +ref AttrCursor::getAttr(Symbol name, bool forceErrors) { auto p = maybeGetAttr(name, forceErrors); if (!p) - throw Error("attribute '%s' does not exist", getAttrPathStr(root->state.symbols.create(name))); + throw Error("attribute '%s' does not exist", getAttrPathStr(name)); return ref(p); } +ref AttrCursor::getAttr(std::string_view name) +{ + return getAttr(root->state.symbols.create(name)); +} + OrSuggestions> AttrCursor::findAlongAttrPath(const std::vector & attrPath, bool force) { auto res = shared_from_this(); for (auto & attr : attrPath) { - auto child = res->maybeGetAttr(root->state.symbols[attr], force); + auto child = res->maybeGetAttr(attr, force); if (!child) { auto suggestions = res->getSuggestionsForAttr(attr); return OrSuggestions>::failed(suggestions); @@ -627,7 +639,7 @@ bool AttrCursor::isDerivation() StorePath AttrCursor::forceDerivation() { - auto aDrvPath = getAttr("drvPath", true); + auto aDrvPath = getAttr(root->state.sDrvPath, true); auto drvPath = root->state.store->parseStorePath(aDrvPath->getString()); if (!root->state.store->isValidPath(drvPath) && !settings.readOnlyMode) { /* The eval cache contains 'drvPath', but the actual path has diff --git a/src/libexpr/eval-cache.hh b/src/libexpr/eval-cache.hh index 288ecd7e3..0ba2e4ed0 100644 --- a/src/libexpr/eval-cache.hh +++ b/src/libexpr/eval-cache.hh @@ -96,9 +96,13 @@ public: Suggestions getSuggestionsForAttr(Symbol name); - std::shared_ptr maybeGetAttr(std::string_view name, bool forceErrors = false); + std::shared_ptr maybeGetAttr(Symbol name, bool forceErrors = false); - ref getAttr(std::string_view name, bool forceErrors = false); + std::shared_ptr maybeGetAttr(std::string_view name); + + ref getAttr(Symbol name, bool forceErrors = false); + + ref getAttr(std::string_view name); /* Get an attribute along a chain of attrsets. Note that this does not auto-call functors or functions. */ diff --git a/src/nix/app.cc b/src/nix/app.cc index eec53ad7c..cce84d026 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -85,9 +85,9 @@ UnresolvedApp Installable::toApp(EvalState & state) else if (type == "derivation") { auto drvPath = cursor->forceDerivation(); - auto outPath = cursor->getAttr("outPath")->getString(); - auto outputName = cursor->getAttr("outputName")->getString(); - auto name = cursor->getAttr("name")->getString(); + auto outPath = cursor->getAttr(state.sOutPath)->getString(); + auto outputName = cursor->getAttr(state.sOutputName)->getString(); + auto name = cursor->getAttr(state.sName)->getString(); auto aPname = cursor->maybeGetAttr("pname"); auto aMeta = cursor->maybeGetAttr("meta"); auto aMainProgram = aMeta ? aMeta->maybeGetAttr("mainProgram") : nullptr; diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 2c4a64c85..040c1c7af 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -1012,7 +1012,7 @@ struct CmdFlakeShow : FlakeCommand, MixJSON auto showDerivation = [&]() { - auto name = visitor.getAttr("name")->getString(); + auto name = visitor.getAttr(state->sName)->getString(); if (json) { std::optional description; if (auto aMeta = visitor.maybeGetAttr("meta")) { diff --git a/src/nix/search.cc b/src/nix/search.cc index 6febc0a55..76451f810 100644 --- a/src/nix/search.cc +++ b/src/nix/search.cc @@ -156,7 +156,7 @@ struct CmdSearch : InstallableCommand, MixJSON recurse(); else if (attrPathS[0] == "legacyPackages" && attrPath.size() > 2) { - auto attr = cursor.maybeGetAttr("recurseForDerivations"); + auto attr = cursor.maybeGetAttr(state->sRecurseForDerivations); if (attr && attr->getBool()) recurse(); }