From 475e0b9e4c89b7882f1133d1f250159126fa1652 Mon Sep 17 00:00:00 2001
From: Dominik Charousset <dominik@charousset.de>
Date: Sun, 29 Sep 2024 17:16:52 +0200
Subject: [PATCH] Remove obsolete operators.hh utility

The utility classes in `operators.hh` overlap with
`broker::detail::comparable`. There is no need to have both in the code
base, so we remove the `operators.hh` header and instead use
`comparable` consistently.
---
 bindings/python/data.cpp              |  1 -
 libbroker/broker/detail/comparable.hh |  6 +++
 libbroker/broker/detail/operators.hh  | 31 --------------
 libbroker/broker/enum_value.hh        | 58 +++++++--------------------
 libbroker/broker/network_info.cc      | 12 +++---
 libbroker/broker/network_info.hh      | 12 ++----
 libbroker/broker/port.cc              | 16 ++++----
 libbroker/broker/port.hh              | 15 ++-----
 libbroker/broker/status.hh            | 17 ++++++--
 libbroker/broker/subnet.cc            | 16 ++++----
 libbroker/broker/subnet.hh            | 13 ++----
 libbroker/broker/topic.cc             |  8 ----
 libbroker/broker/topic.hh             | 18 ++-------
 13 files changed, 71 insertions(+), 152 deletions(-)
 delete mode 100644 libbroker/broker/detail/operators.hh

diff --git a/bindings/python/data.cpp b/bindings/python/data.cpp
index 1854c02ca..0e272d62e 100644
--- a/bindings/python/data.cpp
+++ b/bindings/python/data.cpp
@@ -17,7 +17,6 @@
 #include "broker/convert.hh"
 #include "broker/data.hh"
 #include "broker/detail/assert.hh"
-#include "broker/detail/operators.hh"
 
 namespace py = pybind11;
 using namespace pybind11::literals;
diff --git a/libbroker/broker/detail/comparable.hh b/libbroker/broker/detail/comparable.hh
index 289f76200..23993fb38 100644
--- a/libbroker/broker/detail/comparable.hh
+++ b/libbroker/broker/detail/comparable.hh
@@ -5,6 +5,9 @@ namespace broker::detail {
 /// Barton–Nackman trick implementation.
 template <class Derived, class T = Derived>
 class comparable {
+public:
+  friend Derived;
+
   friend bool operator==(const Derived& lhs, const T& rhs) noexcept {
     return lhs.compare(rhs) == 0;
   }
@@ -52,6 +55,9 @@ class comparable {
   friend bool operator>=(const T& lhs, const Derived& rhs) noexcept {
     return rhs <= lhs;
   }
+
+private:
+  comparable() = default;
 };
 
 template <class Derived>
diff --git a/libbroker/broker/detail/operators.hh b/libbroker/broker/detail/operators.hh
deleted file mode 100644
index 2b23a5dcf..000000000
--- a/libbroker/broker/detail/operators.hh
+++ /dev/null
@@ -1,31 +0,0 @@
-#pragma once
-
-namespace broker::detail {
-
-template <class T, class U = T>
-struct equality_comparable {
-  friend bool operator!=(const T& x, const U& y) {
-    return !(x == y);
-  }
-};
-
-template <class T, class U = T>
-struct less_than_comparable {
-  friend bool operator>(const T& x, const U& y) {
-    return y < x;
-  }
-
-  friend bool operator<=(const T& x, const U& y) {
-    return !(y < x);
-  }
-
-  friend bool operator>=(const T& x, const U& y) {
-    return !(x < y);
-  }
-};
-
-template <class T, class U = T>
-struct totally_ordered : equality_comparable<T, U>,
-                         less_than_comparable<T, U> {};
-
-} // namespace broker::detail
diff --git a/libbroker/broker/enum_value.hh b/libbroker/broker/enum_value.hh
index 998d8d98b..4287c9a9c 100644
--- a/libbroker/broker/enum_value.hh
+++ b/libbroker/broker/enum_value.hh
@@ -5,13 +5,13 @@
 #include <string>
 #include <string_view>
 
-#include "broker/detail/operators.hh"
+#include "broker/detail/comparable.hh"
 
 namespace broker {
 
 /// Stores the name of an enum value.  The receiver is responsible for knowing
 /// how to map the name to the actual value if it needs that information.
-struct enum_value : detail::totally_ordered<enum_value> {
+struct enum_value : detail::comparable<enum_value> {
   /// Default construct empty enum value name.
   enum_value() = default;
 
@@ -21,17 +21,11 @@ struct enum_value : detail::totally_ordered<enum_value> {
   }
 
   std::string name;
-};
-
-/// @relates enum_value
-inline bool operator==(const enum_value& lhs, const enum_value& rhs) {
-  return lhs.name == rhs.name;
-}
 
-/// @relates enum_value
-inline bool operator<(const enum_value& lhs, const enum_value& rhs) {
-  return lhs.name < rhs.name;
-}
+  inline auto compare(const enum_value& other) const {
+    return name.compare(other.name);
+  }
+};
 
 /// @relates enum_value
 template <class Inspector>
@@ -45,8 +39,8 @@ inline void convert(const enum_value& e, std::string& str) {
 }
 
 /// Like enum_value, but wraps a value of type `std::string_view` instead.
-class enum_value_view : detail::totally_ordered<enum_value_view>,
-                        detail::totally_ordered<enum_value_view, enum_value> {
+class enum_value_view : detail::comparable<enum_value_view>,
+                        detail::comparable<enum_value_view, enum_value> {
 public:
   /// Default construct empty enum value name.
   enum_value_view() = default;
@@ -57,37 +51,15 @@ public:
   }
 
   std::string_view name;
-};
-
-/// @relates enum_value
-inline bool operator==(const enum_value_view& lhs, const enum_value_view& rhs) {
-  return lhs.name == rhs.name;
-}
 
-/// @relates enum_value_view
-inline bool operator<(const enum_value_view& lhs, const enum_value_view& rhs) {
-  return lhs.name < rhs.name;
-}
-
-/// @relates enum_value
-inline bool operator==(const enum_value_view& lhs, const enum_value& rhs) {
-  return lhs.name == rhs.name;
-}
-
-/// @relates enum_value_view
-inline bool operator<(const enum_value_view& lhs, const enum_value& rhs) {
-  return lhs.name < rhs.name;
-}
-
-/// @relates enum_value
-inline bool operator==(const enum_value& lhs, const enum_value_view& rhs) {
-  return lhs.name == rhs.name;
-}
+  inline auto compare(const enum_value& other) const {
+    return name.compare(other.name);
+  }
 
-/// @relates enum_value_view
-inline bool operator<(const enum_value& lhs, const enum_value_view& rhs) {
-  return lhs.name < rhs.name;
-}
+  inline auto compare(const enum_value_view& other) const {
+    return name.compare(other.name);
+  }
+};
 
 } // namespace broker
 
diff --git a/libbroker/broker/network_info.cc b/libbroker/broker/network_info.cc
index f26d81073..4e0c840a7 100644
--- a/libbroker/broker/network_info.cc
+++ b/libbroker/broker/network_info.cc
@@ -15,12 +15,12 @@ network_info::network_info(std::string addr, uint16_t port,
                            timeout::seconds retry)
   : address{std::move(addr)}, port{port}, retry{retry} {}
 
-bool operator==(const network_info& x, const network_info& y) {
-  return x.address == y.address && x.port == y.port;
-}
-
-bool operator<(const network_info& x, const network_info& y) {
-  return std::tie(x.address, x.port) < std::tie(y.address, y.port);
+int network_info::compare(const network_info& other) const {
+  auto res = address.compare(other.address);
+  if (res == 0) {
+    return static_cast<int>(port) - static_cast<int>(other.port);
+  }
+  return res;
 }
 
 std::string to_string(const network_info& x) {
diff --git a/libbroker/broker/network_info.hh b/libbroker/broker/network_info.hh
index 01a858196..9ecaa9f93 100644
--- a/libbroker/broker/network_info.hh
+++ b/libbroker/broker/network_info.hh
@@ -4,13 +4,13 @@
 #include <optional>
 #include <string>
 
-#include "broker/detail/operators.hh"
+#include "broker/detail/comparable.hh"
 #include "broker/timeout.hh"
 
 namespace broker {
 
 /// Represents an IP address and TCP port combination.
-struct network_info : detail::totally_ordered<network_info> {
+struct network_info : detail::comparable<network_info> {
   network_info() = default;
   network_info(std::string addr, uint16_t port,
                timeout::seconds retry = timeout::seconds());
@@ -21,13 +21,9 @@ struct network_info : detail::totally_ordered<network_info> {
   bool has_retry_time() const noexcept {
     return retry.count() != 0;
   }
-};
-
-/// @relates network_info
-bool operator==(const network_info& x, const network_info& y);
 
-/// @relates network_info
-bool operator<(const network_info& x, const network_info& y);
+  int compare(const network_info& other) const;
+};
 
 /// @relates network_info
 template <class Inspector>
diff --git a/libbroker/broker/port.cc b/libbroker/broker/port.cc
index bb3ae9fec..09253f26a 100644
--- a/libbroker/broker/port.cc
+++ b/libbroker/broker/port.cc
@@ -15,6 +15,14 @@ port::port() : num_{0}, proto_{protocol::unknown} {}
 
 port::port(number_type n, protocol p) : num_{n}, proto_{p} {}
 
+int port::compare(const port& other) const {
+  auto res = static_cast<int>(num_) - static_cast<int>(other.num_);
+  if (res == 0) {
+    return static_cast<int>(proto_) - static_cast<int>(other.proto_);
+  }
+  return res;
+}
+
 port::number_type port::number() const {
   return num_;
 }
@@ -23,14 +31,6 @@ port::protocol port::type() const {
   return proto_;
 }
 
-bool operator==(const port& lhs, const port& rhs) {
-  return lhs.proto_ == rhs.proto_ && lhs.num_ == rhs.num_;
-}
-
-bool operator<(const port& lhs, const port& rhs) {
-  return std::tie(lhs.num_, lhs.proto_) < std::tie(rhs.num_, rhs.proto_);
-}
-
 void convert(const port& p, std::string& str) {
   str = std::to_string(p.number());
   str += '/';
diff --git a/libbroker/broker/port.hh b/libbroker/broker/port.hh
index 69a597027..778c85521 100644
--- a/libbroker/broker/port.hh
+++ b/libbroker/broker/port.hh
@@ -3,7 +3,7 @@
 #include <cstdint>
 #include <string>
 
-#include "broker/detail/operators.hh"
+#include "broker/detail/comparable.hh"
 #include "broker/fwd.hh"
 
 namespace broker {
@@ -15,7 +15,7 @@ void convert(const port& p, std::string& str);
 bool convert(const std::string& str, port& p);
 
 /// A transport-layer port.
-class port : detail::totally_ordered<port> {
+class port : detail::comparable<port> {
 public:
   using number_type = uint16_t;
 
@@ -32,6 +32,8 @@ public:
   /// Construct a port from number/protocol.
   port(number_type num, protocol p);
 
+  int compare(const port& other) const;
+
   /// @return The port number.
   number_type number() const;
 
@@ -40,9 +42,6 @@ public:
 
   size_t hash() const;
 
-  friend bool operator==(const port& lhs, const port& rhs);
-  friend bool operator<(const port& lhs, const port& rhs);
-
   template <class Inspector>
   friend bool inspect(Inspector& f, port& x) {
     if (f.has_human_readable_format()) {
@@ -78,12 +77,6 @@ bool inspect(Inspector& f, port::protocol& x) {
   return f.apply(get, set);
 }
 
-/// @relates port
-bool operator==(const port& lhs, const port& rhs);
-
-/// @relates port
-bool operator<(const port& lhs, const port& rhs);
-
 } // namespace broker
 
 namespace std {
diff --git a/libbroker/broker/status.hh b/libbroker/broker/status.hh
index 7358a969d..c5e79c273 100644
--- a/libbroker/broker/status.hh
+++ b/libbroker/broker/status.hh
@@ -1,7 +1,6 @@
 #pragma once
 
 #include "broker/convert.hh"
-#include "broker/detail/operators.hh"
 #include "broker/detail/type_traits.hh"
 #include "broker/endpoint_info.hh"
 #include "broker/error.hh"
@@ -100,9 +99,7 @@ template <class Inspector>
 bool inspect(Inspector& f, status& x);
 
 /// Diagnostic status information.
-class status : detail::equality_comparable<status, status>,
-               detail::equality_comparable<status, sc>,
-               detail::equality_comparable<sc, status> {
+class status {
 public:
   template <sc S>
   static status make(endpoint_info ei, std::string msg) {
@@ -193,6 +190,18 @@ private:
   std::string message_;
 };
 
+inline bool operator!=(const status& lhs, const status& rhs) {
+  return !(lhs == rhs);
+}
+
+inline bool operator!=(const status& lhs, sc rhs) {
+  return !(lhs == rhs);
+}
+
+inline bool operator!=(sc lhs, const status& rhs) {
+  return !(lhs == rhs);
+}
+
 /// @relates status
 std::string to_string(const status& x);
 
diff --git a/libbroker/broker/subnet.cc b/libbroker/broker/subnet.cc
index 9483cd24b..af884bcb6 100644
--- a/libbroker/broker/subnet.cc
+++ b/libbroker/broker/subnet.cc
@@ -21,6 +21,14 @@ subnet::subnet(const address& addr, uint8_t length) noexcept
   len_ = 0;
 }
 
+int subnet::compare(const subnet& other) const {
+  auto res = net_.compare(other.net_);
+  if (res == 0) {
+    return static_cast<int>(len_) - static_cast<int>(other.len_);
+  }
+  return res;
+}
+
 bool subnet::init() {
   if (net_.is_v4()) {
     if (len_ > 32)
@@ -51,14 +59,6 @@ size_t subnet::hash() const {
   return caf::hash::fnv<size_t>::compute(net_, len_);
 }
 
-bool operator==(const subnet& lhs, const subnet& rhs) {
-  return lhs.len_ == rhs.len_ && lhs.net_ == rhs.net_;
-}
-
-bool operator<(const subnet& lhs, const subnet& rhs) {
-  return std::tie(lhs.net_, lhs.len_) < std::tie(rhs.net_, lhs.len_);
-}
-
 void convert(const subnet& sn, std::string& str) {
   convert(sn.network(), str);
   str += '/';
diff --git a/libbroker/broker/subnet.hh b/libbroker/broker/subnet.hh
index 03c3c0105..a6a32f5fd 100644
--- a/libbroker/broker/subnet.hh
+++ b/libbroker/broker/subnet.hh
@@ -1,7 +1,7 @@
 #pragma once
 
 #include "broker/address.hh"
-#include "broker/detail/operators.hh"
+#include "broker/detail/comparable.hh"
 #include "broker/fwd.hh"
 
 namespace broker {
@@ -13,7 +13,7 @@ void convert(const subnet& sn, std::string& str);
 bool convert(const std::string& str, subnet& sn);
 
 /// An IPv4 or IPv6 subnet (an address prefix).
-class subnet : detail::totally_ordered<subnet> {
+class subnet : detail::comparable<subnet> {
 public:
   /// Default construct empty subnet ::/0.
   subnet() noexcept;
@@ -40,8 +40,7 @@ public:
     return len_;
   }
 
-  friend bool operator==(const subnet& lhs, const subnet& rhs);
-  friend bool operator<(const subnet& lhs, const subnet& rhs);
+  int compare(const subnet& other) const;
 
   template <class Inspector>
   friend bool inspect(Inspector& f, subnet& x) {
@@ -72,12 +71,6 @@ private:
   uint8_t len_;
 };
 
-/// @relates subnet
-bool operator==(const subnet& lhs, const subnet& rhs);
-
-/// @relates subnet
-bool operator<(const subnet& lhs, const subnet& rhs);
-
 } // namespace broker
 
 namespace std {
diff --git a/libbroker/broker/topic.cc b/libbroker/broker/topic.cc
index fe101c377..1ac1d8cdc 100644
--- a/libbroker/broker/topic.cc
+++ b/libbroker/broker/topic.cc
@@ -61,14 +61,6 @@ std::string_view topic::suffix() const noexcept {
   }
 }
 
-bool operator==(const topic& lhs, const topic& rhs) {
-  return lhs.string() == rhs.string();
-}
-
-bool operator<(const topic& lhs, const topic& rhs) {
-  return lhs.string() < rhs.string();
-}
-
 topic operator/(const topic& lhs, const topic& rhs) {
   topic result{lhs};
   return result /= rhs;
diff --git a/libbroker/broker/topic.hh b/libbroker/broker/topic.hh
index 105d3e338..23d5e44a9 100644
--- a/libbroker/broker/topic.hh
+++ b/libbroker/broker/topic.hh
@@ -7,12 +7,12 @@
 #include <utility>
 #include <vector>
 
-#include "broker/detail/operators.hh"
+#include "broker/detail/comparable.hh"
 
 namespace broker {
 
 /// A hierachical topic used as pub/sub communication pattern.
-class topic : detail::totally_ordered<topic> {
+class topic : detail::comparable<topic> {
 public:
   /// The separator between topic hierarchies.
   static constexpr char sep = '/';
@@ -103,12 +103,8 @@ public:
     return f.apply(x.str_);
   }
 
-  friend bool operator==(const topic& lhs, std::string_view rhs) {
-    return lhs.str_ == rhs;
-  }
-
-  friend bool operator==(std::string_view lhs, const topic& rhs) {
-    return lhs == rhs.str_;
+  auto compare(const topic& other) const {
+    return str_.compare(other.str_);
   }
 
 private:
@@ -124,12 +120,6 @@ inline bool is_prefix(const topic& t, std::string_view prefix) noexcept {
   return topic::is_prefix(t.string(), prefix);
 }
 
-/// @relates topic
-bool operator==(const topic& lhs, const topic& rhs);
-
-/// @relates topic
-bool operator<(const topic& lhs, const topic& rhs);
-
 /// @relates topic
 topic operator/(const topic& lhs, const topic& rhs);