Do big rename to clean up code

- `PathReferences` -> `References`

- `PathReferences<StorePath>` -> `StoreReference`

- `references` -> `others`

- `hasSelfReference` -> `self`

And get rid of silly subclassing

src/libexpr/primops.cc

@@ -1535,7 +1535,8 @@ static void prim_readFile(EvalState & state, const PosIdx pos, Value * * args, V
     StorePathSet refs;
     if (state.store->isInStore(path)) {
         try {
-            refs = state.store->queryPathInfo(state.store->toStorePath(path).first)->references;
+            // FIXME: Are self references becoming non-self references OK?
+            refs = state.store->queryPathInfo(state.store->toStorePath(path).first)->referencesPossiblyToSelf();
         } catch (Error &) { // FIXME: should be InvalidPathError
         }
         // Re-scan references to filter down to just the ones that actually occur in the file.
@@ -1971,7 +1972,7 @@ static void addPath(
             try {
                 auto [storePath, subPath] = state.store->toStorePath(path);
                 // FIXME: we should scanForReferences on the path before adding it
-                refs = state.store->queryPathInfo(storePath)->references;
+                refs = state.store->queryPathInfo(storePath)->referencesPossiblyToSelf();
                 path = state.store->toRealPath(storePath) + subPath;
             } catch (Error &) { // FIXME: should be InvalidPathError
             }
@@ -2010,7 +2011,7 @@ static void addPath(
                     .method = method,
                     .hash = *expectedHash,
                 },
-                {},
+                .references = {},
             });
 
         if (!expectedHash || !state.store->isValidPath(*expectedStorePath)) {

src/libstore/binary-cache-store.cc

@@ -180,8 +180,9 @@ ref<const ValidPathInfo> BinaryCacheStore::addToStoreCommon(
         duration);
 
     /* Verify that all references are valid. This may do some .narinfo
-       reads, but typically they'll already be cached. */
+       reads, but typically they'll already be cached. Note that
-    for (auto & ref : info.references)
+       self-references are always valid. */
+    for (auto & ref : info.references.others)
         try {
             queryPathInfo(ref);
         } catch (InvalidPath &) {
@@ -314,8 +315,8 @@ StorePath BinaryCacheStore::addToStoreFromDump(Source & dump, std::string_view n
                         .hash = nar.first,
                     },
                     .references = {
-                        .references = references,
+                        .others = references,
-                        .hasSelfReference = false,
+                        .self = false,
                     },
                 },
             },
@@ -434,8 +435,8 @@ StorePath BinaryCacheStore::addToStore(
                         .hash = h,
                     },
                     .references = {
-                        .references = references,
+                        .others = references,
-                        .hasSelfReference = false,
+                        .self = false,
                     },
                 },
             },

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

@@ -2421,26 +2421,26 @@ DrvOutputs LocalDerivationGoal::registerOutputs()
             }
         };
 
-        auto rewriteRefs = [&]() -> PathReferences<StorePath> {
+        auto rewriteRefs = [&]() -> StoreReferences {
             /* In the CA case, we need the rewritten refs to calculate the
                final path, therefore we look for a *non-rewritten
                self-reference, and use a bool rather try to solve the
                computationally intractable fixed point. */
-            PathReferences<StorePath> res {
+            StoreReferences res {
-                .hasSelfReference = false,
+                .self = false,
             };
             for (auto & r : references) {
                 auto name = r.name();
                 auto origHash = std::string { r.hashPart() };
                 if (r == *scratchPath) {
-                    res.hasSelfReference = true;
+                    res.self = true;
                 } else if (auto outputRewrite = get(outputRewrites, origHash)) {
                     std::string newRef = *outputRewrite;
                     newRef += '-';
                     newRef += name;
-                    res.references.insert(StorePath { newRef });
+                    res.others.insert(StorePath { newRef });
                 } else {
-                    res.references.insert(r);
+                    res.others.insert(r);
                 }
             }
             return res;
@@ -2523,7 +2523,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs()
                 auto narHashAndSize = hashPath(htSHA256, actualPath);
                 ValidPathInfo newInfo0 { requiredFinalPath, narHashAndSize.first };
                 newInfo0.narSize = narHashAndSize.second;
-                static_cast<PathReferences<StorePath> &>(newInfo0) = rewriteRefs();
+                newInfo0.references = rewriteRefs();
                 return newInfo0;
             },
 

src/libstore/build/substitution-goal.cc

@@ -128,7 +128,7 @@ void PathSubstitutionGoal::tryNext()
     }
 
     if (info->path != storePath) {
-        if (info->isContentAddressed(*sub) && info->references.empty() && !info->hasSelfReference) {
+        if (info->isContentAddressed(*sub) && info->references.empty()) {
             auto info2 = std::make_shared<ValidPathInfo>(*info);
             info2->path = storePath;
             info = info2;
@@ -165,7 +165,7 @@ void PathSubstitutionGoal::tryNext()
 
     /* To maintain the closure invariant, we first have to realise the
        paths referenced by this one. */
-    for (auto & i : info->references)
+    for (auto & i : info->references.others)
         addWaitee(worker.makePathSubstitutionGoal(i));
 
     if (waitees.empty()) /* to prevent hang (no wake-up event) */
@@ -187,7 +187,7 @@ void PathSubstitutionGoal::referencesValid()
         return;
     }
 
-    for (auto & i : info->references)
+    for (auto & i : info->references.others)
         assert(worker.store.isValidPath(i));
 
     state = &PathSubstitutionGoal::tryToRun;

src/libstore/content-address.hh

@@ -4,6 +4,7 @@
 #include "hash.hh"
 #include "path.hh"
 #include "comparator.hh"
+#include "reference-set.hh"
 
 namespace nix {
 
@@ -94,48 +95,7 @@ Hash getContentAddressHash(const ContentAddress & ca);
  * References set
  */
 
-template<typename Ref>
+typedef References<StorePath> StoreReferences;
-struct PathReferences
-{
-    std::set<Ref> references;
-    bool hasSelfReference = false;
-
-    /* Functions to view references + hasSelfReference as one set, mainly for
-       compatibility's sake. */
-    StorePathSet referencesPossiblyToSelf(const Ref & self) const;
-    void insertReferencePossiblyToSelf(const Ref & self, Ref && ref);
-    void setReferencesPossiblyToSelf(const Ref & self, std::set<Ref> && refs);
-
-    GENERATE_CMP(PathReferences<Ref>, me->references, me->hasSelfReference);
-};
-
-template<typename Ref>
-StorePathSet PathReferences<Ref>::referencesPossiblyToSelf(const Ref & self) const
-{
-    StorePathSet refs { references };
-    if (hasSelfReference)
-        refs.insert(self);
-    return refs;
-}
-
-template<typename Ref>
-void PathReferences<Ref>::insertReferencePossiblyToSelf(const Ref & self, Ref && ref)
-{
-    if (ref == self)
-        hasSelfReference = true;
-    else
-        references.insert(std::move(ref));
-}
-
-template<typename Ref>
-void PathReferences<Ref>::setReferencesPossiblyToSelf(const Ref & self, std::set<Ref> && refs)
-{
-    if (refs.count(self))
-        hasSelfReference = true;
-        refs.erase(self);
-
-    references = refs;
-}
 
 /*
  * Full content address
@@ -153,7 +113,7 @@ struct TextInfo : TextHash {
 
 struct FixedOutputInfo : FixedOutputHash {
     // References for the paths
-    PathReferences<StorePath> references;
+    StoreReferences references;
 
     GENERATE_CMP(FixedOutputInfo, *(const FixedOutputHash *)me, me->references);
 };

src/libstore/daemon.cc

@@ -758,7 +758,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
         else {
             to << 1
                << (i->second.deriver ? store->printStorePath(*i->second.deriver) : "");
-            worker_proto::write(*store, to, i->second.referencesPossiblyToSelf(path));
+            worker_proto::write(*store, to, i->second.references.possiblyToSelf(path));
             to << i->second.downloadSize
                << i->second.narSize;
         }
@@ -781,7 +781,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
         for (auto & i : infos) {
             to << store->printStorePath(i.first)
                << (i.second.deriver ? store->printStorePath(*i.second.deriver) : "");
-            worker_proto::write(*store, to, i.second.referencesPossiblyToSelf(i.first));
+            worker_proto::write(*store, to, i.second.references.possiblyToSelf(i.first));
             to << i.second.downloadSize << i.second.narSize;
         }
         break;

src/libstore/local-store.cc

@@ -1157,11 +1157,10 @@ void LocalStore::querySubstitutablePathInfos(const StorePathCAMap & paths, Subst
                 auto narInfo = std::dynamic_pointer_cast<const NarInfo>(
                     std::shared_ptr<const ValidPathInfo>(info));
                 infos.insert_or_assign(path.first, SubstitutablePathInfo{
-                    info->references,
+                    .deriver = info->deriver,
-                    info->hasSelfReference,
+                    .references = info->references,
-                    info->deriver,
+                    .downloadSize = narInfo ? narInfo->fileSize : 0,
-                    narInfo ? narInfo->fileSize : 0,
+                    .narSize = info->narSize,
-                    info->narSize,
                 });
             } catch (InvalidPath &) {
             } catch (SubstituterDisabled &) {
@@ -1228,7 +1227,7 @@ void LocalStore::registerValidPaths(const ValidPathInfos & infos)
         topoSort(paths,
             {[&](const StorePath & path) {
                 auto i = infos.find(path);
-                return i == infos.end() ? StorePathSet() : i->second.references;
+                return i == infos.end() ? StorePathSet() : i->second.references.others;
             }},
             {[&](const StorePath & path, const StorePath & parent) {
                 return BuildError(
@@ -1427,8 +1426,8 @@ StorePath LocalStore::addToStoreFromDump(Source & source0, std::string_view name
                 .hash = hash,
             },
             .references = {
-                .references = references,
+                .others = references,
-                .hasSelfReference = false,
+                .self = false,
             },
         },
     };
@@ -1526,7 +1525,8 @@ StorePath LocalStore::addTextToStore(
 
             ValidPathInfo info { dstPath, narHash };
             info.narSize = sink.s.size();
-            info.references = references;
+            // No self reference allowed with text-hashing
+            info.references.others = references;
             info.ca = TextHash { .hash = hash };
             registerValidPath(info);
         }

src/libstore/make-content-addressed.cc

@@ -27,15 +27,15 @@ std::map<StorePath, StorePath> makeContentAddressed(
 
         StringMap rewrites;
 
-        PathReferences<StorePath> refs;
+        StoreReferences refs;
-        refs.hasSelfReference = oldInfo->hasSelfReference;
+        refs.self = oldInfo->references.self;
-        for (auto & ref : oldInfo->references) {
+        for (auto & ref : oldInfo->references.others) {
             auto i = remappings.find(ref);
             auto replacement = i != remappings.end() ? i->second : ref;
             // FIXME: warn about unremapped paths?
             if (replacement != ref) {
                 rewrites.insert_or_assign(srcStore.printStorePath(ref), srcStore.printStorePath(replacement));
-                refs.references.insert(std::move(replacement));
+                refs.others.insert(std::move(replacement));
             }
         }
 

src/libstore/misc.cc

@@ -40,7 +40,7 @@ void Store::computeFSClosure(const StorePathSet & startPaths,
                         std::future<ref<const ValidPathInfo>> & fut) {
             StorePathSet res;
             auto info = fut.get();
-            for (auto& ref : info->references)
+            for (auto & ref : info->references.others)
                 res.insert(ref);
 
             if (includeOutputs && path.isDerivation())
@@ -223,7 +223,7 @@ void Store::queryMissing(const std::vector<DerivedPath> & targets,
                 state->narSize += info->second.narSize;
             }
 
-            for (auto & ref : info->second.references)
+            for (auto & ref : info->second.references.others)
                 pool.enqueue(std::bind(doPath, DerivedPath::Opaque { ref }));
           },
         }, req.raw());
@@ -241,7 +241,7 @@ StorePaths Store::topoSortPaths(const StorePathSet & paths)
     return topoSort(paths,
         {[&](const StorePath & path) {
             try {
-                return queryPathInfo(path)->references;
+                return queryPathInfo(path)->references.others;
             } catch (InvalidPath &) {
                 return StorePathSet();
             }
@@ -297,7 +297,7 @@ std::map<DrvOutput, StorePath> drvOutputReferences(
 
     auto info = store.queryPathInfo(outputPath);
 
-    return drvOutputReferences(Realisation::closure(store, inputRealisations), info->references);
+    return drvOutputReferences(Realisation::closure(store, inputRealisations), info->referencesPossiblyToSelf());
 }
 
 }

src/libstore/path-info.hh

@@ -14,20 +14,22 @@ namespace nix {
 class Store;
 
 
-struct SubstitutablePathInfo : PathReferences<StorePath>
+struct SubstitutablePathInfo
 {
     std::optional<StorePath> deriver;
+    StoreReferences references;
     uint64_t downloadSize; /* 0 = unknown or inapplicable */
     uint64_t narSize; /* 0 = unknown */
 };
 
 typedef std::map<StorePath, SubstitutablePathInfo> SubstitutablePathInfos;
 
-struct ValidPathInfo : PathReferences<StorePath>
+struct ValidPathInfo
 {
     StorePath path;
     std::optional<StorePath> deriver;
     Hash narHash;
+    StoreReferences references;
     time_t registrationTime = 0;
     uint64_t narSize = 0; // 0 = unknown
     uint64_t id; // internal use only
@@ -61,7 +63,6 @@ struct ValidPathInfo : PathReferences<StorePath>
         return
             path == i.path
             && narHash == i.narHash
-            && hasSelfReference == i.hasSelfReference
             && references == i.references;
     }
 

src/libstore/remote-store.cc

@@ -402,7 +402,7 @@ void RemoteStore::querySubstitutablePathInfos(const StorePathCAMap & pathsMap, S
             auto deriver = readString(conn->from);
             if (deriver != "")
                 info.deriver = parseStorePath(deriver);
-            info.setReferencesPossiblyToSelf(i.first, worker_proto::read(*this, conn->from, Phantom<StorePathSet> {}));
+            info.references.setPossiblyToSelf(i.first, worker_proto::read(*this, conn->from, Phantom<StorePathSet> {}));
             info.downloadSize = readLongLong(conn->from);
             info.narSize = readLongLong(conn->from);
             infos.insert_or_assign(i.first, std::move(info));
@@ -426,7 +426,7 @@ void RemoteStore::querySubstitutablePathInfos(const StorePathCAMap & pathsMap, S
             auto deriver = readString(conn->from);
             if (deriver != "")
                 info.deriver = parseStorePath(deriver);
-            info.setReferencesPossiblyToSelf(path, worker_proto::read(*this, conn->from, Phantom<StorePathSet> {}));
+            info.references.setPossiblyToSelf(path, worker_proto::read(*this, conn->from, Phantom<StorePathSet> {}));
             info.downloadSize = readLongLong(conn->from);
             info.narSize = readLongLong(conn->from);
         }

src/libstore/store-api.cc

@@ -169,13 +169,13 @@ StorePath Store::makeOutputPath(std::string_view id,
 static std::string makeType(
     const Store & store,
     std::string && type,
-    const PathReferences<StorePath> & references)
+    const StoreReferences & references)
 {
-    for (auto & i : references.references) {
+    for (auto & i : references.others) {
         type += ":";
         type += store.printStorePath(i);
     }
-    if (references.hasSelfReference) type += ":self";
+    if (references.self) type += ":self";
     return std::move(type);
 }
 
@@ -185,8 +185,7 @@ StorePath Store::makeFixedOutputPath(std::string_view name, const FixedOutputInf
     if (info.hash.type == htSHA256 && info.method == FileIngestionMethod::Recursive) {
         return makeStorePath(makeType(*this, "source", info.references), info.hash, name);
     } else {
-        assert(info.references.references.size() == 0);
+        assert(info.references.size() == 0);
-        assert(!info.references.hasSelfReference);
         return makeStorePath("output:out",
             hashString(htSHA256,
                 "fixed:out:"
@@ -201,7 +200,7 @@ StorePath Store::makeTextPath(std::string_view name, const TextInfo & info) cons
 {
     assert(info.hash.type == htSHA256);
     return makeStorePath(
-        makeType(*this, "text", PathReferences<StorePath> { info.references }),
+        makeType(*this, "text", StoreReferences { info.references }),
         info.hash,
         name);
 }
@@ -311,7 +310,7 @@ void Store::addMultipleToStore(
             bytesExpected += info.narSize;
             act.setExpected(actCopyPath, bytesExpected);
 
-            return info.references;
+            return info.references.others;
         },
 
         [&](const StorePath & path) {
@@ -816,7 +815,7 @@ std::string Store::makeValidityRegistration(const StorePathSet & paths,
 
         s += (format("%1%\n") % info->references.size()).str();
 
-        for (auto & j : info->references)
+        for (auto & j : info->referencesPossiblyToSelf())
             s += printStorePath(j) + "\n";
     }
 
@@ -878,7 +877,7 @@ json Store::pathInfoToJSON(const StorePathSet & storePaths,
 
             {
                 auto& jsonRefs = (jsonPath["references"] = json::array());
-                for (auto & ref : info->references)
+                for (auto & ref : info->referencesPossiblyToSelf())
                     jsonRefs.emplace_back(printStorePath(ref));
             }
 
@@ -1231,17 +1230,17 @@ std::string showPaths(const PathSet & paths)
 
 StorePathSet ValidPathInfo::referencesPossiblyToSelf() const
 {
-    return PathReferences<StorePath>::referencesPossiblyToSelf(path);
+    return references.possiblyToSelf(path);
 }
 
 void ValidPathInfo::insertReferencePossiblyToSelf(StorePath && ref)
 {
-    return PathReferences<StorePath>::insertReferencePossiblyToSelf(path, std::move(ref));
+    return references.insertPossiblyToSelf(path, std::move(ref));
 }
 
 void ValidPathInfo::setReferencesPossiblyToSelf(StorePathSet && refs)
 {
-    return PathReferences<StorePath>::setReferencesPossiblyToSelf(path, std::move(refs));
+    return references.setPossiblyToSelf(path, std::move(refs));
 }
 
 std::string ValidPathInfo::fingerprint(const Store & store) const
@@ -1271,16 +1270,16 @@ std::optional<StorePathDescriptor> ValidPathInfo::fullStorePathDescriptorOpt() c
         .name = std::string { path.name() },
         .info = std::visit(overloaded {
             [&](const TextHash & th) -> ContentAddressWithReferences {
-                assert(!hasSelfReference);
+                assert(!references.self);
                 return TextInfo {
                     th,
-                    .references = references,
+                    .references = references.others,
                 };
             },
             [&](const FixedOutputHash & foh) -> ContentAddressWithReferences {
                 return FixedOutputInfo {
                     foh,
-                    .references = static_cast<PathReferences<StorePath>>(*this),
+                    .references = references,
                 };
             },
         }, *ca),
@@ -1341,11 +1340,14 @@ ValidPathInfo::ValidPathInfo(
 {
     std::visit(overloaded {
         [this](TextInfo && ti) {
-            this->references = std::move(ti.references);
+            this->references = {
+                .others = std::move(ti.references),
+                .self = false,
+            };
             this->ca = std::move((TextHash &&) ti);
         },
         [this](FixedOutputInfo && foi) {
-            *(static_cast<PathReferences<StorePath> *>(this)) = std::move(foi.references);
+            this->references = std::move(foi.references);
             this->ca = std::move((FixedOutputHash &&) foi);
         },
     }, std::move(info.info));

src/libutil/reference-set.hh

@@ -0,0 +1,68 @@
+#pragma once
+
+#include "comparator.hh"
+
+#include <set>
+
+namespace nix {
+
+template<typename Ref>
+struct References
+{
+    std::set<Ref> others;
+    bool self = false;
+
+    bool empty() const;
+    size_t size() const;
+
+    /* Functions to view references + self as one set, mainly for
+       compatibility's sake. */
+    std::set<Ref> possiblyToSelf(const Ref & self) const;
+    void insertPossiblyToSelf(const Ref & self, Ref && ref);
+    void setPossiblyToSelf(const Ref & self, std::set<Ref> && refs);
+
+    GENERATE_CMP(References<Ref>, me->others, me->self);
+};
+
+template<typename Ref>
+bool References<Ref>::empty() const
+{
+    return !self && others.empty();
+}
+
+template<typename Ref>
+size_t References<Ref>::size() const
+{
+    return (self ? 1 : 0) + others.size();
+}
+
+template<typename Ref>
+std::set<Ref> References<Ref>::possiblyToSelf(const Ref & selfRef) const
+{
+    std::set<Ref> refs { others };
+    if (self)
+        refs.insert(selfRef);
+    return refs;
+}
+
+template<typename Ref>
+void References<Ref>::insertPossiblyToSelf(const Ref & selfRef, Ref && ref)
+{
+    if (ref == selfRef)
+        self = true;
+    else
+        others.insert(std::move(ref));
+}
+
+template<typename Ref>
+void References<Ref>::setPossiblyToSelf(const Ref & selfRef, std::set<Ref> && refs)
+{
+    if (refs.count(selfRef)) {
+        self = true;
+        refs.erase(selfRef);
+    }
+
+    others = refs;
+}
+
+}

src/nix/profile.cc

@@ -205,12 +205,14 @@ struct ProfileManifest
                         .method = FileIngestionMethod::Recursive,
                         .hash = narHash,
                     },
-                    .references = { references },
+                    .references = {
+                        .others = std::move(references),
+                        .self = false,
+                    },
                 },
             },
             narHash,
         };
-        info.references = std::move(references);
         info.narSize = sink.s.size();
 
         StringSource source(sink.s);

src/nix/sigs.cc

@@ -63,8 +63,7 @@ struct CmdCopySigs : StorePathsCommand
                        binary. */
                     if (info->narHash != info2->narHash ||
                         info->narSize != info2->narSize ||
-                        info->references != info2->references ||
+                        info->references != info2->references)
-                        info->hasSelfReference != info2->hasSelfReference)
                         continue;
 
                     for (auto & sig : info2->sigs)

src/nix/why-depends.cc

@@ -136,7 +136,7 @@ struct CmdWhyDepends : SourceExprCommand
         for (auto & path : closure)
             graph.emplace(path, Node {
                 .path = path,
-                .refs = store->queryPathInfo(path)->references,
+                .refs = store->queryPathInfo(path)->references.others,
                 .dist = path == dependencyPath ? 0 : inf
             });