Fix `With{Equal,Compare}` in C++17 for corner cases:
1. Heterogeneous secondary comparisons (`!=`, `<`, `>`, `<=`, and `>=`)
are not defined if the other type is also implemented using `WithEqual`
or `WithCompare`.
2. Heterogeneous primary comparisons with swapped parameters (`==` and
`RIEGELI_COMPARE`) are added only for types explicitly specified by
additional template parameters of `WithEqual` and `WithCompare`.
This avoids ambiguous overloads and cyclic template instantiations:
1. If both parameter types consider swapping the parameters, but the types are
not comparable, then each definition is rewritten from the other one, and
their SFINAE constraints depend on each other.
This does not happen only if the code tries to compare incomparable values.
Due to how ADL works, a call to an unqualified function name with an argument
of type `A<T>` brings not only functions related to `A`, but also functions
related to `T`. The argument of type `A<T>` will not match the parameter
of type `T`, but the function template is instantiated earlier, trying to
compare `T` against `A<T>`.
2. Automatic parameter swapping could only generate a constrained template
rather than overloads with concrete parameter types.
That converts parameters in the callee rather than in the caller, which leads
to different overload resolution.
That would create ambiguities in cases like `CompactString == Chain`.
The parameters are not swapped because both types use `WithEqual`.
Intended `Chain == absl::string_view` is not found because it requires
parameter swapping after all, since the converted parameter does not use
`WithEqual`.
PiperOrigin-RevId: 900626664
diff --git a/riegeli/base/any.h b/riegeli/base/any.h
index 43dd57e..81c397b 100644
--- a/riegeli/base/any.h
+++ b/riegeli/base/any.h
@@ -51,7 +51,8 @@
// Common base class of `Any` and `AnyRef`.
template <typename Handle, size_t inline_size, size_t inline_align>
-class AnyBase : public WithEqual<AnyBase<Handle, inline_size, inline_align>> {
+class AnyBase : public WithEqual<AnyBase<Handle, inline_size, inline_align>,
+ std::nullptr_t> {
public:
// Returns a `Handle` to the `Manager`, or a default `Handle` for an empty
// `AnyBase`.
diff --git a/riegeli/base/c_string_ref.h b/riegeli/base/c_string_ref.h
index 8ce4564..7d5a60c 100644
--- a/riegeli/base/c_string_ref.h
+++ b/riegeli/base/c_string_ref.h
@@ -52,7 +52,8 @@
// object passed as a default argument to the constructor.
//
// `CStringRef` does not own string contents and is efficiently copyable.
-class ABSL_NULLABILITY_COMPATIBLE CStringRef : public WithEqual<CStringRef> {
+class ABSL_NULLABILITY_COMPATIBLE CStringRef
+ : public WithEqual<CStringRef, std::nullptr_t> {
private:
template <typename T, typename Enable = void>
struct HasCStr : std::false_type {};
diff --git a/riegeli/base/chain_base.h b/riegeli/base/chain_base.h
index ffcbcd2..e957426 100644
--- a/riegeli/base/chain_base.h
+++ b/riegeli/base/chain_base.h
@@ -62,7 +62,7 @@
//
// A `Chain` is implemented with a sequence of blocks holding flat data
// fragments.
-class Chain : public WithCompare<Chain> {
+class Chain : public WithCompare<Chain, absl::string_view> {
private:
class RawBlock;
diff --git a/riegeli/base/compact_string.h b/riegeli/base/compact_string.h
index 66542c8..a212c97 100644
--- a/riegeli/base/compact_string.h
+++ b/riegeli/base/compact_string.h
@@ -74,7 +74,7 @@
// For sizes up to 255 this is less than libc++ `std::string` by about 15, and
// less than libstdc++ `std::string` by about 23.
class ABSL_ATTRIBUTE_TRIVIAL_ABI CompactString
- : public WithCompare<CompactString> {
+ : public WithCompare<CompactString, absl::string_view> {
public:
static constexpr size_t max_size() {
return std::numeric_limits<size_t>::max() - 2 * sizeof(size_t);
diff --git a/riegeli/base/compare.h b/riegeli/base/compare.h
index 52eb456..a23e9e2 100644
--- a/riegeli/base/compare.h
+++ b/riegeli/base/compare.h
@@ -309,122 +309,223 @@
return ordering;
}
-// For types which support equality, derive `T` from `WithEqual<T>`, and define
-// `friend bool operator==` with the first parameter of type `const T&` or `T`,
-// and the second parameter of the same type, or possibly also of other types.
-//
-// `WithEqual` provides `!=`. For heterogeneous equality it provides `==` and
-// `!=` with swapped parameters.
-//
-// In C++20 this is automatic.
+#if !__cpp_impl_three_way_comparison
+
+namespace compare_internal {
+
template <typename T>
-class WithEqual {
+class WithEqualMarker {};
+
+template <typename T>
+class WithCompareMarker {};
+
+template <typename T, typename Other>
+class WithSwappedEqual {
+ friend bool operator==(const Other& a, const T& b) { return b == a; }
+};
+
+template <typename T, typename Other>
+class WithSwappedCompare {
+ friend auto RIEGELI_COMPARE(const Other& a, const T& b) {
+ return NegateOrdering(RIEGELI_COMPARE(b, a));
+ }
+ friend bool operator<(const Other& a, const T& b) {
+ return RIEGELI_COMPARE(b, a) > 0;
+ }
+ friend bool operator>(const Other& a, const T& b) {
+ return RIEGELI_COMPARE(b, a) < 0;
+ }
+ friend bool operator<=(const Other& a, const T& b) {
+ return RIEGELI_COMPARE(b, a) >= 0;
+ }
+ friend bool operator>=(const Other& a, const T& b) {
+ return RIEGELI_COMPARE(b, a) <= 0;
+ }
+};
+
+} // namespace compare_internal
+
+#endif
+
+// In C++17, `WithEqual` emulates C++20 rules of rewriting `!=` from `==`,
+// and swapping parameters in heterogeneous `==` and `!=`. Since C++20 it has
+// no effect.
+//
+// Derive `T` from `WithEqual<T>`. If `T` supports homogeneous comparison,
+// define `friend bool operator==` with the first parameter of type `const T&`
+// or `T` and the second parameter of the same type.
+//
+// If `T` supports heterogeneous comparisons against other types, use
+// `WithEqual<T, Other...>` instead, specifying the types of these parameters.
+// For each of these types, define the appropriate `==` with the first parameter
+// of type `const T&` or `T`.
+//
+// If the other parameter does not have a concrete type because the `==` is a
+// template, do not add a template parameter of `WithEqual`. Instead, define
+// also `==` with swapped parameters, wrapped in
+// `#if !__cpp_impl_three_way_comparison`.
+template <typename T, typename... Others>
+class WithEqual
+#if !__cpp_impl_three_way_comparison
+ : public compare_internal::WithEqualMarker<T>,
+ public compare_internal::WithSwappedEqual<T, Others>...
+#endif
+{
public:
#if !__cpp_impl_three_way_comparison
template <
- typename Other,
- std::enable_if_t<compare_internal::HasEqual<T, Other>::value, int> = 0>
- friend bool operator!=(const T& a, const Other& b) {
+ typename DependentT = T,
+ std::enable_if_t<
+ compare_internal::HasEqual<DependentT, DependentT>::value, int> = 0>
+ friend bool operator!=(const T& a, const T& b) {
return !(a == b);
}
template <
typename Other,
- std::enable_if_t<std::conjunction_v<std::negation<std::is_same<Other, T>>,
+ std::enable_if_t<std::conjunction_v<std::negation<std::is_same<T, Other>>,
compare_internal::HasEqual<T, Other>>,
int> = 0>
- friend bool operator==(const Other& a, const T& b) {
- return b == a;
+ friend bool operator!=(const T& a, const Other& b) {
+ return !(a == b);
}
- template <
- typename Other,
- std::enable_if_t<std::conjunction_v<std::negation<std::is_same<Other, T>>,
- compare_internal::HasEqual<T, Other>>,
- int> = 0>
+
+ template <typename Other,
+ std::enable_if_t<
+ std::conjunction_v<
+ std::negation<std::is_same<Other, T>>,
+ std::negation<std::is_base_of<
+ compare_internal::WithEqualMarker<Other>, Other>>,
+ compare_internal::HasEqual<Other, T>>,
+ int> = 0>
friend bool operator!=(const Other& a, const T& b) {
- return !(b == a);
+ return !(a == b);
}
#endif
};
-// For types which support comparison, derive `T` from `WithCompare<T>`. and
-// define `friend bool operator==` and `friend auto RIEGELI_COMPARE` with the
-// first parameter of type `const T&` or `T`, and the second parameter of the
-// same type, or possibly also of other types.
+// In C++17, `WithCompare` emulates C++20 rules of rewriting `<`, `>`, `<=`,
+// and `>=` from `RIEGELI_COMPARE`, and swapping parameters in heterogeneous
+// `==`, `!=`, `RIEGELI_COMPARE`, `<`, `>`, `<=`, and `>=`. Since C++20 it has
+// no effect.
//
-// `WithCompare` provides `!=`, `<`, `>`, `<=`, and `>=`. For heterogeneous
-// comparison it provides `==`, `!=`, `RIEGELI_COMPARE, `<`, `>`, `<=`, and `>=`
-// with swapped parameters.
-//
-// In C++20 this is automatic.
-template <typename T>
-class WithCompare : public WithEqual<T> {
+// `WithCompare` extends `WithEqual` and is used analogously to `WithEqual`.
+// Define `friend bool operator==` and `friend auto RIEGELI_COMPARE`.
+template <typename T, typename... Others>
+class WithCompare : public WithEqual<T, Others...>
+#if !__cpp_impl_three_way_comparison
+ ,
+ public compare_internal::WithCompareMarker<T>,
+ public compare_internal::WithSwappedCompare<T, Others>...
+#endif
+{
public:
#if !__cpp_impl_three_way_comparison
template <
- typename Other,
- std::enable_if_t<compare_internal::HasCompare<T, Other>::value, int> = 0>
- friend bool operator<(const T& a, const Other& b) {
+ typename DependentT = T,
+ std::enable_if_t<
+ compare_internal::HasCompare<DependentT, DependentT>::value, int> = 0>
+ friend bool operator<(const T& a, const T& b) {
return RIEGELI_COMPARE(a, b) < 0;
}
template <
- typename Other,
- std::enable_if_t<compare_internal::HasCompare<T, Other>::value, int> = 0>
- friend bool operator>(const T& a, const Other& b) {
+ typename DependentT = T,
+ std::enable_if_t<
+ compare_internal::HasCompare<DependentT, DependentT>::value, int> = 0>
+ friend bool operator>(const T& a, const T& b) {
return RIEGELI_COMPARE(a, b) > 0;
}
template <
- typename Other,
- std::enable_if_t<compare_internal::HasCompare<T, Other>::value, int> = 0>
- friend bool operator<=(const T& a, const Other& b) {
+ typename DependentT = T,
+ std::enable_if_t<
+ compare_internal::HasCompare<DependentT, DependentT>::value, int> = 0>
+ friend bool operator<=(const T& a, const T& b) {
return RIEGELI_COMPARE(a, b) <= 0;
}
template <
- typename Other,
- std::enable_if_t<compare_internal::HasCompare<T, Other>::value, int> = 0>
+ typename DependentT = T,
+ std::enable_if_t<
+ compare_internal::HasCompare<DependentT, DependentT>::value, int> = 0>
+ friend bool operator>=(const T& a, const T& b) {
+ return RIEGELI_COMPARE(a, b) >= 0;
+ }
+
+ template <typename Other,
+ std::enable_if_t<
+ std::conjunction_v<std::negation<std::is_same<T, Other>>,
+ compare_internal::HasCompare<T, Other>>,
+ int> = 0>
+ friend bool operator<(const T& a, const Other& b) {
+ return RIEGELI_COMPARE(a, b) < 0;
+ }
+ template <typename Other,
+ std::enable_if_t<
+ std::conjunction_v<std::negation<std::is_same<T, Other>>,
+ compare_internal::HasCompare<T, Other>>,
+ int> = 0>
+ friend bool operator>(const T& a, const Other& b) {
+ return RIEGELI_COMPARE(a, b) > 0;
+ }
+ template <typename Other,
+ std::enable_if_t<
+ std::conjunction_v<std::negation<std::is_same<T, Other>>,
+ compare_internal::HasCompare<T, Other>>,
+ int> = 0>
+ friend bool operator<=(const T& a, const Other& b) {
+ return RIEGELI_COMPARE(a, b) <= 0;
+ }
+ template <typename Other,
+ std::enable_if_t<
+ std::conjunction_v<std::negation<std::is_same<T, Other>>,
+ compare_internal::HasCompare<T, Other>>,
+ int> = 0>
friend bool operator>=(const T& a, const Other& b) {
return RIEGELI_COMPARE(a, b) >= 0;
}
template <typename Other,
std::enable_if_t<
- std::conjunction_v<std::negation<std::is_same<Other, T>>,
- compare_internal::HasCompare<T, Other>>,
- int> = 0>
- friend auto RIEGELI_COMPARE(const Other& a, const T& b) {
- return NegateOrdering(RIEGELI_COMPARE(b, a));
- }
- template <typename Other,
- std::enable_if_t<
- std::conjunction_v<std::negation<std::is_same<Other, T>>,
- compare_internal::HasCompare<T, Other>>,
+ std::conjunction_v<
+ std::negation<std::is_same<Other, T>>,
+ std::negation<std::is_base_of<
+ compare_internal::WithCompareMarker<Other>, Other>>,
+ compare_internal::HasCompare<Other, T>>,
int> = 0>
friend bool operator<(const Other& a, const T& b) {
- return 0 < RIEGELI_COMPARE(b, a);
+ return RIEGELI_COMPARE(a, b) < 0;
}
template <typename Other,
std::enable_if_t<
- std::conjunction_v<std::negation<std::is_same<Other, T>>,
- compare_internal::HasCompare<T, Other>>,
+ std::conjunction_v<
+ std::negation<std::is_same<Other, T>>,
+ std::negation<std::is_base_of<
+ compare_internal::WithCompareMarker<Other>, Other>>,
+ compare_internal::HasCompare<Other, T>>,
int> = 0>
friend bool operator>(const Other& a, const T& b) {
- return 0 > RIEGELI_COMPARE(b, a);
+ return RIEGELI_COMPARE(a, b) > 0;
}
template <typename Other,
std::enable_if_t<
- std::conjunction_v<std::negation<std::is_same<Other, T>>,
- compare_internal::HasCompare<T, Other>>,
+ std::conjunction_v<
+ std::negation<std::is_same<Other, T>>,
+ std::negation<std::is_base_of<
+ compare_internal::WithCompareMarker<Other>, Other>>,
+ compare_internal::HasCompare<Other, T>>,
int> = 0>
friend bool operator<=(const Other& a, const T& b) {
- return 0 <= RIEGELI_COMPARE(b, a);
+ return RIEGELI_COMPARE(a, b) <= 0;
}
template <typename Other,
std::enable_if_t<
- std::conjunction_v<std::negation<std::is_same<Other, T>>,
- compare_internal::HasCompare<T, Other>>,
+ std::conjunction_v<
+ std::negation<std::is_same<Other, T>>,
+ std::negation<std::is_base_of<
+ compare_internal::WithCompareMarker<Other>, Other>>,
+ compare_internal::HasCompare<Other, T>>,
int> = 0>
friend bool operator>=(const Other& a, const T& b) {
- return 0 >= RIEGELI_COMPARE(b, a);
+ return RIEGELI_COMPARE(a, b) >= 0;
}
#endif
};
diff --git a/riegeli/base/dependency.h b/riegeli/base/dependency.h
index 9ecb4aa..23b0008 100644
--- a/riegeli/base/dependency.h
+++ b/riegeli/base/dependency.h
@@ -670,7 +670,8 @@
template <typename Base, typename Handle, typename Manager>
class DependencyDerived
: public Base,
- public WithEqual<DependencyDerived<Base, Handle, Manager>> {
+ public WithEqual<DependencyDerived<Base, Handle, Manager>,
+ std::nullptr_t> {
public:
using Base::Base;
diff --git a/riegeli/base/hybrid_direct_internal.h b/riegeli/base/hybrid_direct_internal.h
index be5781a..85f0bea 100644
--- a/riegeli/base/hybrid_direct_internal.h
+++ b/riegeli/base/hybrid_direct_internal.h
@@ -252,7 +252,7 @@
return *this;
}
- friend bool operator==(const IndexIterator& a, const IndexIterator& b) {
+ friend bool operator==(IndexIterator a, IndexIterator b) {
return a.index_ == b.index_;
}
diff --git a/riegeli/base/intrusive_shared_ptr.h b/riegeli/base/intrusive_shared_ptr.h
index 7e339b4..6c21793 100644
--- a/riegeli/base/intrusive_shared_ptr.h
+++ b/riegeli/base/intrusive_shared_ptr.h
@@ -93,7 +93,7 @@
// class. Prefer `SharedPtr` unless `IntrusiveSharedPtr` is needed.
template <typename T>
class ABSL_ATTRIBUTE_TRIVIAL_ABI ABSL_NULLABILITY_COMPATIBLE IntrusiveSharedPtr
- : public WithEqual<IntrusiveSharedPtr<T>> {
+ : public WithEqual<IntrusiveSharedPtr<T>, std::nullptr_t> {
public:
// Creates an empty `IntrusiveSharedPtr`.
constexpr IntrusiveSharedPtr() = default;
diff --git a/riegeli/base/optional_compact_string.h b/riegeli/base/optional_compact_string.h
index 506df7f..02adc36 100644
--- a/riegeli/base/optional_compact_string.h
+++ b/riegeli/base/optional_compact_string.h
@@ -37,7 +37,8 @@
// `CompactString`. It allows examining the contents as `absl::string_view` or
// `const char*`, but not as `CompactString`, except by copying or moving from.
class ABSL_ATTRIBUTE_TRIVIAL_ABI OptionalCompactString
- : public WithCompare<OptionalCompactString> {
+ : public WithCompare<OptionalCompactString, std::nullptr_t,
+ absl::string_view> {
public:
// Creates a null `OptionalCompactString`.
OptionalCompactString() = default;
diff --git a/riegeli/base/shared_ptr.h b/riegeli/base/shared_ptr.h
index b7c0ac7..0fbc791 100644
--- a/riegeli/base/shared_ptr.h
+++ b/riegeli/base/shared_ptr.h
@@ -58,7 +58,7 @@
// class. Prefer `SharedPtr` unless `IntrusiveSharedPtr` is needed.
template <typename T>
class ABSL_ATTRIBUTE_TRIVIAL_ABI ABSL_NULLABILITY_COMPATIBLE SharedPtr
- : public WithEqual<SharedPtr<T>> {
+ : public WithEqual<SharedPtr<T>, std::nullptr_t> {
private:
template <typename SubT>
struct IsCompatibleProperSubtype
diff --git a/riegeli/bytes/cfile_handle.h b/riegeli/bytes/cfile_handle.h
index 910b3e6..71999c0 100644
--- a/riegeli/bytes/cfile_handle.h
+++ b/riegeli/bytes/cfile_handle.h
@@ -109,7 +109,7 @@
// // Optional. If absent, `absl::OkStatus()` is assumed.
// absl::Status Close();
// ```
-class CFileHandle : public WithEqual<CFileHandle> {
+class CFileHandle : public WithEqual<CFileHandle, FILE*, std::nullptr_t> {
public:
// Creates a `CFileHandle` which does not refer to a target.
CFileHandle() = default;
@@ -507,7 +507,7 @@
// The `FILE*` can be `nullptr` which means absent.
class UnownedCFile
: public cfile_internal::CFileBase<cfile_internal::UnownedCFileDeleter>,
- public WithEqual<UnownedCFile> {
+ public WithEqual<UnownedCFile, FILE*> {
public:
using CFileBase::CFileBase;
@@ -544,7 +544,7 @@
// The `FILE*` can be `nullptr` which means absent.
class OwnedCFile
: public cfile_internal::CFileBase<cfile_internal::OwnedCFileDeleter>,
- public WithEqual<OwnedCFile> {
+ public WithEqual<OwnedCFile, FILE*> {
public:
using CFileBase::CFileBase;
diff --git a/riegeli/bytes/fd_handle.h b/riegeli/bytes/fd_handle.h
index dab232a..a8fbf68 100644
--- a/riegeli/bytes/fd_handle.h
+++ b/riegeli/bytes/fd_handle.h
@@ -145,7 +145,7 @@
// // Optional. If absent, `absl::OkStatus()` is assumed.
// absl::Status Close();
// ```
-class FdHandle : public WithEqual<FdHandle> {
+class FdHandle : public WithEqual<FdHandle, int, std::nullptr_t> {
public:
// Creates an `FdHandle` which does not refer to a target.
FdHandle() = default;
@@ -543,7 +543,7 @@
//
// The fd can be negative which means absent.
class UnownedFd : public fd_internal::FdBase<fd_internal::UnownedFdDeleter>,
- public WithEqual<UnownedFd> {
+ public WithEqual<UnownedFd, int, std::nullptr_t> {
public:
using FdBase::FdBase;
@@ -578,7 +578,7 @@
//
// The fd can be negative which means absent.
class OwnedFd : public fd_internal::FdBase<fd_internal::OwnedFdDeleter>,
- public WithEqual<OwnedFd> {
+ public WithEqual<OwnedFd, int, std::nullptr_t> {
public:
using Permissions = fd_internal::Permissions;
#ifndef _WIN32
diff --git a/riegeli/containers/linear_sorted_string_set.h b/riegeli/containers/linear_sorted_string_set.h
index 0b68f1f..fe8715b 100644
--- a/riegeli/containers/linear_sorted_string_set.h
+++ b/riegeli/containers/linear_sorted_string_set.h
@@ -383,7 +383,8 @@
//
// The prefix is known to be shared with the previous element. It is not
// guaranteed to be the longest shared prefix though.
-class LinearSortedStringSet::SplitElement : public WithCompare<SplitElement> {
+class LinearSortedStringSet::SplitElement
+ : public WithCompare<SplitElement, absl::string_view> {
public:
explicit SplitElement(absl::string_view prefix, absl::string_view suffix)
: prefix_(prefix), suffix_(suffix) {}
diff --git a/riegeli/digests/digester_handle.h b/riegeli/digests/digester_handle.h
index bb9e9b9..91763a2 100644
--- a/riegeli/digests/digester_handle.h
+++ b/riegeli/digests/digester_handle.h
@@ -124,7 +124,8 @@
//
// For digesting many small values it is better to use `DigestingWriter` which
// adds a buffering layer.
-class DigesterBaseHandle : public WithEqual<DigesterBaseHandle> {
+class DigesterBaseHandle
+ : public WithEqual<DigesterBaseHandle, std::nullptr_t> {
public:
// Creates a `DigesterBaseHandle` which does not refer to a target.
DigesterBaseHandle() = default;
