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

Known behavior changes:

- `MemorySourceAccessor`'s comparison operators no longer forget to
  compare the `SourceAccessor` base class.

Progress on #10832

What remains for that issue is hopefully much easier!

src/libutil/args.cc

@@ -57,8 +57,7 @@ void Completions::add(std::string completion, std::string description)
     });
 }
 
-bool Completion::operator<(const Completion & other) const
-{ return completion < other.completion || (completion == other.completion && description < other.description); }
+auto Completion::operator<=>(const Completion & other) const noexcept = default;
 
 std::string completionMarker = "___COMPLETE___";
 

src/libutil/args.hh

@@ -380,7 +380,7 @@ struct Completion {
     std::string completion;
     std::string description;
 
-    bool operator<(const Completion & other) const;
+    auto operator<=>(const Completion & other) const noexcept;
 };
 
 /**

src/libutil/canon-path.hh

@@ -169,7 +169,7 @@ public:
      * a directory is always followed directly by its children. For
      * instance, 'foo' < 'foo/bar' < 'foo!'.
      */
-    bool operator < (const CanonPath & x) const
+    auto operator <=> (const CanonPath & x) const
     {
         auto i = path.begin();
         auto j = x.path.begin();
@@ -178,10 +178,9 @@ public:
             if (c_i == '/') c_i = 0;
             auto c_j = *j;
             if (c_j == '/') c_j = 0;
-            if (c_i < c_j) return true;
-            if (c_i > c_j) return false;
+            if (auto cmp = c_i <=> c_j; cmp != 0) return cmp;
         }
-        return i == path.end() && j != x.path.end();
+        return (i != path.end()) <=> (j != x.path.end());
     }
 
     /**

src/libutil/comparator.hh

@@ -1,17 +1,8 @@
 #pragma once
 ///@file
 
-#define DECLARE_ONE_CMP(PRE, QUAL, COMPARATOR, MY_TYPE) \
-    PRE bool QUAL operator COMPARATOR(const MY_TYPE & other) const;
-#define DECLARE_EQUAL(prefix, qualification, my_type) \
-    DECLARE_ONE_CMP(prefix, qualification, ==, my_type)
-#define DECLARE_LEQ(prefix, qualification, my_type) \
-    DECLARE_ONE_CMP(prefix, qualification, <, my_type)
-#define DECLARE_NEQ(prefix, qualification, my_type) \
-    DECLARE_ONE_CMP(prefix, qualification, !=, my_type)
-
-#define GENERATE_ONE_CMP(PRE, QUAL, COMPARATOR, MY_TYPE, ...) \
-    PRE bool QUAL operator COMPARATOR(const MY_TYPE & other) const { \
+#define GENERATE_ONE_CMP(PRE, RET, QUAL, COMPARATOR, MY_TYPE, ...) \
+    PRE RET QUAL operator COMPARATOR(const MY_TYPE & other) const noexcept { \
       __VA_OPT__(const MY_TYPE * me = this;) \
       auto fields1 = std::tie( __VA_ARGS__ ); \
       __VA_OPT__(me = &other;) \
@@ -19,30 +10,9 @@
       return fields1 COMPARATOR fields2; \
     }
 #define GENERATE_EQUAL(prefix, qualification, my_type, args...) \
-    GENERATE_ONE_CMP(prefix, qualification, ==, my_type, args)
-#define GENERATE_LEQ(prefix, qualification, my_type, args...) \
-    GENERATE_ONE_CMP(prefix, qualification, <, my_type, args)
-#define GENERATE_NEQ(prefix, qualification, my_type, args...) \
-    GENERATE_ONE_CMP(prefix, qualification, !=, my_type, args)
-
-/**
- * Declare comparison methods without defining them.
- */
-#define DECLARE_CMP(my_type) \
-    DECLARE_EQUAL(,,my_type) \
-    DECLARE_LEQ(,,my_type) \
-    DECLARE_NEQ(,,my_type)
-
-/**
- * @param prefix This is for something before each declaration like
- * `template<classname Foo>`.
- *
- * @param my_type the type are defining operators for.
- */
-#define DECLARE_CMP_EXT(prefix, qualification, my_type) \
-    DECLARE_EQUAL(prefix, qualification, my_type) \
-    DECLARE_LEQ(prefix, qualification, my_type) \
-    DECLARE_NEQ(prefix, qualification, my_type)
+    GENERATE_ONE_CMP(prefix, bool, qualification, ==, my_type, args)
+#define GENERATE_SPACESHIP(prefix, ret, qualification, my_type, args...) \
+    GENERATE_ONE_CMP(prefix, ret, qualification, <=>, my_type, args)
 
 /**
  * Awful hacky generation of the comparison operators by doing a lexicographic
@@ -55,15 +25,19 @@
  * will generate comparison operators semantically equivalent to:
  *
  * ```
- * bool operator<(const ClassName& other) {
- *   return field1 < other.field1 && field2 < other.field2 && ...;
+ * auto operator<=>(const ClassName& other) const noexcept {
+ *   if (auto cmp = field1 <=> other.field1; cmp != 0)
+ *      return cmp;
+ *   if (auto cmp = field2 <=> other.field2; cmp != 0)
+ *      return cmp;
+ *   ...
+ *   return 0;
  * }
  * ```
  */
 #define GENERATE_CMP(args...) \
     GENERATE_EQUAL(,,args) \
-    GENERATE_LEQ(,,args) \
-    GENERATE_NEQ(,,args)
+    GENERATE_SPACESHIP(,auto,,args)
 
 /**
  * @param prefix This is for something before each declaration like
@@ -71,7 +45,6 @@
  *
  * @param my_type the type are defining operators for.
  */
-#define GENERATE_CMP_EXT(prefix, my_type, args...) \
+#define GENERATE_CMP_EXT(prefix, ret, my_type, args...) \
     GENERATE_EQUAL(prefix, my_type ::, my_type, args) \
-    GENERATE_LEQ(prefix, my_type ::, my_type, args) \
-    GENERATE_NEQ(prefix, my_type ::, my_type, args)
+    GENERATE_SPACESHIP(prefix, ret, my_type ::, my_type, args)

src/libutil/error.cc

@@ -46,27 +46,22 @@ std::ostream & operator <<(std::ostream & os, const HintFmt & hf)
 /**
  * An arbitrarily defined value comparison for the purpose of using traces in the key of a sorted container.
  */
-inline bool operator<(const Trace& lhs, const Trace& rhs)
+inline std::strong_ordering operator<=>(const Trace& lhs, const Trace& rhs)
 {
     // `std::shared_ptr` does not have value semantics for its comparison
     // functions, so we need to check for nulls and compare the dereferenced
     // values here.
     if (lhs.pos != rhs.pos) {
-        if (!lhs.pos)
-            return true;
-        if (!rhs.pos)
-            return false;
-        if (*lhs.pos != *rhs.pos)
-            return *lhs.pos < *rhs.pos;
+        if (auto cmp = bool{lhs.pos} <=> bool{rhs.pos}; cmp != 0)
+            return cmp;
+        if (auto cmp = *lhs.pos <=> *rhs.pos; cmp != 0)
+            return cmp;
     }
     // This formats a freshly formatted hint string and then throws it away, which
     // shouldn't be much of a problem because it only runs when pos is equal, and this function is
     // used for trace printing, which is infrequent.
-    return lhs.hint.str() < rhs.hint.str();
+    return lhs.hint.str() <=> rhs.hint.str();
 }
-inline bool operator> (const Trace& lhs, const Trace& rhs) { return rhs < lhs; }
-inline bool operator<=(const Trace& lhs, const Trace& rhs) { return !(lhs > rhs); }
-inline bool operator>=(const Trace& lhs, const Trace& rhs) { return !(lhs < rhs); }
 
 // print lines of code to the ostream, indicating the error column.
 void printCodeLines(std::ostream & out,

src/libutil/error.hh

@@ -75,10 +75,7 @@ struct Trace {
     TracePrint print = TracePrint::Default;
 };
 
-inline bool operator<(const Trace& lhs, const Trace& rhs);
-inline bool operator> (const Trace& lhs, const Trace& rhs);
-inline bool operator<=(const Trace& lhs, const Trace& rhs);
-inline bool operator>=(const Trace& lhs, const Trace& rhs);
+inline std::strong_ordering operator<=>(const Trace& lhs, const Trace& rhs);
 
 struct ErrorInfo {
     Verbosity level;

src/libutil/experimental-features.hh

@@ -1,7 +1,6 @@
 #pragma once
 ///@file
 
-#include "comparator.hh"
 #include "error.hh"
 #include "json-utils.hh"
 #include "types.hh"

src/libutil/git.hh

@@ -39,7 +39,8 @@ struct TreeEntry
     Mode mode;
     Hash hash;
 
-    GENERATE_CMP(TreeEntry, me->mode, me->hash);
+    bool operator ==(const TreeEntry &) const = default;
+    auto operator <=>(const TreeEntry &) const = default;
 };
 
 /**

src/libutil/hash.cc

@@ -41,7 +41,7 @@ Hash::Hash(HashAlgorithm algo) : algo(algo)
 }
 
 
-bool Hash::operator == (const Hash & h2) const
+bool Hash::operator == (const Hash & h2) const noexcept
 {
     if (hashSize != h2.hashSize) return false;
     for (unsigned int i = 0; i < hashSize; i++)
@@ -50,7 +50,7 @@ bool Hash::operator == (const Hash & h2) const
 }
 
 
-std::strong_ordering Hash::operator <=> (const Hash & h) const
+std::strong_ordering Hash::operator <=> (const Hash & h) const noexcept
 {
     if (auto cmp = hashSize <=> h.hashSize; cmp != 0) return cmp;
     for (unsigned int i = 0; i < hashSize; i++) {

src/libutil/hash.hh

@@ -88,12 +88,12 @@ public:
     /**
      * Check whether two hashes are equal.
      */
-    bool operator == (const Hash & h2) const;
+    bool operator == (const Hash & h2) const noexcept;
 
     /**
      * Compare how two hashes are ordered.
      */
-    std::strong_ordering operator <=> (const Hash & h2) const;
+    std::strong_ordering operator <=> (const Hash & h2) const noexcept;
 
     /**
      * Returns the length of a base-16 representation of this hash.

src/libutil/memory-source-accessor.hh

@@ -15,11 +15,15 @@ struct MemorySourceAccessor : virtual SourceAccessor
      * defining what a "file system object" is in Nix.
      */
     struct File {
+        bool operator == (const File &) const noexcept;
+        std::strong_ordering operator <=> (const File &) const noexcept;
+
         struct Regular {
             bool executable = false;
             std::string contents;
 
-            GENERATE_CMP(Regular, me->executable, me->contents);
+            bool operator == (const Regular &) const = default;
+            auto operator <=> (const Regular &) const = default;
         };
 
         struct Directory {
@@ -27,13 +31,16 @@ struct MemorySourceAccessor : virtual SourceAccessor
 
             std::map<Name, File, std::less<>> contents;
 
-            GENERATE_CMP(Directory, me->contents);
+            bool operator == (const Directory &) const noexcept;
+            // TODO libc++ 16 (used by darwin) missing `std::map::operator <=>`, can't do yet.
+            bool operator < (const Directory &) const noexcept;
         };
 
         struct Symlink {
             std::string target;
 
-            GENERATE_CMP(Symlink, me->target);
+            bool operator == (const Symlink &) const = default;
+            auto operator <=> (const Symlink &) const = default;
         };
 
         using Raw = std::variant<Regular, Directory, Symlink>;
@@ -41,14 +48,15 @@ struct MemorySourceAccessor : virtual SourceAccessor
 
         MAKE_WRAPPER_CONSTRUCTOR(File);
 
-        GENERATE_CMP(File, me->raw);
-
         Stat lstat() const;
     };
 
     File root { File::Directory {} };
 
-    GENERATE_CMP(MemorySourceAccessor, me->root);
+    bool operator == (const MemorySourceAccessor &) const noexcept = default;
+    bool operator < (const MemorySourceAccessor & other) const noexcept {
+        return root < other.root;
+    }
 
     std::string readFile(const CanonPath & path) override;
     bool pathExists(const CanonPath & path) override;
@@ -72,6 +80,20 @@ struct MemorySourceAccessor : virtual SourceAccessor
     SourcePath addFile(CanonPath path, std::string && contents);
 };
 
+
+inline bool MemorySourceAccessor::File::Directory::operator == (
+    const MemorySourceAccessor::File::Directory &) const noexcept = default;
+inline bool MemorySourceAccessor::File::Directory::operator < (
+    const MemorySourceAccessor::File::Directory & other) const noexcept
+{
+    return contents < other.contents;
+}
+
+inline bool MemorySourceAccessor::File::operator == (
+    const MemorySourceAccessor::File &) const noexcept = default;
+inline std::strong_ordering MemorySourceAccessor::File::operator <=> (
+    const MemorySourceAccessor::File &) const noexcept = default;
+
 /**
  * Write to a `MemorySourceAccessor` at the given path
  */

src/libutil/position.cc

@@ -17,12 +17,6 @@ Pos::operator std::shared_ptr<Pos>() const
     return std::make_shared<Pos>(&*this);
 }
 
-bool Pos::operator<(const Pos &rhs) const
-{
-    return std::forward_as_tuple(line, column, origin)
-        < std::forward_as_tuple(rhs.line, rhs.column, rhs.origin);
-}
-
 std::optional<LinesOfCode> Pos::getCodeLines() const
 {
     if (line == 0)

src/libutil/position.hh

@@ -22,21 +22,17 @@ struct Pos
 
     struct Stdin {
         ref<std::string> source;
-        bool operator==(const Stdin & rhs) const
+        bool operator==(const Stdin & rhs) const noexcept
         { return *source == *rhs.source; }
-        bool operator!=(const Stdin & rhs) const
-        { return *source != *rhs.source; }
-        bool operator<(const Stdin & rhs) const
-        { return *source < *rhs.source; }
+        std::strong_ordering operator<=>(const Stdin & rhs) const noexcept
+        { return *source <=> *rhs.source; }
     };
     struct String {
         ref<std::string> source;
-        bool operator==(const String & rhs) const
+        bool operator==(const String & rhs) const noexcept
         { return *source == *rhs.source; }
-        bool operator!=(const String & rhs) const
-        { return *source != *rhs.source; }
-        bool operator<(const String & rhs) const
-        { return *source < *rhs.source; }
+        std::strong_ordering operator<=>(const String & rhs) const noexcept
+        { return *source <=> *rhs.source; }
     };
 
     typedef std::variant<std::monostate, Stdin, String, SourcePath> Origin;
@@ -65,8 +61,7 @@ struct Pos
     std::optional<LinesOfCode> getCodeLines() const;
 
     bool operator==(const Pos & rhs) const = default;
-    bool operator!=(const Pos & rhs) const = default;
-    bool operator<(const Pos & rhs) const;
+    auto operator<=>(const Pos & rhs) const = default;
 
     struct LinesIterator {
         using difference_type = size_t;

src/libutil/ref.hh

@@ -87,9 +87,9 @@ public:
         return p != other.p;
     }
 
-    bool operator < (const ref<T> & other) const
+    auto operator <=> (const ref<T> & other) const
     {
-        return p < other.p;
+        return p <=> other.p;
     }
 
 private:

src/libutil/source-accessor.hh

@@ -152,9 +152,9 @@ struct SourceAccessor : std::enable_shared_from_this<SourceAccessor>
         return number == x.number;
     }
 
-    bool operator < (const SourceAccessor & x) const
+    auto operator <=> (const SourceAccessor & x) const
     {
-        return number < x.number;
+        return number <=> x.number;
     }
 
     void setPathDisplay(std::string displayPrefix, std::string displaySuffix = "");

src/libutil/source-path.cc

@@ -47,19 +47,14 @@ SourcePath SourcePath::operator / (const CanonPath & x) const
 SourcePath SourcePath::operator / (std::string_view c) const
 { return {accessor, path / c}; }
 
-bool SourcePath::operator==(const SourcePath & x) const
+bool SourcePath::operator==(const SourcePath & x) const noexcept
 {
     return std::tie(*accessor, path) == std::tie(*x.accessor, x.path);
 }
 
-bool SourcePath::operator!=(const SourcePath & x) const
+std::strong_ordering SourcePath::operator<=>(const SourcePath & x) const noexcept
 {
-    return std::tie(*accessor, path) != std::tie(*x.accessor, x.path);
-}
-
-bool SourcePath::operator<(const SourcePath & x) const
-{
-    return std::tie(*accessor, path) < std::tie(*x.accessor, x.path);
+    return std::tie(*accessor, path) <=> std::tie(*x.accessor, x.path);
 }
 
 std::ostream & operator<<(std::ostream & str, const SourcePath & path)

src/libutil/source-path.hh

@@ -103,9 +103,8 @@ struct SourcePath
      */
     SourcePath operator / (std::string_view c) const;
 
-    bool operator==(const SourcePath & x) const;
-    bool operator!=(const SourcePath & x) const;
-    bool operator<(const SourcePath & x) const;
+    bool operator==(const SourcePath & x) const noexcept;
+    std::strong_ordering operator<=>(const SourcePath & x) const noexcept;
 
     /**
      * Convenience wrapper around `SourceAccessor::resolveSymlinks()`.

src/libutil/suggestions.hh

@@ -1,7 +1,6 @@
 #pragma once
 ///@file
 
-#include "comparator.hh"
 #include "types.hh"
 #include <set>
 
@@ -20,7 +19,8 @@ public:
 
     std::string to_string() const;
 
-    GENERATE_CMP(Suggestion, me->distance, me->suggestion)
+    bool operator ==(const Suggestion &) const = default;
+    auto operator <=>(const Suggestion &) const = default;
 };
 
 class Suggestions {

src/libutil/url.cc

@@ -132,7 +132,7 @@ std::string ParsedURL::to_string() const
         + (fragment.empty() ? "" : "#" + percentEncode(fragment));
 }
 
-bool ParsedURL::operator ==(const ParsedURL & other) const
+bool ParsedURL::operator ==(const ParsedURL & other) const noexcept
 {
     return
         scheme == other.scheme

src/libutil/url.hh

@@ -18,7 +18,7 @@ struct ParsedURL
 
     std::string to_string() const;
 
-    bool operator ==(const ParsedURL & other) const;
+    bool operator ==(const ParsedURL & other) const noexcept;
 
     /**
      * Remove `.` and `..` path elements.