From 242f9bf3dc04170502020fb0338b78ea76b9ebac Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 30 Sep 2021 21:31:21 +0000 Subject: `std::visit` by reference I had started the trend of doing `std::visit` by value (because a type error once mislead me into thinking that was the only form that existed). While the optomizer in principle should be able to deal with extra coppying or extra indirection once the lambdas inlined, sticking with by reference is the conventional default. I hope this might even improve performance. --- src/libstore/build/entry-points.cc | 6 ++-- src/libstore/build/local-derivation-goal.cc | 18 ++++++------ src/libstore/content-address.cc | 16 +++++------ src/libstore/daemon.cc | 4 +-- src/libstore/derivations.cc | 44 ++++++++++++++--------------- src/libstore/derived-path.cc | 8 +++--- src/libstore/legacy-ssh-store.cc | 4 +-- src/libstore/local-store.cc | 8 +++--- src/libstore/misc.cc | 4 +-- src/libstore/path-with-outputs.cc | 4 +-- src/libstore/remote-store.cc | 8 +++--- src/libstore/store-api.cc | 8 +++--- 12 files changed, 66 insertions(+), 66 deletions(-) (limited to 'src/libstore') diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index 96deb81d1..2b77e4354 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -11,12 +11,12 @@ void Store::buildPaths(const std::vector & reqs, BuildMode buildMod Worker worker(*this, evalStore ? *evalStore : *this); Goals goals; - for (auto & br : reqs) { + for (const auto & br : reqs) { std::visit(overloaded { - [&](DerivedPath::Built bfd) { + [&](const DerivedPath::Built & bfd) { goals.insert(worker.makeDerivationGoal(bfd.drvPath, bfd.outputs, buildMode)); }, - [&](DerivedPath::Opaque bo) { + [&](const DerivedPath::Opaque & bo) { goals.insert(worker.makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair)); }, }, br.raw()); diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 3aa9f12fe..e91e35851 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1094,10 +1094,10 @@ void LocalDerivationGoal::writeStructuredAttrs() static StorePath pathPartOfReq(const DerivedPath & req) { return std::visit(overloaded { - [&](DerivedPath::Opaque bo) { + [&](const DerivedPath::Opaque & bo) { return bo.path; }, - [&](DerivedPath::Built bfd) { + [&](const DerivedPath::Built & bfd) { return bfd.drvPath; }, }, req.raw()); @@ -2155,8 +2155,8 @@ void LocalDerivationGoal::registerOutputs() /* Since we'll use the already installed versions of these, we can treat them as leaves and ignore any references they have. */ - [&](AlreadyRegistered _) { return StringSet {}; }, - [&](PerhapsNeedToRegister refs) { + [&](const AlreadyRegistered &) { return StringSet {}; }, + [&](const PerhapsNeedToRegister & refs) { StringSet referencedOutputs; /* FIXME build inverted map up front so no quadratic waste here */ for (auto & r : refs.refs) @@ -2192,11 +2192,11 @@ void LocalDerivationGoal::registerOutputs() }; std::optional referencesOpt = std::visit(overloaded { - [&](AlreadyRegistered skippedFinalPath) -> std::optional { + [&](const AlreadyRegistered & skippedFinalPath) -> std::optional { finish(skippedFinalPath.path); return std::nullopt; }, - [&](PerhapsNeedToRegister r) -> std::optional { + [&](const PerhapsNeedToRegister & r) -> std::optional { return r.refs; }, }, outputReferencesIfUnregistered.at(outputName)); @@ -2312,7 +2312,7 @@ void LocalDerivationGoal::registerOutputs() }; ValidPathInfo newInfo = std::visit(overloaded { - [&](DerivationOutputInputAddressed output) { + [&](const DerivationOutputInputAddressed & output) { /* input-addressed case */ auto requiredFinalPath = output.path; /* Preemptively add rewrite rule for final hash, as that is @@ -2331,14 +2331,14 @@ void LocalDerivationGoal::registerOutputs() newInfo0.references.insert(newInfo0.path); return newInfo0; }, - [&](DerivationOutputCAFixed dof) { + [&](const DerivationOutputCAFixed & dof) { auto newInfo0 = newInfoFromCA(DerivationOutputCAFloating { .method = dof.hash.method, .hashType = dof.hash.hash.type, }); /* Check wanted hash */ - Hash & wanted = dof.hash.hash; + const Hash & wanted = dof.hash.hash; assert(newInfo0.ca); auto got = getContentAddressHash(*newInfo0.ca); if (wanted != got) { diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 90a3ad1f5..974d1c471 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -31,10 +31,10 @@ std::string makeFixedOutputCA(FileIngestionMethod method, const Hash & hash) std::string renderContentAddress(ContentAddress ca) { return std::visit(overloaded { - [](TextHash th) { + [](TextHash & th) { return "text:" + th.hash.to_string(Base32, true); }, - [](FixedOutputHash fsh) { + [](FixedOutputHash & fsh) { return makeFixedOutputCA(fsh.method, fsh.hash); } }, ca); @@ -43,10 +43,10 @@ std::string renderContentAddress(ContentAddress ca) std::string renderContentAddressMethod(ContentAddressMethod cam) { return std::visit(overloaded { - [](TextHashMethod &th) { + [](TextHashMethod & th) { return std::string{"text:"} + printHashType(htSHA256); }, - [](FixedOutputHashMethod &fshm) { + [](FixedOutputHashMethod & fshm) { return "fixed:" + makeFileIngestionPrefix(fshm.fileIngestionMethod) + printHashType(fshm.hashType); } }, cam); @@ -104,12 +104,12 @@ ContentAddress parseContentAddress(std::string_view rawCa) { return std::visit( overloaded { - [&](TextHashMethod thm) { + [&](TextHashMethod & thm) { return ContentAddress(TextHash { .hash = Hash::parseNonSRIUnprefixed(rest, htSHA256) }); }, - [&](FixedOutputHashMethod fohMethod) { + [&](FixedOutputHashMethod & fohMethod) { return ContentAddress(FixedOutputHash { .method = fohMethod.fileIngestionMethod, .hash = Hash::parseNonSRIUnprefixed(rest, std::move(fohMethod.hashType)), @@ -137,10 +137,10 @@ std::string renderContentAddress(std::optional ca) Hash getContentAddressHash(const ContentAddress & ca) { return std::visit(overloaded { - [](TextHash th) { + [](const TextHash & th) { return th.hash; }, - [](FixedOutputHash fsh) { + [](const FixedOutputHash & fsh) { return fsh.hash; } }, ca); diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 487416a13..164a9b2be 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -395,13 +395,13 @@ static void performOp(TunnelLogger * logger, ref store, FramedSource source(from); // TODO this is essentially RemoteStore::addCAToStore. Move it up to Store. return std::visit(overloaded { - [&](TextHashMethod &_) { + [&](TextHashMethod &) { // We could stream this by changing Store std::string contents = source.drain(); auto path = store->addTextToStore(name, contents, refs, repair); return store->queryPathInfo(path); }, - [&](FixedOutputHashMethod &fohm) { + [&](FixedOutputHashMethod & fohm) { if (!refs.empty()) throw UnimplementedError("cannot yet have refs with flat or nar-hashed data"); auto path = store->addToStoreFromDump(source, name, fohm.fileIngestionMethod, fohm.hashType, repair); diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 899475860..ef8765841 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -10,18 +10,18 @@ namespace nix { std::optional DerivationOutput::path(const Store & store, std::string_view drvName, std::string_view outputName) const { return std::visit(overloaded { - [](DerivationOutputInputAddressed doi) -> std::optional { + [](const DerivationOutputInputAddressed & doi) -> std::optional { return { doi.path }; }, - [&](DerivationOutputCAFixed dof) -> std::optional { + [&](const DerivationOutputCAFixed & dof) -> std::optional { return { dof.path(store, drvName, outputName) }; }, - [](DerivationOutputCAFloating dof) -> std::optional { + [](const DerivationOutputCAFloating & dof) -> std::optional { return std::nullopt; }, - [](DerivationOutputDeferred) -> std::optional { + [](const DerivationOutputDeferred &) -> std::optional { return std::nullopt; }, }, output); @@ -332,22 +332,22 @@ string Derivation::unparse(const Store & store, bool maskOutputs, if (first) first = false; else s += ','; s += '('; printUnquotedString(s, i.first); std::visit(overloaded { - [&](DerivationOutputInputAddressed doi) { + [&](const DerivationOutputInputAddressed & doi) { s += ','; printUnquotedString(s, maskOutputs ? "" : store.printStorePath(doi.path)); s += ','; printUnquotedString(s, ""); s += ','; printUnquotedString(s, ""); }, - [&](DerivationOutputCAFixed dof) { + [&](const DerivationOutputCAFixed & dof) { s += ','; printUnquotedString(s, maskOutputs ? "" : store.printStorePath(dof.path(store, name, i.first))); s += ','; printUnquotedString(s, dof.hash.printMethodAlgo()); s += ','; printUnquotedString(s, dof.hash.hash.to_string(Base16, false)); }, - [&](DerivationOutputCAFloating dof) { + [&](const DerivationOutputCAFloating & dof) { s += ','; printUnquotedString(s, ""); s += ','; printUnquotedString(s, makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)); s += ','; printUnquotedString(s, ""); }, - [&](DerivationOutputDeferred) { + [&](const DerivationOutputDeferred &) { s += ','; printUnquotedString(s, ""); s += ','; printUnquotedString(s, ""); s += ','; printUnquotedString(s, ""); @@ -420,13 +420,13 @@ DerivationType BasicDerivation::type() const std::optional floatingHashType; for (auto & i : outputs) { std::visit(overloaded { - [&](DerivationOutputInputAddressed _) { + [&](const DerivationOutputInputAddressed &) { inputAddressedOutputs.insert(i.first); }, - [&](DerivationOutputCAFixed _) { + [&](const DerivationOutputCAFixed &) { fixedCAOutputs.insert(i.first); }, - [&](DerivationOutputCAFloating dof) { + [&](const DerivationOutputCAFloating & dof) { floatingCAOutputs.insert(i.first); if (!floatingHashType) { floatingHashType = dof.hashType; @@ -435,7 +435,7 @@ DerivationType BasicDerivation::type() const throw Error("All floating outputs must use the same hash type"); } }, - [&](DerivationOutputDeferred _) { + [&](const DerivationOutputDeferred &) { deferredIAOutputs.insert(i.first); }, }, i.second.output); @@ -538,15 +538,15 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m const auto & res = pathDerivationModulo(store, i.first); std::visit(overloaded { // Regular non-CA derivation, replace derivation - [&](Hash drvHash) { + [&](const Hash & drvHash) { inputs2.insert_or_assign(drvHash.to_string(Base16, false), i.second); }, - [&](DeferredHash deferredHash) { + [&](const DeferredHash & deferredHash) { isDeferred = true; inputs2.insert_or_assign(deferredHash.hash.to_string(Base16, false), i.second); }, // CA derivation's output hashes - [&](CaOutputHashes outputHashes) { + [&](const CaOutputHashes & outputHashes) { std::set justOut = { "out" }; for (auto & output : i.second) { /* Put each one in with a single "out" output.. */ @@ -572,17 +572,17 @@ std::map staticOutputHashes(Store & store, const Derivation & { std::map res; std::visit(overloaded { - [&](Hash drvHash) { + [&](const Hash & drvHash) { for (auto & outputName : drv.outputNames()) { res.insert({outputName, drvHash}); } }, - [&](DeferredHash deferredHash) { + [&](const DeferredHash & deferredHash) { for (auto & outputName : drv.outputNames()) { res.insert({outputName, deferredHash.hash}); } }, - [&](CaOutputHashes outputHashes) { + [&](const CaOutputHashes & outputHashes) { res = outputHashes; }, }, hashDerivationModulo(store, drv, true)); @@ -666,22 +666,22 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr for (auto & i : drv.outputs) { out << i.first; std::visit(overloaded { - [&](DerivationOutputInputAddressed doi) { + [&](const DerivationOutputInputAddressed & doi) { out << store.printStorePath(doi.path) << "" << ""; }, - [&](DerivationOutputCAFixed dof) { + [&](const DerivationOutputCAFixed & dof) { out << store.printStorePath(dof.path(store, drv.name, i.first)) << dof.hash.printMethodAlgo() << dof.hash.hash.to_string(Base16, false); }, - [&](DerivationOutputCAFloating dof) { + [&](const DerivationOutputCAFloating & dof) { out << "" << (makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)) << ""; }, - [&](DerivationOutputDeferred) { + [&](const DerivationOutputDeferred &) { out << "" << "" << ""; diff --git a/src/libstore/derived-path.cc b/src/libstore/derived-path.cc index 8da81d0ac..e55af21e9 100644 --- a/src/libstore/derived-path.cc +++ b/src/libstore/derived-path.cc @@ -24,8 +24,8 @@ StorePathSet BuiltPath::outPaths() const { return std::visit( overloaded{ - [](BuiltPath::Opaque p) { return StorePathSet{p.path}; }, - [](BuiltPath::Built b) { + [](const BuiltPath::Opaque & p) { return StorePathSet{p.path}; }, + [](const BuiltPath::Built & b) { StorePathSet res; for (auto & [_, path] : b.outputs) res.insert(path); @@ -94,8 +94,8 @@ RealisedPath::Set BuiltPath::toRealisedPaths(Store & store) const RealisedPath::Set res; std::visit( overloaded{ - [&](BuiltPath::Opaque p) { res.insert(p.path); }, - [&](BuiltPath::Built p) { + [&](const BuiltPath::Opaque & p) { res.insert(p.path); }, + [&](const BuiltPath::Built & p) { auto drvHashes = staticOutputHashes(store, store.readDerivation(p.drvPath)); for (auto& [outputName, outputPath] : p.outputs) { diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index c660d7f5b..7ae4e7c24 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -290,10 +290,10 @@ public: for (auto & p : drvPaths) { auto sOrDrvPath = StorePathWithOutputs::tryFromDerivedPath(p); std::visit(overloaded { - [&](StorePathWithOutputs s) { + [&](const StorePathWithOutputs & s) { ss.push_back(s.to_string(*this)); }, - [&](StorePath drvPath) { + [&](const StorePath & drvPath) { throw Error("wanted to fetch '%s' but the legacy ssh protocol doesn't support merely substituting drv files via the build paths command. It would build them instead. Try using ssh-ng://", printStorePath(drvPath)); }, }, sOrDrvPath); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 634e9eb8b..5b2490472 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -681,7 +681,7 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat std::optional h; for (auto & i : drv.outputs) { std::visit(overloaded { - [&](DerivationOutputInputAddressed doia) { + [&](const DerivationOutputInputAddressed & doia) { if (!h) { // somewhat expensive so we do lazily auto temp = hashDerivationModulo(*this, drv, true); @@ -693,14 +693,14 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat printStorePath(drvPath), printStorePath(doia.path), printStorePath(recomputed)); envHasRightPath(doia.path, i.first); }, - [&](DerivationOutputCAFixed dof) { + [&](const DerivationOutputCAFixed & dof) { StorePath path = makeFixedOutputPath(dof.hash.method, dof.hash.hash, drvName); envHasRightPath(path, i.first); }, - [&](DerivationOutputCAFloating _) { + [&](const DerivationOutputCAFloating &) { /* Nothing to check */ }, - [&](DerivationOutputDeferred) { + [&](const DerivationOutputDeferred &) { }, }, i.second.output); } diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index b4929b445..f184dd857 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -166,7 +166,7 @@ void Store::queryMissing(const std::vector & targets, } std::visit(overloaded { - [&](DerivedPath::Built bfd) { + [&](const DerivedPath::Built & bfd) { if (!isValidPath(bfd.drvPath)) { // FIXME: we could try to substitute the derivation. auto state(state_.lock()); @@ -199,7 +199,7 @@ void Store::queryMissing(const std::vector & targets, mustBuildDrv(bfd.drvPath, *drv); }, - [&](DerivedPath::Opaque bo) { + [&](const DerivedPath::Opaque & bo) { if (isValidPath(bo.path)) return; diff --git a/src/libstore/path-with-outputs.cc b/src/libstore/path-with-outputs.cc index 865d64cf2..e5a121e00 100644 --- a/src/libstore/path-with-outputs.cc +++ b/src/libstore/path-with-outputs.cc @@ -31,14 +31,14 @@ std::vector toDerivedPaths(const std::vector std::variant StorePathWithOutputs::tryFromDerivedPath(const DerivedPath & p) { return std::visit(overloaded { - [&](DerivedPath::Opaque bo) -> std::variant { + [&](const DerivedPath::Opaque & bo) -> std::variant { if (bo.path.isDerivation()) { // drv path gets interpreted as "build", not "get drv file itself" return bo.path; } return StorePathWithOutputs { bo.path }; }, - [&](DerivedPath::Built bfd) -> std::variant { + [&](const DerivedPath::Built & bfd) -> std::variant { return StorePathWithOutputs { bfd.drvPath, bfd.outputs }; }, }, p.raw()); diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index b57a4cb05..fa5ea8af7 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -528,13 +528,13 @@ ref RemoteStore::addCAToStore( if (repair) throw Error("repairing is not supported when building through the Nix daemon protocol < 1.25"); std::visit(overloaded { - [&](TextHashMethod thm) -> void { + [&](const TextHashMethod & thm) -> void { std::string s = dump.drain(); conn->to << wopAddTextToStore << name << s; worker_proto::write(*this, conn->to, references); conn.processStderr(); }, - [&](FixedOutputHashMethod fohm) -> void { + [&](const FixedOutputHashMethod & fohm) -> void { conn->to << wopAddToStore << name @@ -705,10 +705,10 @@ static void writeDerivedPaths(RemoteStore & store, ConnectionHandle & conn, cons for (auto & p : reqs) { auto sOrDrvPath = StorePathWithOutputs::tryFromDerivedPath(p); std::visit(overloaded { - [&](StorePathWithOutputs s) { + [&](const StorePathWithOutputs & s) { ss.push_back(s.to_string(store)); }, - [&](StorePath drvPath) { + [&](const StorePath & drvPath) { throw Error("trying to request '%s', but daemon protocol %d.%d is too old (< 1.29) to request a derivation file", store.printStorePath(drvPath), GET_PROTOCOL_MAJOR(conn->daemonVersion), diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 970bafd88..b5ff3dccf 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -199,10 +199,10 @@ StorePath Store::makeFixedOutputPathFromCA(std::string_view name, ContentAddress { // New template return std::visit(overloaded { - [&](TextHash th) { + [&](const TextHash & th) { return makeTextPath(name, th.hash, references); }, - [&](FixedOutputHash fsh) { + [&](const FixedOutputHash & fsh) { return makeFixedOutputPath(fsh.method, fsh.hash, name, references, hasSelfReference); } }, ca); @@ -1114,10 +1114,10 @@ bool ValidPathInfo::isContentAddressed(const Store & store) const if (! ca) return false; auto caPath = std::visit(overloaded { - [&](TextHash th) { + [&](const TextHash & th) { return store.makeTextPath(path.name(), th.hash, references); }, - [&](FixedOutputHash fsh) { + [&](const FixedOutputHash & fsh) { auto refs = references; bool hasSelfReference = false; if (refs.count(path)) { -- cgit v1.2.3