Fixing #7479

Types converted:

- `NixStringContextElem`
- `OutputsSpec`
- `ExtendedOutputsSpec`
- `DerivationOutput`
- `DerivationType`

Existing ones mostly conforming the pattern cleaned up:

- `ContentAddressMethod`
- `ContentAddressWithReferences`

The `DerivationGoal::derivationType` field had a bogus initialization,
now caught, so I made it `std::optional`. I think #8829 can make it
non-optional again because it will ensure we always have the derivation
when we construct a `DerivationGoal`.

See that issue (#7479) for details on the general goal.

`git grep 'Raw::Raw'` indicates the two types I didn't yet convert
`DerivedPath` and `BuiltPath` (and their `Single` variants) . This is
because @roberth and I (can't find issue right now...) plan on reworking
them somewhat, so I didn't want to churn them more just yet.

Co-authored-by: Eelco Dolstra <edolstra@gmail.com>

src/libcmd/installable-attr-path.cc

@@ -80,7 +80,7 @@ DerivedPathsWithInfo InstallableAttrPath::toDerivedPaths()
             [&](const ExtendedOutputsSpec::Explicit & e) -> OutputsSpec {
                 return e;
             },
-        }, extendedOutputsSpec.raw());
+        }, extendedOutputsSpec.raw);
 
         auto [iter, didInsert] = byDrvPath.emplace(*drvPath, newOutputs);
 
@@ -96,6 +96,7 @@ DerivedPathsWithInfo InstallableAttrPath::toDerivedPaths()
                 .outputs = outputs,
             },
             .info = make_ref<ExtraPathInfoValue>(ExtraPathInfoValue::Value {
+                .extendedOutputsSpec = outputs,
                 /* FIXME: reconsider backwards compatibility above
                    so we can fill in this info. */
             }),
@@ -114,7 +115,7 @@ InstallableAttrPath InstallableAttrPath::parse(
     return {
         state, cmd, v,
         prefix == "." ? "" : std::string { prefix },
-        extendedOutputsSpec
+        std::move(extendedOutputsSpec),
     };
 }
 

src/libcmd/installable-derived-path.cc

@@ -55,7 +55,7 @@ InstallableDerivedPath InstallableDerivedPath::parse(
                 .outputs = outputSpec,
             };
         },
-    }, extendedOutputsSpec.raw());
+    }, extendedOutputsSpec.raw);
     return InstallableDerivedPath {
         store,
         std::move(derivedPath),

src/libcmd/installable-flake.cc

@@ -141,7 +141,7 @@ DerivedPathsWithInfo InstallableFlake::toDerivedPaths()
                 [&](const ExtendedOutputsSpec::Explicit & e) -> OutputsSpec {
                     return e;
                 },
-            }, extendedOutputsSpec.raw()),
+            }, extendedOutputsSpec.raw),
         },
         .info = make_ref<ExtraPathInfoFlake>(
             ExtraPathInfoValue::Value {

src/libcmd/installables.cc

@@ -459,7 +459,7 @@ Installables SourceExprCommand::parseInstallables(
             result.push_back(
                 make_ref<InstallableAttrPath>(
                     InstallableAttrPath::parse(
-                        state, *this, vFile, prefix, extendedOutputsSpec)));
+                        state, *this, vFile, std::move(prefix), std::move(extendedOutputsSpec))));
         }
 
     } else {
@@ -475,7 +475,7 @@ Installables SourceExprCommand::parseInstallables(
             if (prefix.find('/') != std::string::npos) {
                 try {
                     result.push_back(make_ref<InstallableDerivedPath>(
-                        InstallableDerivedPath::parse(store, prefix, extendedOutputsSpec)));
+                        InstallableDerivedPath::parse(store, prefix, extendedOutputsSpec.raw)));
                     continue;
                 } catch (BadStorePath &) {
                 } catch (...) {
@@ -491,7 +491,7 @@ Installables SourceExprCommand::parseInstallables(
                         getEvalState(),
                         std::move(flakeRef),
                         fragment,
-                        extendedOutputsSpec,
+                        std::move(extendedOutputsSpec),
                         getDefaultFlakeAttrPaths(),
                         getDefaultFlakeAttrPathPrefixes(),
                         lockFlags));

src/libexpr/eval-cache.cc

@@ -604,7 +604,7 @@ string_t AttrCursor::getStringWithContext()
                         [&](const NixStringContextElem::Opaque & o) -> const StorePath & {
                             return o.path;
                         },
-                    }, c.raw());
+                    }, c.raw);
                     if (!root->state.store->isValidPath(path)) {
                         valid = false;
                         break;

src/libexpr/eval.cc

@@ -2364,7 +2364,7 @@ std::pair<SingleDerivedPath, std::string_view> EvalState::coerceToSingleDerivedP
         [&](NixStringContextElem::Built && b) -> SingleDerivedPath {
             return std::move(b);
         },
-    }, ((NixStringContextElem &&) *context.begin()).raw());
+    }, ((NixStringContextElem &&) *context.begin()).raw);
     return {
         std::move(derivedPath),
         std::move(s),

src/libexpr/flake/flakeref.cc

@@ -246,7 +246,7 @@ std::tuple<FlakeRef, std::string, ExtendedOutputsSpec> parseFlakeRefWithFragment
 {
     auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse(url);
     auto [flakeRef, fragment] = parseFlakeRefWithFragment(std::string { prefix }, baseDir, allowMissing, isFlake);
-    return {std::move(flakeRef), fragment, extendedOutputsSpec};
+    return {std::move(flakeRef), fragment, std::move(extendedOutputsSpec)};
 }
 
 }

src/libexpr/primops.cc

@@ -69,7 +69,7 @@ StringMap EvalState::realiseContext(const NixStringContext & context)
                 res.insert_or_assign(ctxS, ctxS);
                 ensureValid(d.drvPath);
             },
-        }, c.raw());
+        }, c.raw);
     }
 
     if (drvs.empty()) return {};
@@ -1265,7 +1265,7 @@ drvName, Bindings * attrs, Value & v)
             [&](const NixStringContextElem::Opaque & o) {
                 drv.inputSrcs.insert(o.path);
             },
-        }, c.raw());
+        }, c.raw);
     }
 
     /* Do we have all required attributes? */
@@ -1334,13 +1334,13 @@ drvName, Bindings * attrs, Value & v)
             if (isImpure)
                 drv.outputs.insert_or_assign(i,
                     DerivationOutput::Impure {
-                        .method = method.raw,
+                        .method = method,
                         .hashType = ht,
                     });
             else
                 drv.outputs.insert_or_assign(i,
                     DerivationOutput::CAFloating {
-                        .method = method.raw,
+                        .method = method,
                         .hashType = ht,
                     });
         }
@@ -1373,7 +1373,7 @@ drvName, Bindings * attrs, Value & v)
                 drv.env[i] = state.store->printStorePath(outPath);
                 drv.outputs.insert_or_assign(
                     i,
-                    DerivationOutputInputAddressed {
+                    DerivationOutput::InputAddressed {
                         .path = std::move(outPath),
                     });
             }
@@ -1381,7 +1381,7 @@ drvName, Bindings * attrs, Value & v)
             ;
         case DrvHash::Kind::Deferred:
             for (auto & i : outputs) {
-                drv.outputs.insert_or_assign(i, DerivationOutputDeferred {});
+                drv.outputs.insert_or_assign(i, DerivationOutput::Deferred {});
             }
         }
     }
@@ -2054,7 +2054,7 @@ static void prim_toFile(EvalState & state, const PosIdx pos, Value * * args, Val
     StorePathSet refs;
 
     for (auto c : context) {
-        if (auto p = std::get_if<NixStringContextElem::Opaque>(&c))
+        if (auto p = std::get_if<NixStringContextElem::Opaque>(&c.raw))
             refs.insert(p->path);
         else
             state.debugThrowLastTrace(EvalError({

src/libexpr/primops/context.cc

@@ -51,13 +51,13 @@ static void prim_unsafeDiscardOutputDependency(EvalState & state, const PosIdx p
 
     NixStringContext context2;
     for (auto && c : context) {
-        if (auto * ptr = std::get_if<NixStringContextElem::DrvDeep>(&c)) {
+        if (auto * ptr = std::get_if<NixStringContextElem::DrvDeep>(&c.raw)) {
             context2.emplace(NixStringContextElem::Opaque {
                 .path = ptr->drvPath
             });
         } else {
             /* Can reuse original item */
-            context2.emplace(std::move(c));
+            context2.emplace(std::move(c).raw);
         }
     }
 
@@ -114,7 +114,7 @@ static void prim_getContext(EvalState & state, const PosIdx pos, Value * * args,
             [&](NixStringContextElem::Opaque && o) {
                 contextInfos[std::move(o.path)].path = true;
             },
-        }, ((NixStringContextElem &&) i).raw());
+        }, ((NixStringContextElem &&) i).raw);
     }
 
     auto attrs = state.buildBindings(contextInfos.size());

src/libexpr/tests/value/context.cc

@@ -47,7 +47,7 @@ TEST(NixStringContextElemTest, slash_invalid) {
 TEST(NixStringContextElemTest, opaque) {
     std::string_view opaque = "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x";
     auto elem = NixStringContextElem::parse(opaque);
-    auto * p = std::get_if<NixStringContextElem::Opaque>(&elem);
+    auto * p = std::get_if<NixStringContextElem::Opaque>(&elem.raw);
     ASSERT_TRUE(p);
     ASSERT_EQ(p->path, StorePath { opaque });
     ASSERT_EQ(elem.to_string(), opaque);
@@ -60,7 +60,7 @@ TEST(NixStringContextElemTest, opaque) {
 TEST(NixStringContextElemTest, drvDeep) {
     std::string_view drvDeep = "=g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x.drv";
     auto elem = NixStringContextElem::parse(drvDeep);
-    auto * p = std::get_if<NixStringContextElem::DrvDeep>(&elem);
+    auto * p = std::get_if<NixStringContextElem::DrvDeep>(&elem.raw);
     ASSERT_TRUE(p);
     ASSERT_EQ(p->drvPath, StorePath { drvDeep.substr(1) });
     ASSERT_EQ(elem.to_string(), drvDeep);
@@ -73,7 +73,7 @@ TEST(NixStringContextElemTest, drvDeep) {
 TEST(NixStringContextElemTest, built_opaque) {
     std::string_view built = "!foo!g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x.drv";
     auto elem = NixStringContextElem::parse(built);
-    auto * p = std::get_if<NixStringContextElem::Built>(&elem);
+    auto * p = std::get_if<NixStringContextElem::Built>(&elem.raw);
     ASSERT_TRUE(p);
     ASSERT_EQ(p->output, "foo");
     ASSERT_EQ(*p->drvPath, ((SingleDerivedPath) SingleDerivedPath::Opaque {
@@ -96,7 +96,7 @@ TEST(NixStringContextElemTest, built_built) {
 
     std::string_view built = "!foo!bar!g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x.drv";
     auto elem = NixStringContextElem::parse(built, mockXpSettings);
-    auto * p = std::get_if<NixStringContextElem::Built>(&elem);
+    auto * p = std::get_if<NixStringContextElem::Built>(&elem.raw);
     ASSERT_TRUE(p);
     ASSERT_EQ(p->output, "foo");
     auto * drvPath = std::get_if<SingleDerivedPath::Built>(&*p->drvPath);

src/libexpr/value/context.cc

@@ -99,7 +99,7 @@ std::string NixStringContextElem::to_string() const
             res += '=';
             res += d.drvPath.to_string();
         },
-    }, raw());
+    }, raw);
 
     return res;
 }

src/libexpr/value/context.hh

@@ -4,8 +4,7 @@
 #include "util.hh"
 #include "comparator.hh"
 #include "derived-path.hh"
-
+#include "variant-wrapper.hh"
-#include <variant>
 
 #include <nlohmann/json_fwd.hpp>
 
@@ -26,58 +25,47 @@ public:
     }
 };
 
-/**
+struct NixStringContextElem {
- * Plain opaque path to some store object.
+    /**
- *
+     * Plain opaque path to some store object.
- * Encoded as just the path: ‘<path>’.
+     *
- */
+     * Encoded as just the path: ‘<path>’.
-typedef SingleDerivedPath::Opaque NixStringContextElem_Opaque;
+     */
+    using Opaque = SingleDerivedPath::Opaque;
 
-/**
+    /**
- * Path to a derivation and its entire build closure.
+     * Path to a derivation and its entire build closure.
- *
+     *
- * The path doesn't just refer to derivation itself and its closure, but
+     * The path doesn't just refer to derivation itself and its closure, but
- * also all outputs of all derivations in that closure (including the
+     * also all outputs of all derivations in that closure (including the
- * root derivation).
+     * root derivation).
- *
+     *
- * Encoded in the form ‘=<drvPath>’.
+     * Encoded in the form ‘=<drvPath>’.
- */
+     */
-struct NixStringContextElem_DrvDeep {
+    struct DrvDeep {
-    StorePath drvPath;
+        StorePath drvPath;
 
-    GENERATE_CMP(NixStringContextElem_DrvDeep, me->drvPath);
+        GENERATE_CMP(DrvDeep, me->drvPath);
-};
+    };
 
-/**
+    /**
- * Derivation output.
+     * Derivation output.
- *
+     *
- * Encoded in the form ‘!<output>!<drvPath>’.
+     * Encoded in the form ‘!<output>!<drvPath>’.
- */
+     */
-typedef SingleDerivedPath::Built NixStringContextElem_Built;
+    using Built = SingleDerivedPath::Built;
 
-using _NixStringContextElem_Raw = std::variant<
+    using Raw = std::variant<
-    NixStringContextElem_Opaque,
+        Opaque,
-    NixStringContextElem_DrvDeep,
+        DrvDeep,
-    NixStringContextElem_Built
+        Built
->;
+    >;
 
-struct NixStringContextElem : _NixStringContextElem_Raw {
+    Raw raw;
-    using Raw = _NixStringContextElem_Raw;
-    using Raw::Raw;
 
-    using Opaque = NixStringContextElem_Opaque;
+    GENERATE_CMP(NixStringContextElem, me->raw);
-    using DrvDeep = NixStringContextElem_DrvDeep;
-    using Built = NixStringContextElem_Built;
 
-    inline const Raw & raw() const & {
+    MAKE_WRAPPER_CONSTRUCTOR(NixStringContextElem);
-        return static_cast<const Raw &>(*this);
-    }
-    inline Raw & raw() & {
-        return static_cast<Raw &>(*this);
-    }
-    inline Raw && raw() && {
-        return static_cast<Raw &&>(*this);
-    }
 
     /**
      * Decode a context string, one of:

src/libstore/build/derivation-goal.cc

@@ -521,7 +521,7 @@ void DerivationGoal::inputsRealised()
             [&](const DerivationType::Impure &) {
                 return true;
             }
-        }, drvType.raw());
+        }, drvType.raw);
 
         if (resolveDrv && !fullDrv.inputDrvs.empty()) {
             experimentalFeatureSettings.require(Xp::CaDerivations);
@@ -996,10 +996,11 @@ void DerivationGoal::buildDone()
         }
 
         else {
+            assert(derivationType);
             st =
                 dynamic_cast<NotDeterministic*>(&e) ? BuildResult::NotDeterministic :
                 statusOk(status) ? BuildResult::OutputRejected :
-                !derivationType.isSandboxed() || diskFull ? BuildResult::TransientFailure :
+                !derivationType->isSandboxed() || diskFull ? BuildResult::TransientFailure :
                 BuildResult::PermanentFailure;
         }
 
@@ -1358,7 +1359,7 @@ std::pair<bool, SingleDrvOutputs> DerivationGoal::checkPathValidity()
         [&](const OutputsSpec::Names & names) {
             return static_cast<StringSet>(names);
         },
-    }, wantedOutputs.raw());
+    }, wantedOutputs.raw);
     SingleDrvOutputs validOutputs;
 
     for (auto & i : queryPartialDerivationOutputMap()) {

src/libstore/build/derivation-goal.hh

@@ -184,7 +184,7 @@ struct DerivationGoal : public Goal
     /**
      * The sort of derivation we are building.
      */
-    DerivationType derivationType;
+    std::optional<DerivationType> derivationType;
 
     typedef void (DerivationGoal::*GoalState)();
     GoalState state;

src/libstore/build/local-derivation-goal.cc

@@ -178,6 +178,8 @@ void LocalDerivationGoal::tryLocalBuild()
         return;
     }
 
+    assert(derivationType);
+
     /* Are we doing a chroot build? */
     {
         auto noChroot = parsedDrv->getBoolAttr("__noChroot");
@@ -195,7 +197,7 @@ void LocalDerivationGoal::tryLocalBuild()
         else if (settings.sandboxMode == smDisabled)
             useChroot = false;
         else if (settings.sandboxMode == smRelaxed)
-            useChroot = derivationType.isSandboxed() && !noChroot;
+            useChroot = derivationType->isSandboxed() && !noChroot;
     }
 
     auto & localStore = getLocalStore();
@@ -689,7 +691,7 @@ void LocalDerivationGoal::startBuilder()
                 "nogroup:x:65534:\n", sandboxGid()));
 
         /* Create /etc/hosts with localhost entry. */
-        if (derivationType.isSandboxed())
+        if (derivationType->isSandboxed())
             writeFile(chrootRootDir + "/etc/hosts", "127.0.0.1 localhost\n::1 localhost\n");
 
         /* Make the closure of the inputs available in the chroot,
@@ -893,7 +895,7 @@ void LocalDerivationGoal::startBuilder()
            us.
         */
 
-        if (derivationType.isSandboxed())
+        if (derivationType->isSandboxed())
             privateNetwork = true;
 
         userNamespaceSync.create();
@@ -1121,7 +1123,7 @@ void LocalDerivationGoal::initEnv()
        derivation, tell the builder, so that for instance `fetchurl'
        can skip checking the output.  On older Nixes, this environment
        variable won't be set, so `fetchurl' will do the check. */
-    if (derivationType.isFixed()) env["NIX_OUTPUT_CHECKED"] = "1";
+    if (derivationType->isFixed()) env["NIX_OUTPUT_CHECKED"] = "1";
 
     /* *Only* if this is a fixed-output derivation, propagate the
        values of the environment variables specified in the
@@ -1132,7 +1134,7 @@ void LocalDerivationGoal::initEnv()
        to the builder is generally impure, but the output of
        fixed-output derivations is by definition pure (since we
        already know the cryptographic hash of the output). */
-    if (!derivationType.isSandboxed()) {
+    if (!derivationType->isSandboxed()) {
         for (auto & i : parsedDrv->getStringsAttr("impureEnvVars").value_or(Strings()))
             env[i] = getEnv(i).value_or("");
     }
@@ -1797,7 +1799,7 @@ void LocalDerivationGoal::runChild()
             /* Fixed-output derivations typically need to access the
                network, so give them access to /etc/resolv.conf and so
                on. */
-            if (!derivationType.isSandboxed()) {
+            if (!derivationType->isSandboxed()) {
                 // Only use nss functions to resolve hosts and
                 // services. Don’t use it for anything else that may
                 // be configured for this system. This limits the
@@ -2048,7 +2050,7 @@ void LocalDerivationGoal::runChild()
                     #include "sandbox-defaults.sb"
                     ;
 
-                if (!derivationType.isSandboxed())
+                if (!derivationType->isSandboxed())
                     sandboxProfile +=
                         #include "sandbox-network.sb"
                         ;
@@ -2599,7 +2601,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs()
                 });
             },
 
-        }, output->raw());
+        }, output->raw);
 
         /* FIXME: set proper permissions in restorePath() so
             we don't have to do another traversal. */

src/libstore/build/worker.cc

@@ -268,7 +268,10 @@ void Worker::run(const Goals & _topGoals)
     for (auto & i : _topGoals) {
         topGoals.insert(i);
         if (auto goal = dynamic_cast<DerivationGoal *>(i.get())) {
-            topPaths.push_back(DerivedPath::Built{makeConstantStorePathRef(goal->drvPath), goal->wantedOutputs});
+            topPaths.push_back(DerivedPath::Built {
+                .drvPath = makeConstantStorePathRef(goal->drvPath),
+                .outputs = goal->wantedOutputs,
+            });
         } else if (auto goal = dynamic_cast<PathSubstitutionGoal *>(i.get())) {
             topPaths.push_back(DerivedPath::Opaque{goal->storePath});
         }

src/libstore/content-address.cc

@@ -115,7 +115,7 @@ ContentAddress ContentAddress::parse(std::string_view rawCa)
     auto [caMethod, hashType] = parseContentAddressMethodPrefix(rest);
 
     return ContentAddress {
-        .method = std::move(caMethod).raw,
+        .method = std::move(caMethod),
         .hash = Hash::parseNonSRIUnprefixed(rest, hashType),
     };
 }

src/libstore/content-address.hh

@@ -5,6 +5,7 @@
 #include "hash.hh"
 #include "path.hh"
 #include "comparator.hh"
+#include "variant-wrapper.hh"
 
 namespace nix {
 
@@ -71,11 +72,7 @@ struct ContentAddressMethod
 
     GENERATE_CMP(ContentAddressMethod, me->raw);
 
-    /* The moral equivalent of `using Raw::Raw;` */
+    MAKE_WRAPPER_CONSTRUCTOR(ContentAddressMethod);
-    ContentAddressMethod(auto &&... arg)
-        : raw(std::forward<decltype(arg)>(arg)...)
-    { }
-
 
     /**
      * Parse the prefix tag which indicates how the files
@@ -252,10 +249,7 @@ struct ContentAddressWithReferences
 
     GENERATE_CMP(ContentAddressWithReferences, me->raw);
 
-    /* The moral equivalent of `using Raw::Raw;` */
+    MAKE_WRAPPER_CONSTRUCTOR(ContentAddressWithReferences);
-    ContentAddressWithReferences(auto &&... arg)
-        : raw(std::forward<decltype(arg)>(arg)...)
-    { }
 
     /**
      * Create a `ContentAddressWithReferences` from a mere

src/libstore/derivations.cc

@@ -32,7 +32,7 @@ std::optional<StorePath> DerivationOutput::path(const Store & store, std::string
         [](const DerivationOutput::Impure &) -> std::optional<StorePath> {
             return std::nullopt;
         },
-    }, raw());
+    }, raw);
 }
 
 
@@ -60,7 +60,7 @@ bool DerivationType::isCA() const
         [](const Impure &) {
             return true;
         },
-    }, raw());
+    }, raw);
 }
 
 bool DerivationType::isFixed() const
@@ -75,7 +75,7 @@ bool DerivationType::isFixed() const
         [](const Impure &) {
             return false;
         },
-    }, raw());
+    }, raw);
 }
 
 bool DerivationType::hasKnownOutputPaths() const
@@ -90,7 +90,7 @@ bool DerivationType::hasKnownOutputPaths() const
         [](const Impure &) {
             return false;
         },
-    }, raw());
+    }, raw);
 }
 
 
@@ -106,7 +106,7 @@ bool DerivationType::isSandboxed() const
         [](const Impure &) {
             return false;
         },
-    }, raw());
+    }, raw);
 }
 
 
@@ -122,7 +122,7 @@ bool DerivationType::isPure() const
         [](const Impure &) {
             return false;
         },
-    }, raw());
+    }, raw);
 }
 
 
@@ -408,13 +408,13 @@ std::string Derivation::unparse(const Store & store, bool maskOutputs,
                 s += ','; printUnquotedString(s, "");
                 s += ','; printUnquotedString(s, "");
             },
-            [&](const DerivationOutputImpure & doi) {
+            [&](const DerivationOutput::Impure & doi) {
                 // FIXME
                 s += ','; printUnquotedString(s, "");
                 s += ','; printUnquotedString(s, doi.method.renderPrefix() + printHashType(doi.hashType));
                 s += ','; printUnquotedString(s, "impure");
             }
-        }, i.second.raw());
+        }, i.second.raw);
         s += ')';
     }
 
@@ -509,7 +509,7 @@ DerivationType BasicDerivation::type() const
             [&](const DerivationOutput::Impure &) {
                 impureOutputs.insert(i.first);
             },
-        }, i.second.raw());
+        }, i.second.raw);
     }
 
     if (inputAddressedOutputs.empty()
@@ -626,7 +626,7 @@ DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOut
     if (type.isFixed()) {
         std::map<std::string, Hash> outputHashes;
         for (const auto & i : drv.outputs) {
-            auto & dof = std::get<DerivationOutput::CAFixed>(i.second.raw());
+            auto & dof = std::get<DerivationOutput::CAFixed>(i.second.raw);
             auto hash = hashString(htSHA256, "fixed:out:"
                 + dof.ca.printMethodAlgo() + ":"
                 + dof.ca.hash.to_string(Base16, false) + ":"
@@ -663,7 +663,7 @@ DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOut
         [](const DerivationType::Impure &) -> DrvHash::Kind {
             assert(false);
         }
-    }, drv.type().raw());
+    }, drv.type().raw);
 
     std::map<std::string, StringSet> inputs2;
     for (auto & [drvPath, inputOutputs0] : drv.inputDrvs) {
@@ -720,10 +720,10 @@ StringSet BasicDerivation::outputNames() const
 DerivationOutputsAndOptPaths BasicDerivation::outputsAndOptPaths(const Store & store) const
 {
     DerivationOutputsAndOptPaths outsAndOptPaths;
-    for (auto output : outputs)
+    for (auto & [outputName, output] : outputs)
         outsAndOptPaths.insert(std::make_pair(
-            output.first,
+            outputName,
-            std::make_pair(output.second, output.second.path(store, name, output.first))
+            std::make_pair(output, output.path(store, name, outputName))
             )
         );
     return outsAndOptPaths;
@@ -798,7 +798,7 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr
                     << (doi.method.renderPrefix() + printHashType(doi.hashType))
                     << "impure";
             },
-        }, i.second.raw());
+        }, i.second.raw);
     }
     WorkerProto::write(store,
         WorkerProto::WriteConn { .to = out },
@@ -840,7 +840,7 @@ static void rewriteDerivation(Store & store, BasicDerivation & drv, const String
 
     auto hashModulo = hashDerivationModulo(store, Derivation(drv), true);
     for (auto & [outputName, output] : drv.outputs) {
-        if (std::holds_alternative<DerivationOutput::Deferred>(output.raw())) {
+        if (std::holds_alternative<DerivationOutput::Deferred>(output.raw)) {
             auto h = get(hashModulo.hashes, outputName);
             if (!h)
                 throw Error("derivation '%s' output '%s' has no hash (derivations.cc/rewriteDerivation)",
@@ -955,7 +955,7 @@ void Derivation::checkInvariants(Store & store, const StorePath & drvPath) const
             [&](const DerivationOutput::Impure &) {
                 /* Nothing to check */
             },
-        }, i.second.raw());
+        }, i.second.raw);
     }
 }
 
@@ -984,7 +984,7 @@ nlohmann::json DerivationOutput::toJSON(
             res["hashAlgo"] = doi.method.renderPrefix() + printHashType(doi.hashType);
             res["impure"] = true;
         },
-    }, raw());
+    }, raw);
     return res;
 }
 

src/libstore/derivations.hh

@@ -9,6 +9,7 @@
 #include "derived-path.hh"
 #include "sync.hh"
 #include "comparator.hh"
+#include "variant-wrapper.hh"
 
 #include <map>
 #include <variant>
@@ -20,108 +21,110 @@ class Store;
 
 /* Abstract syntax of derivations. */
 
-/**
- * The traditional non-fixed-output derivation type.
- */
-struct DerivationOutputInputAddressed
-{
-    StorePath path;
-
-    GENERATE_CMP(DerivationOutputInputAddressed, me->path);
-};
-
-/**
- * Fixed-output derivations, whose output paths are content
- * addressed according to that fixed output.
- */
-struct DerivationOutputCAFixed
-{
-    /**
-     * Method and hash used for expected hash computation.
-     *
-     * References are not allowed by fiat.
-     */
-    ContentAddress ca;
-
-    /**
-     * Return the \ref StorePath "store path" corresponding to this output
-     *
-     * @param drvName The name of the derivation this is an output of, without the `.drv`.
-     * @param outputName The name of this output.
-     */
-    StorePath path(const Store & store, std::string_view drvName, std::string_view outputName) const;
-
-    GENERATE_CMP(DerivationOutputCAFixed, me->ca);
-};
-
-/**
- * Floating-output derivations, whose output paths are content
- * addressed, but not fixed, and so are dynamically calculated from
- * whatever the output ends up being.
- * */
-struct DerivationOutputCAFloating
-{
-    /**
-     * How the file system objects will be serialized for hashing
-     */
-    ContentAddressMethod method;
-
-    /**
-     * How the serialization will be hashed
-     */
-    HashType hashType;
-
-    GENERATE_CMP(DerivationOutputCAFloating, me->method, me->hashType);
-};
-
-/**
- * Input-addressed output which depends on a (CA) derivation whose hash
- * isn't known yet.
- */
-struct DerivationOutputDeferred {
-    GENERATE_CMP(DerivationOutputDeferred);
-};
-
-/**
- * Impure output which is moved to a content-addressed location (like
- * CAFloating) but isn't registered as a realization.
- */
-struct DerivationOutputImpure
-{
-    /**
-     * How the file system objects will be serialized for hashing
-     */
-    ContentAddressMethod method;
-
-    /**
-     * How the serialization will be hashed
-     */
-    HashType hashType;
-
-    GENERATE_CMP(DerivationOutputImpure, me->method, me->hashType);
-};
-
-typedef std::variant<
-    DerivationOutputInputAddressed,
-    DerivationOutputCAFixed,
-    DerivationOutputCAFloating,
-    DerivationOutputDeferred,
-    DerivationOutputImpure
-> _DerivationOutputRaw;
-
 /**
  * A single output of a BasicDerivation (and Derivation).
  */
-struct DerivationOutput : _DerivationOutputRaw
+struct DerivationOutput
 {
-    using Raw = _DerivationOutputRaw;
+    /**
-    using Raw::Raw;
+     * The traditional non-fixed-output derivation type.
+     */
+    struct InputAddressed
+    {
+        StorePath path;
 
-    using InputAddressed = DerivationOutputInputAddressed;
+        GENERATE_CMP(InputAddressed, me->path);
-    using CAFixed = DerivationOutputCAFixed;
+    };
-    using CAFloating = DerivationOutputCAFloating;
+
-    using Deferred = DerivationOutputDeferred;
+    /**
-    using Impure = DerivationOutputImpure;
+     * Fixed-output derivations, whose output paths are content
+     * addressed according to that fixed output.
+     */
+    struct CAFixed
+    {
+        /**
+         * Method and hash used for expected hash computation.
+         *
+         * References are not allowed by fiat.
+         */
+        ContentAddress ca;
+
+        /**
+         * Return the \ref StorePath "store path" corresponding to this output
+         *
+         * @param drvName The name of the derivation this is an output of, without the `.drv`.
+         * @param outputName The name of this output.
+         */
+        StorePath path(const Store & store, std::string_view drvName, std::string_view outputName) const;
+
+        GENERATE_CMP(CAFixed, me->ca);
+    };
+
+    /**
+     * Floating-output derivations, whose output paths are content
+     * addressed, but not fixed, and so are dynamically calculated from
+     * whatever the output ends up being.
+     * */
+    struct CAFloating
+    {
+        /**
+         * How the file system objects will be serialized for hashing
+         */
+        ContentAddressMethod method;
+
+        /**
+         * How the serialization will be hashed
+         */
+        HashType hashType;
+
+        GENERATE_CMP(CAFloating, me->method, me->hashType);
+    };
+
+    /**
+     * Input-addressed output which depends on a (CA) derivation whose hash
+     * isn't known yet.
+     */
+    struct Deferred {
+        GENERATE_CMP(Deferred);
+    };
+
+    /**
+     * Impure output which is moved to a content-addressed location (like
+     * CAFloating) but isn't registered as a realization.
+     */
+    struct Impure
+    {
+        /**
+         * How the file system objects will be serialized for hashing
+         */
+        ContentAddressMethod method;
+
+        /**
+         * How the serialization will be hashed
+         */
+        HashType hashType;
+
+        GENERATE_CMP(Impure, me->method, me->hashType);
+    };
+
+    typedef std::variant<
+        InputAddressed,
+        CAFixed,
+        CAFloating,
+        Deferred,
+        Impure
+    > Raw;
+
+    Raw raw;
+
+    GENERATE_CMP(DerivationOutput, me->raw);
+
+    MAKE_WRAPPER_CONSTRUCTOR(DerivationOutput);
+
+    /**
+     * Force choosing a variant
+     */
+    DerivationOutput() = delete;
 
     /**
      * \note when you use this function you should make sure that you're
@@ -131,10 +134,6 @@ struct DerivationOutput : _DerivationOutputRaw
      */
     std::optional<StorePath> path(const Store & store, std::string_view drvName, std::string_view outputName) const;
 
-    inline const Raw & raw() const {
-        return static_cast<const Raw &>(*this);
-    }
-
     nlohmann::json toJSON(
         const Store & store,
         std::string_view drvName,
@@ -167,61 +166,71 @@ typedef std::map<std::string, std::pair<DerivationOutput, std::optional<StorePat
  */
 typedef std::map<StorePath, StringSet> DerivationInputs;
 
-/**
+struct DerivationType {
- * Input-addressed derivation types
- */
-struct DerivationType_InputAddressed {
     /**
-     * True iff the derivation type can't be determined statically,
+     * Input-addressed derivation types
-     * for instance because it (transitively) depends on a content-addressed
-     * derivation.
-    */
-    bool deferred;
-};
-
-/**
- * Content-addressed derivation types
- */
-struct DerivationType_ContentAddressed {
-    /**
-     * Whether the derivation should be built safely inside a sandbox.
      */
-    bool sandboxed;
+    struct InputAddressed {
+        /**
+         * True iff the derivation type can't be determined statically,
+         * for instance because it (transitively) depends on a content-addressed
+         * derivation.
+        */
+        bool deferred;
+
+        GENERATE_CMP(InputAddressed, me->deferred);
+    };
+
     /**
-     * Whether the derivation's outputs' content-addresses are "fixed"
+     * Content-addressed derivation types
-     * or "floating.
-     *
-     *  - Fixed: content-addresses are written down as part of the
-     *    derivation itself. If the outputs don't end up matching the
-     *    build fails.
-     *
-     *  - Floating: content-addresses are not written down, we do not
-     *    know them until we perform the build.
      */
-    bool fixed;
+    struct ContentAddressed {
-};
+        /**
+         * Whether the derivation should be built safely inside a sandbox.
+         */
+        bool sandboxed;
+        /**
+         * Whether the derivation's outputs' content-addresses are "fixed"
+         * or "floating".
+         *
+         *  - Fixed: content-addresses are written down as part of the
+         *    derivation itself. If the outputs don't end up matching the
+         *    build fails.
+         *
+         *  - Floating: content-addresses are not written down, we do not
+         *    know them until we perform the build.
+         */
+        bool fixed;
 
-/**
+        GENERATE_CMP(ContentAddressed, me->sandboxed, me->fixed);
- * Impure derivation type
+    };
- *
- * This is similar at buil-time to the content addressed, not standboxed, not fixed
- * type, but has some restrictions on its usage.
- */
-struct DerivationType_Impure {
-};
 
-typedef std::variant<
+    /**
-    DerivationType_InputAddressed,
+     * Impure derivation type
-    DerivationType_ContentAddressed,
+     *
-    DerivationType_Impure
+     * This is similar at buil-time to the content addressed, not standboxed, not fixed
-> _DerivationTypeRaw;
+     * type, but has some restrictions on its usage.
+     */
+    struct Impure {
+        GENERATE_CMP(Impure);
+    };
 
-struct DerivationType : _DerivationTypeRaw {
+    typedef std::variant<
-    using Raw = _DerivationTypeRaw;
+        InputAddressed,
-    using Raw::Raw;
+        ContentAddressed,
-    using InputAddressed = DerivationType_InputAddressed;
+        Impure
-    using ContentAddressed = DerivationType_ContentAddressed;
+    > Raw;
-    using Impure = DerivationType_Impure;
+
+    Raw raw;
+
+    GENERATE_CMP(DerivationType, me->raw);
+
+    MAKE_WRAPPER_CONSTRUCTOR(DerivationType);
+
+    /**
+     * Force choosing a variant
+     */
+    DerivationType() = delete;
 
     /**
      * Do the outputs of the derivation have paths calculated from their
@@ -257,10 +266,6 @@ struct DerivationType : _DerivationTypeRaw {
      * closure, or if fixed output.
      */
     bool hasKnownOutputPaths() const;
-
-    inline const Raw & raw() const {
-        return static_cast<const Raw &>(*this);
-    }
 };
 
 struct BasicDerivation

src/libstore/misc.cc

@@ -88,7 +88,7 @@ const ContentAddress * getDerivationCA(const BasicDerivation & drv)
     auto out = drv.outputs.find("out");
     if (out == drv.outputs.end())
         return nullptr;
-    if (auto dof = std::get_if<DerivationOutput::CAFixed>(&out->second)) {
+    if (auto dof = std::get_if<DerivationOutput::CAFixed>(&out->second.raw)) {
         return &dof->ca;
     }
     return nullptr;
@@ -370,7 +370,7 @@ OutputPathMap resolveDerivedPath(Store & store, const DerivedPath::Built & bfd,
             }
             return outputsOpt;
         },
-    }, bfd.outputs.raw());
+    }, bfd.outputs.raw);
 
     OutputPathMap outputs;
     for (auto & [outputName, outputPathOpt] : outputsOpt) {
@@ -418,7 +418,7 @@ OutputPathMap resolveDerivedPath(Store & store, const DerivedPath::Built & bfd)
         [&](const OutputsSpec::Names & names) {
             return static_cast<StringSet>(names);
         },
-    }, bfd.outputs.raw());
+    }, bfd.outputs.raw);
     for (auto iter = outputMap.begin(); iter != outputMap.end();) {
         auto & outputName = iter->first;
         if (bfd.outputs.contains(outputName)) {
@@ -431,7 +431,7 @@ OutputPathMap resolveDerivedPath(Store & store, const DerivedPath::Built & bfd)
     if (!outputsLeft.empty())
         throw Error("derivation '%s' does not have an outputs %s",
             store.printStorePath(drvPath),
-            concatStringsSep(", ", quoteStrings(std::get<OutputsSpec::Names>(bfd.outputs))));
+            concatStringsSep(", ", quoteStrings(std::get<OutputsSpec::Names>(bfd.outputs.raw))));
     return outputMap;
 }
 

src/libstore/outputs-spec.cc

@@ -17,7 +17,7 @@ bool OutputsSpec::contains(const std::string & outputName) const
         [&](const OutputsSpec::Names & outputNames) {
             return outputNames.count(outputName) > 0;
         },
-    }, raw());
+    }, raw);
 }
 
 static std::string outputSpecRegexStr =
@@ -49,7 +49,7 @@ OutputsSpec OutputsSpec::parse(std::string_view s)
     std::optional spec = parseOpt(s);
     if (!spec)
         throw Error("invalid outputs specifier '%s'", s);
-    return *spec;
+    return std::move(*spec);
 }
 
 
@@ -85,7 +85,7 @@ std::string OutputsSpec::to_string() const
         [&](const OutputsSpec::Names & outputNames) -> std::string {
             return concatStringsSep(",", outputNames);
         },
-    }, raw());
+    }, raw);
 }
 
 
@@ -98,7 +98,7 @@ std::string ExtendedOutputsSpec::to_string() const
         [&](const ExtendedOutputsSpec::Explicit & outputSpec) -> std::string {
             return "^" + outputSpec.to_string();
         },
-    }, raw());
+    }, raw);
 }
 
 
@@ -118,9 +118,9 @@ OutputsSpec OutputsSpec::union_(const OutputsSpec & that) const
                     ret.insert(thoseNames.begin(), thoseNames.end());
                     return ret;
                 },
-            }, that.raw());
+            }, that.raw);
         },
-    }, raw());
+    }, raw);
 }
 
 
@@ -142,9 +142,9 @@ bool OutputsSpec::isSubsetOf(const OutputsSpec & that) const
                             ret = false;
                     return ret;
                 },
-            }, raw());
+            }, raw);
         },
-    }, that.raw());
+    }, that.raw);
 }
 
 }
@@ -169,7 +169,7 @@ void adl_serializer<OutputsSpec>::to_json(json & json, OutputsSpec t) {
         [&](const OutputsSpec::Names & names) {
             json = names;
         },
-    }, t.raw());
+    }, t.raw);
 }
 
 
@@ -189,7 +189,7 @@ void adl_serializer<ExtendedOutputsSpec>::to_json(json & json, ExtendedOutputsSp
         [&](const ExtendedOutputsSpec::Explicit & e) {
             adl_serializer<OutputsSpec>::to_json(json, e);
         },
-    }, t.raw());
+    }, t.raw);
 }
 
 }

src/libstore/outputs-spec.hh

@@ -6,62 +6,57 @@
 #include <set>
 #include <variant>
 
+#include "comparator.hh"
 #include "json-impls.hh"
+#include "comparator.hh"
+#include "variant-wrapper.hh"
 
 namespace nix {
 
-/**
+struct OutputsSpec {
- * A non-empty set of outputs, specified by name
+    /**
- */
+     * A non-empty set of outputs, specified by name
-struct OutputNames : std::set<std::string> {
+     */
-    using std::set<std::string>::set;
+    struct Names : std::set<std::string> {
+        using std::set<std::string>::set;
 
-    /* These need to be "inherited manually" */
+        /* These need to be "inherited manually" */
 
-    OutputNames(const std::set<std::string> & s)
+        Names(const std::set<std::string> & s)
-        : std::set<std::string>(s)
+            : std::set<std::string>(s)
-    { assert(!empty()); }
+        { assert(!empty()); }
+
+        /**
+         * Needs to be "inherited manually"
+         */
+        Names(std::set<std::string> && s)
+            : std::set<std::string>(s)
+        { assert(!empty()); }
+
+        /* This set should always be non-empty, so we delete this
+           constructor in order make creating empty ones by mistake harder.
+           */
+        Names() = delete;
+    };
 
     /**
-     * Needs to be "inherited manually"
+     * The set of all outputs, without needing to name them explicitly
      */
-    OutputNames(std::set<std::string> && s)
+    struct All : std::monostate { };
-        : std::set<std::string>(s)
-    { assert(!empty()); }
 
-    /* This set should always be non-empty, so we delete this
+    typedef std::variant<All, Names> Raw;
-       constructor in order make creating empty ones by mistake harder.
-       */
-    OutputNames() = delete;
-};
 
-/**
+    Raw raw;
- * The set of all outputs, without needing to name them explicitly
- */
-struct AllOutputs : std::monostate { };
 
-typedef std::variant<AllOutputs, OutputNames> _OutputsSpecRaw;
+    GENERATE_CMP(OutputsSpec, me->raw);
 
-struct OutputsSpec : _OutputsSpecRaw {
+    MAKE_WRAPPER_CONSTRUCTOR(OutputsSpec);
-    using Raw = _OutputsSpecRaw;
-    using Raw::Raw;
 
     /**
      * Force choosing a variant
      */
     OutputsSpec() = delete;
 
-    using Names = OutputNames;
-    using All = AllOutputs;
-
-    inline const Raw & raw() const {
-        return static_cast<const Raw &>(*this);
-    }
-
-    inline Raw & raw() {
-        return static_cast<Raw &>(*this);
-    }
-
     bool contains(const std::string & output) const;
 
     /**
@@ -84,20 +79,22 @@ struct OutputsSpec : _OutputsSpecRaw {
     std::string to_string() const;
 };
 
-struct DefaultOutputs : std::monostate { };
+struct ExtendedOutputsSpec {
-
+    struct Default : std::monostate { };
-typedef std::variant<DefaultOutputs, OutputsSpec> _ExtendedOutputsSpecRaw;
-
-struct ExtendedOutputsSpec : _ExtendedOutputsSpecRaw {
-    using Raw = _ExtendedOutputsSpecRaw;
-    using Raw::Raw;
-
-    using Default = DefaultOutputs;
     using Explicit = OutputsSpec;
 
-    inline const Raw & raw() const {
+    typedef std::variant<Default, Explicit> Raw;
-        return static_cast<const Raw &>(*this);
+
-    }
+    Raw raw;
+
+    GENERATE_CMP(ExtendedOutputsSpec, me->raw);
+
+    MAKE_WRAPPER_CONSTRUCTOR(ExtendedOutputsSpec);
+
+    /**
+     * Force choosing a variant
+     */
+    ExtendedOutputsSpec() = delete;
 
     /**
      * Parse a string of the form 'prefix^output1,...outputN' or

src/libstore/path-with-outputs.cc

@@ -63,7 +63,7 @@ StorePathWithOutputs::ParseResult StorePathWithOutputs::tryFromDerivedPath(const
                             [&](const OutputsSpec::Names & outputs) {
                                 return static_cast<StringSet>(outputs);
                             },
-                        }, bfd.outputs.raw()),
+                        }, bfd.outputs.raw),
                     };
                 },
                 [&](const SingleDerivedPath::Built &) -> StorePathWithOutputs::ParseResult {

src/libutil/variant-wrapper.hh

@@ -0,0 +1,30 @@
+#pragma once
+///@file
+
+// not used, but will be used by callers
+#include <variant>
+
+/**
+ * Force the default versions of all constructors (copy, move, copy
+ * assignment).
+ */
+#define FORCE_DEFAULT_CONSTRUCTORS(CLASS_NAME) \
+    CLASS_NAME(const CLASS_NAME &) = default; \
+    CLASS_NAME(CLASS_NAME &) = default; \
+    CLASS_NAME(CLASS_NAME &&) = default; \
+    \
+    CLASS_NAME & operator =(const CLASS_NAME &) = default; \
+    CLASS_NAME & operator =(CLASS_NAME &) = default;
+
+/**
+ * Make a wrapper constructor. All args are forwarded to the
+ * construction of the "raw" field. (Which we assume is the only one.)
+ *
+ * The moral equivalent of `using Raw::Raw;`
+ */
+#define MAKE_WRAPPER_CONSTRUCTOR(CLASS_NAME) \
+    FORCE_DEFAULT_CONSTRUCTORS(CLASS_NAME) \
+    \
+    CLASS_NAME(auto &&... arg) \
+        : raw(std::forward<decltype(arg)>(arg)...) \
+    { }

src/nix/app.cc

@@ -81,7 +81,7 @@ UnresolvedApp InstallableValue::toApp(EvalState & state)
                         .path = o.path,
                     };
                 },
-            }, c.raw()));
+            }, c.raw));
         }
 
         return UnresolvedApp{App {

src/nix/bundle.cc

@@ -80,7 +80,7 @@ struct CmdBundle : InstallableValueCommand
         auto [bundlerFlakeRef, bundlerName, extendedOutputsSpec] = parseFlakeRefWithFragmentAndExtendedOutputsSpec(bundler, absPath("."));
         const flake::LockFlags lockFlags{ .writeLockFile = false };
         InstallableFlake bundler{this,
-            evalState, std::move(bundlerFlakeRef), bundlerName, extendedOutputsSpec,
+            evalState, std::move(bundlerFlakeRef), bundlerName, std::move(extendedOutputsSpec),
             {"bundlers." + settings.thisSystem.get() + ".default",
              "defaultBundler." + settings.thisSystem.get()
             },

src/nix/develop.cc

@@ -547,7 +547,7 @@ struct CmdDevelop : Common, MixEnvironment
                 state,
                 std::move(nixpkgs),
                 "bashInteractive",
-                DefaultOutputs(),
+                ExtendedOutputsSpec::Default(),
                 Strings{},
                 Strings{"legacyPackages." + settings.thisSystem.get() + "."},
                 nixpkgsLockFlags);

src/nix/flake.cc

@@ -778,7 +778,7 @@ struct CmdFlakeInitCommon : virtual Args, EvalCommand
         auto [templateFlakeRef, templateName] = parseFlakeRefWithFragment(templateUrl, absPath("."));
 
         auto installable = InstallableFlake(nullptr,
-            evalState, std::move(templateFlakeRef), templateName, DefaultOutputs(),
+            evalState, std::move(templateFlakeRef), templateName, ExtendedOutputsSpec::Default(),
             defaultTemplateAttrPaths,
             defaultTemplateAttrPathsPrefixes,
             lockFlags);