Merge pull request NixOS#10838 from obsidiansystems/spaceship-no-comp…

…arator

Remove `comparator.hh` and switch to `<=>` in a bunch of places

src/libcmd/built-path.cc

@@ -1,37 +1,32 @@
 #include "built-path.hh"
 #include "derivations.hh"
 #include "store-api.hh"
+#include "comparator.hh"
 
 #include <nlohmann/json.hpp>
 
 #include <optional>
 
 namespace nix {
 
-#define CMP_ONE(CHILD_TYPE, MY_TYPE, FIELD, COMPARATOR) \
-    bool MY_TYPE ::operator COMPARATOR (const MY_TYPE & other) const \
-    { \
-        const MY_TYPE* me = this; \
-        auto fields1 = std::tie(*me->drvPath, me->FIELD); \
-        me = &other; \
-        auto fields2 = std::tie(*me->drvPath, me->FIELD); \
-        return fields1 COMPARATOR fields2; \
-    }
-#define CMP(CHILD_TYPE, MY_TYPE, FIELD) \
-    CMP_ONE(CHILD_TYPE, MY_TYPE, FIELD, ==) \
-    CMP_ONE(CHILD_TYPE, MY_TYPE, FIELD, !=) \
-    CMP_ONE(CHILD_TYPE, MY_TYPE, FIELD, <)
-
-#define FIELD_TYPE std::pair<std::string, StorePath>
-CMP(SingleBuiltPath, SingleBuiltPathBuilt, output)
-#undef FIELD_TYPE
-
-#define FIELD_TYPE std::map<std::string, StorePath>
-CMP(SingleBuiltPath, BuiltPathBuilt, outputs)
-#undef FIELD_TYPE
-
-#undef CMP
-#undef CMP_ONE
+// Custom implementation to avoid `ref` ptr equality
+GENERATE_CMP_EXT(
+    ,
+    std::strong_ordering,
+    SingleBuiltPathBuilt,
+    *me->drvPath,
+    me->output);
+
+// Custom implementation to avoid `ref` ptr equality
+
+// TODO no `GENERATE_CMP_EXT` because no `std::set::operator<=>` on
+// Darwin, per header.
+GENERATE_EQUAL(
+    ,
+    BuiltPathBuilt ::,
+    BuiltPathBuilt,
+    *me->drvPath,
+    me->outputs);
 
 StorePath SingleBuiltPath::outPath() const
 {

src/libcmd/built-path.hh

@@ -18,7 +18,8 @@ struct SingleBuiltPathBuilt {
     static SingleBuiltPathBuilt parse(const StoreDirConfig & store, std::string_view, std::string_view);
     nlohmann::json toJSON(const StoreDirConfig & store) const;
 
-    DECLARE_CMP(SingleBuiltPathBuilt);
+    bool operator ==(const SingleBuiltPathBuilt &) const noexcept;
+    std::strong_ordering operator <=>(const SingleBuiltPathBuilt &) const noexcept;
 };
 
 using _SingleBuiltPathRaw = std::variant<
@@ -33,6 +34,9 @@ struct SingleBuiltPath : _SingleBuiltPathRaw {
     using Opaque = DerivedPathOpaque;
     using Built = SingleBuiltPathBuilt;
 
+    bool operator == (const SingleBuiltPath &) const = default;
+    auto operator <=> (const SingleBuiltPath &) const = default;
+
     inline const Raw & raw() const {
         return static_cast<const Raw &>(*this);
     }
@@ -59,11 +63,13 @@ struct BuiltPathBuilt {
     ref<SingleBuiltPath> drvPath;
     std::map<std::string, StorePath> outputs;
 
+    bool operator == (const BuiltPathBuilt &) const noexcept;
+    // TODO libc++ 16 (used by darwin) missing `std::map::operator <=>`, can't do yet.
+    //std::strong_ordering operator <=> (const BuiltPathBuilt &) const noexcept;
+
     std::string to_string(const StoreDirConfig & store) const;
     static BuiltPathBuilt parse(const StoreDirConfig & store, std::string_view, std::string_view);
     nlohmann::json toJSON(const StoreDirConfig & store) const;
-
-    DECLARE_CMP(BuiltPathBuilt);
 };
 
 using _BuiltPathRaw = std::variant<
@@ -82,6 +88,10 @@ struct BuiltPath : _BuiltPathRaw {
     using Opaque = DerivedPathOpaque;
     using Built = BuiltPathBuilt;
 
+    bool operator == (const BuiltPath &) const = default;
+    // TODO libc++ 16 (used by darwin) missing `std::map::operator <=>`, can't do yet.
+    //auto operator <=> (const BuiltPath &) const = default;
+
     inline const Raw & raw() const {
         return static_cast<const Raw &>(*this);
     }

src/libexpr-c/nix_api_external.cc

@@ -115,7 +115,7 @@ class NixCExternalValue : public nix::ExternalValueBase
     /**
      * Compare to another value of the same type.
      */
-    virtual bool operator==(const ExternalValueBase & b) const override
+    virtual bool operator==(const ExternalValueBase & b) const noexcept override
     {
         if (!desc.equal) {
             return false;

src/libexpr/attr-set.hh

@@ -27,9 +27,9 @@ struct Attr
     Attr(Symbol name, Value * value, PosIdx pos = noPos)
         : name(name), pos(pos), value(value) { };
     Attr() { };
-    bool operator < (const Attr & a) const
+    auto operator <=> (const Attr & a) const
     {
-        return name < a.name;
+        return name <=> a.name;
     }
 };
 

src/libexpr/eval.cc

@@ -2842,7 +2842,7 @@ std::string ExternalValueBase::coerceToString(EvalState & state, const PosIdx &
 }
 
 
-bool ExternalValueBase::operator==(const ExternalValueBase & b) const
+bool ExternalValueBase::operator==(const ExternalValueBase & b) const noexcept
 {
     return false;
 }

src/libexpr/pos-idx.hh

@@ -28,20 +28,15 @@ public:
         return id > 0;
     }
 
-    bool operator<(const PosIdx other) const
+    auto operator<=>(const PosIdx other) const
     {
-        return id < other.id;
+        return id <=> other.id;
     }
 
     bool operator==(const PosIdx other) const
     {
         return id == other.id;
     }
-
-    bool operator!=(const PosIdx other) const
-    {
-        return id != other.id;
-    }
 };
 
 inline PosIdx noPos = {};

src/libexpr/symbol-table.hh

@@ -62,9 +62,8 @@ public:
 
     explicit operator bool() const { return id > 0; }
 
-    bool operator<(const Symbol other) const { return id < other.id; }
+    auto operator<=>(const Symbol other) const { return id <=> other.id; }
     bool operator==(const Symbol other) const { return id == other.id; }
-    bool operator!=(const Symbol other) const { return id != other.id; }
 };
 
 /**

src/libexpr/value.hh

@@ -111,7 +111,7 @@ class ExternalValueBase
      * Compare to another value of the same type. Defaults to uncomparable,
      * i.e. always false.
      */
-    virtual bool operator ==(const ExternalValueBase & b) const;
+    virtual bool operator ==(const ExternalValueBase & b) const noexcept;
 
     /**
      * Print the value as JSON. Defaults to unconvertable, i.e. throws an error

src/libfetchers/fetchers.cc

@@ -150,7 +150,7 @@ Attrs Input::toAttrs() const
     return attrs;
 }
 
-bool Input::operator ==(const Input & other) const
+bool Input::operator ==(const Input & other) const noexcept
 {
     return attrs == other.attrs;
 }

src/libfetchers/fetchers.hh

@@ -89,7 +89,7 @@ public:
      */
     bool isLocked() const;
 
-    bool operator ==(const Input & other) const;
+    bool operator ==(const Input & other) const noexcept;
 
     bool contains(const Input & other) const;
 

src/libflake/flake/flakeref.cc

@@ -36,11 +36,6 @@ std::ostream & operator << (std::ostream & str, const FlakeRef & flakeRef)
     return str;
 }
 
-bool FlakeRef::operator ==(const FlakeRef & other) const
-{
-    return input == other.input && subdir == other.subdir;
-}
-
 FlakeRef FlakeRef::resolve(ref<Store> store) const
 {
     auto [input2, extraAttrs] = lookupInRegistries(store, input);

src/libflake/flake/flakeref.hh

@@ -46,7 +46,7 @@ struct FlakeRef
      */
     Path subdir;
 
-    bool operator==(const FlakeRef & other) const;
+    bool operator ==(const FlakeRef & other) const = default;
 
     FlakeRef(fetchers::Input && input, const Path & subdir)
         : input(std::move(input)), subdir(subdir)

src/libflake/flake/lockfile.cc

@@ -249,11 +249,6 @@ bool LockFile::operator ==(const LockFile & other) const
     return toJSON().first == other.toJSON().first;
 }
 
-bool LockFile::operator !=(const LockFile & other) const
-{
-    return !(*this == other);
-}
-
 InputPath parseInputPath(std::string_view s)
 {
     InputPath path;

src/libflake/flake/lockfile.hh

@@ -74,9 +74,6 @@ struct LockFile
     std::optional<FlakeRef> isUnlocked() const;
 
     bool operator ==(const LockFile & other) const;
-    // Needed for old gcc versions that don't synthesize it (like gcc 8.2.2
-    // that is still the default on aarch64-linux)
-    bool operator !=(const LockFile & other) const;
 
     std::shared_ptr<Node> findInput(const InputPath & path);
 

src/libstore/build-result.cc

@@ -2,17 +2,7 @@
 
 namespace nix {
 
-GENERATE_CMP_EXT(
-    ,
-    BuildResult,
-    me->status,
-    me->errorMsg,
-    me->timesBuilt,
-    me->isNonDeterministic,
-    me->builtOutputs,
-    me->startTime,
-    me->stopTime,
-    me->cpuUser,
-    me->cpuSystem);
+bool BuildResult::operator==(const BuildResult &) const noexcept = default;
+std::strong_ordering BuildResult::operator<=>(const BuildResult &) const noexcept = default;
 
 }

src/libstore/build-result.hh

@@ -3,7 +3,6 @@
 
 #include "realisation.hh"
 #include "derived-path.hh"
-#include "comparator.hh"
 
 #include <string>
 #include <chrono>
@@ -101,7 +100,8 @@ struct BuildResult
      */
     std::optional<std::chrono::microseconds> cpuUser, cpuSystem;
 
-    DECLARE_CMP(BuildResult);
+    bool operator ==(const BuildResult &) const noexcept;
+    std::strong_ordering operator <=>(const BuildResult &) const noexcept;
 
     bool success()
     {

src/libstore/content-address.hh

@@ -73,6 +73,7 @@ struct ContentAddressMethod
 
     Raw raw;
 
+    bool operator ==(const ContentAddressMethod &) const = default;
     auto operator <=>(const ContentAddressMethod &) const = default;
 
     MAKE_WRAPPER_CONSTRUCTOR(ContentAddressMethod);
@@ -159,6 +160,7 @@ struct ContentAddress
      */
     Hash hash;
 
+    bool operator ==(const ContentAddress &) const = default;
     auto operator <=>(const ContentAddress &) const = default;
 
     /**
@@ -217,6 +219,10 @@ struct StoreReferences
      * iff self is true.
      */
     size_t size() const;
+
+    bool operator ==(const StoreReferences &) const = default;
+    // TODO libc++ 16 (used by darwin) missing `std::map::operator <=>`, can't do yet.
+    //auto operator <=>(const StoreReferences &) const = default;
 };
 
 // This matches the additional info that we need for makeTextPath
@@ -232,6 +238,10 @@ struct TextInfo
      * disallowed
      */
     StorePathSet references;
+
+    bool operator ==(const TextInfo &) const = default;
+    // TODO libc++ 16 (used by darwin) missing `std::map::operator <=>`, can't do yet.
+    //auto operator <=>(const TextInfo &) const = default;
 };
 
 struct FixedOutputInfo
@@ -250,6 +260,10 @@ struct FixedOutputInfo
      * References to other store objects or this one.
      */
     StoreReferences references;
+
+    bool operator ==(const FixedOutputInfo &) const = default;
+    // TODO libc++ 16 (used by darwin) missing `std::map::operator <=>`, can't do yet.
+    //auto operator <=>(const FixedOutputInfo &) const = default;
 };
 
 /**
@@ -266,6 +280,10 @@ struct ContentAddressWithReferences
 
     Raw raw;
 
+    bool operator ==(const ContentAddressWithReferences &) const = default;
+    // TODO libc++ 16 (used by darwin) missing `std::map::operator <=>`, can't do yet.
+    //auto operator <=>(const ContentAddressWithReferences &) const = default;
+
     MAKE_WRAPPER_CONSTRUCTOR(ContentAddressWithReferences);
 
     /**