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.
2021-09-30 21:31:21 +00:00 · 2021-09-30 21:31:21 +00:00 · 242f9bf3dc
parent 6a8d6246f6
commit 242f9bf3dc
19 changed files with 88 additions and 88 deletions
--- a/src/libcmd/command.cc
+++ b/src/libcmd/command.cc
@ -203,10 +203,10 @@ void MixProfile::updateProfile(const BuiltPaths & buildables)

    for (auto & buildable : buildables) {
        std::visit(overloaded {
-            [&](BuiltPath::Opaque bo) {
+            [&](const BuiltPath::Opaque & bo) {
                result.push_back(bo.path);
            },
-            [&](BuiltPath::Built bfd) {
+            [&](const BuiltPath::Built & bfd) {
                for (auto & output : bfd.outputs) {
                    result.push_back(output.second);
                }
--- a/src/libcmd/installables.cc
+++ b/src/libcmd/installables.cc
@ -697,13 +697,13 @@ std::shared_ptr<Installable> SourceExprCommand::parseInstallable(
 BuiltPaths getBuiltPaths(ref<Store> evalStore, ref<Store> store, const DerivedPaths & hopefullyBuiltPaths)
 {
    BuiltPaths res;
-    for (auto & b : hopefullyBuiltPaths)
+    for (const auto & b : hopefullyBuiltPaths)
        std::visit(
            overloaded{
-                [&](DerivedPath::Opaque bo) {
+                [&](const DerivedPath::Opaque & bo) {
                    res.push_back(BuiltPath::Opaque{bo.path});
                },
-                [&](DerivedPath::Built bfd) {
+                [&](const DerivedPath::Built & bfd) {
                    OutputPathMap outputs;
                    auto drv = evalStore->readDerivation(bfd.drvPath);
                    auto outputHashes = staticOutputHashes(*evalStore, drv); // FIXME: expensive
@ -823,10 +823,10 @@ StorePathSet toDerivations(
 {
    StorePathSet drvPaths;

-    for (auto & i : installables)
-        for (auto & b : i->toDerivedPaths())
+    for (const auto & i : installables)
+        for (const auto & b : i->toDerivedPaths())
            std::visit(overloaded {
-                [&](DerivedPath::Opaque bo) {
+                [&](const DerivedPath::Opaque & bo) {
                    if (!useDeriver)
                        throw Error("argument '%s' did not evaluate to a derivation", i->what());
                    auto derivers = store->queryValidDerivers(bo.path);
@ -835,7 +835,7 @@ StorePathSet toDerivations(
                    // FIXME: use all derivers?
                    drvPaths.insert(*derivers.begin());
                },
-                [&](DerivedPath::Built bfd) {
+                [&](const DerivedPath::Built & bfd) {
                    drvPaths.insert(bfd.drvPath);
                },
            }, b.raw());
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@ -1174,7 +1174,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * *
        // hash per output.
        auto hashModulo = hashDerivationModulo(*state.store, Derivation(drv), true);
        std::visit(overloaded {
-            [&](Hash h) {
+            [&](Hash & h) {
                for (auto & i : outputs) {
                    auto outPath = state.store->makeOutputPath(i, h, drvName);
                    drv.env[i] = state.store->printStorePath(outPath);
@ -1186,11 +1186,11 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * *
                        });
                }
            },
-            [&](CaOutputHashes) {
+            [&](CaOutputHashes &) {
                // Shouldn't happen as the toplevel derivation is not CA.
                assert(false);
            },
-            [&](DeferredHash _) {
+            [&](DeferredHash &) {
                for (auto & i : outputs) {
                    drv.outputs.insert_or_assign(i,
                        DerivationOutput {
--- a/src/libstore/build/entry-points.cc
+++ b/src/libstore/build/entry-points.cc
@ -11,12 +11,12 @@ void Store::buildPaths(const std::vector<DerivedPath> & 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());
--- 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<StorePathSet> referencesOpt = std::visit(overloaded {
-            [&](AlreadyRegistered skippedFinalPath) -> std::optional<StorePathSet> {
+            [&](const AlreadyRegistered & skippedFinalPath) -> std::optional<StorePathSet> {
                finish(skippedFinalPath.path);
                return std::nullopt;
            },
-            [&](PerhapsNeedToRegister r) -> std::optional<StorePathSet> {
+            [&](const PerhapsNeedToRegister & r) -> std::optional<StorePathSet> {
                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) {
--- 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);
@ -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<ContentAddress> 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);
--- a/src/libstore/daemon.cc
+++ b/src/libstore/daemon.cc
@ -395,7 +395,7 @@ static void performOp(TunnelLogger * logger, ref<Store> 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);
--- a/src/libstore/derivations.cc
+++ b/src/libstore/derivations.cc
@ -10,18 +10,18 @@ namespace nix {
 std::optional<StorePath> DerivationOutput::path(const Store & store, std::string_view drvName, std::string_view outputName) const
 {
    return std::visit(overloaded {
-        [](DerivationOutputInputAddressed doi) -> std::optional<StorePath> {
+        [](const DerivationOutputInputAddressed & doi) -> std::optional<StorePath> {
            return { doi.path };
        },
-        [&](DerivationOutputCAFixed dof) -> std::optional<StorePath> {
+        [&](const DerivationOutputCAFixed & dof) -> std::optional<StorePath> {
            return {
                dof.path(store, drvName, outputName)
            };
        },
-        [](DerivationOutputCAFloating dof) -> std::optional<StorePath> {
+        [](const DerivationOutputCAFloating & dof) -> std::optional<StorePath> {
            return std::nullopt;
        },
-        [](DerivationOutputDeferred) -> std::optional<StorePath> {
+        [](const DerivationOutputDeferred &) -> std::optional<StorePath> {
            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<HashType> 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<std::string> justOut = { "out" };
                for (auto & output : i.second) {
                    /* Put each one in with a single "out" output.. */
@ -572,17 +572,17 @@ std::map<std::string, Hash> staticOutputHashes(Store & store, const Derivation &
 {
    std::map<std::string, Hash> 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 << ""
                    << ""
                    << "";
--- 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) {
--- 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);
--- 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<Hash> 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);
    }
--- a/src/libstore/misc.cc
+++ b/src/libstore/misc.cc
@ -166,7 +166,7 @@ void Store::queryMissing(const std::vector<DerivedPath> & 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<DerivedPath> & targets,
                mustBuildDrv(bfd.drvPath, *drv);

          },
-          [&](DerivedPath::Opaque bo) {
+          [&](const DerivedPath::Opaque & bo) {

            if (isValidPath(bo.path)) return;

--- a/src/libstore/path-with-outputs.cc
+++ b/src/libstore/path-with-outputs.cc
@ -31,14 +31,14 @@ std::vector<DerivedPath> toDerivedPaths(const std::vector<StorePathWithOutputs>
 std::variant<StorePathWithOutputs, StorePath> StorePathWithOutputs::tryFromDerivedPath(const DerivedPath & p)
 {
    return std::visit(overloaded {
-        [&](DerivedPath::Opaque bo) -> std::variant<StorePathWithOutputs, StorePath> {
+        [&](const DerivedPath::Opaque & bo) -> std::variant<StorePathWithOutputs, StorePath> {
            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<StorePathWithOutputs, StorePath> {
+        [&](const DerivedPath::Built & bfd) -> std::variant<StorePathWithOutputs, StorePath> {
            return StorePathWithOutputs { bfd.drvPath, bfd.outputs };
        },
    }, p.raw());
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@ -528,13 +528,13 @@ ref<const ValidPathInfo> 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),
--- 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)) {
--- a/src/nix/build.cc
+++ b/src/nix/build.cc
@ -66,12 +66,12 @@ struct CmdBuild : InstallablesCommand, MixDryRun, MixJSON, MixProfile
                for (const auto & [_i, buildable] : enumerate(buildables)) {
                    auto i = _i;
                    std::visit(overloaded {
-                        [&](BuiltPath::Opaque bo) {
+                        [&](const BuiltPath::Opaque & bo) {
                            std::string symlink = outLink;
                            if (i) symlink += fmt("-%d", i);
                            store2->addPermRoot(bo.path, absPath(symlink));
                        },
-                        [&](BuiltPath::Built bfd) {
+                        [&](const BuiltPath::Built & bfd) {
                            for (auto & output : bfd.outputs) {
                                std::string symlink = outLink;
                                if (i) symlink += fmt("-%d", i);
--- a/src/nix/log.cc
+++ b/src/nix/log.cc
@ -35,10 +35,10 @@ struct CmdLog : InstallableCommand
        RunPager pager;
        for (auto & sub : subs) {
            auto log = std::visit(overloaded {
-                [&](DerivedPath::Opaque bo) {
+                [&](const DerivedPath::Opaque & bo) {
                    return sub->getBuildLog(bo.path);
                },
-                [&](DerivedPath::Built bfd) {
+                [&](const DerivedPath::Built & bfd) {
                    return sub->getBuildLog(bfd.drvPath);
                },
            }, b.raw());
--- a/src/nix/profile.cc
+++ b/src/nix/profile.cc
@ -260,11 +260,11 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile
                    ProfileElement element;

                    std::visit(overloaded {
-                        [&](BuiltPath::Opaque bo) {
+                        [&](const BuiltPath::Opaque & bo) {
                            pathsToBuild.push_back(bo);
                            element.storePaths.insert(bo.path);
                        },
-                        [&](BuiltPath::Built bfd) {
+                        [&](const BuiltPath::Built & bfd) {
                            // TODO: Why are we querying if we know the output
                            // names already? Is it just to figure out what the
                            // default one is?
--- a/src/nix/show-derivation.cc
+++ b/src/nix/show-derivation.cc
@ -65,18 +65,18 @@ struct CmdShowDerivation : InstallablesCommand
                    auto & outputName = _outputName; // work around clang bug
                    auto outputObj { outputsObj.object(outputName) };
                    std::visit(overloaded {
-                        [&](DerivationOutputInputAddressed doi) {
+                        [&](const DerivationOutputInputAddressed & doi) {
                            outputObj.attr("path", store->printStorePath(doi.path));
                        },
-                        [&](DerivationOutputCAFixed dof) {
+                        [&](const DerivationOutputCAFixed & dof) {
                            outputObj.attr("path", store->printStorePath(dof.path(*store, drv.name, outputName)));
                            outputObj.attr("hashAlgo", dof.hash.printMethodAlgo());
                            outputObj.attr("hash", dof.hash.hash.to_string(Base16, false));
                        },
-                        [&](DerivationOutputCAFloating dof) {
+                        [&](const DerivationOutputCAFloating & dof) {
                            outputObj.attr("hashAlgo", makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType));
                        },
-                        [&](DerivationOutputDeferred) {},
+                        [&](const DerivationOutputDeferred &) {},
                    }, output.output);
                }
            }