Re: PR libstdc++/78420 Make std::less etc. yield total order for pointers

[Jonathan Wakely <jwakely.gcc@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]

On 14 March 2018 at 23:01, Jonathan Wakely wrote:
> Here's a very different patch. This gets rid of the __ptr_rel_ops and
> just puts the special handling for pointers directly in the
> std::less<_Tp*> etc. specializations. Then std::less<void> uses
> std::less<P*> for some pointer type P*. I've also added a ton of ugly
> metaprogramming to detect the "if the call operator calls a built-in
> operator comparing pointers" condition. The idea is borrowed from
> Casey Carter and Eric Niebler's Ranges work, but basically it checks
> if operator<(T,U) or T.operator<(U) can be called, and if not the
> comparison must be using a built-in operator. If both T and U are
> convertible to pointers (specifically, to const volatile void* which
> is the most accepting of all pointers) then we assume we're using a
> built-in operator comparing pointers, and delegate to std::less<const
> volatile void*>, which ensures a total order.
>
> Tested powerpc64le-linux, committed to trunk. This fixes a regression,
> but I'm not sure about backporting it yet, I haven't even tried
> testing it on the branches.

After testing the traits for detecting overloaded operators I made a
"small tweak" to reorganize them, and didn't retest. This fixes the
bug I introduced by reorganizing.

Tested powerpc64le-linux, committed to trunk.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 7507 bytes --]

commit 6c0e2be69e11e4ec8f9b04d3f80b9d4496737b0b
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Mar 22 13:08:52 2018 +0000

    PR libstdc++/85040 fix std::less<void> etc. ambiguities
    
            PR libstdc++/85040
            * include/bits/stl_function.h (greater::__not_overloaded)
            (less::__not_overloaded, greater_equal::__not_overloaded)
            (less_equal::__not_overloaded): Fix ambiguous specializations.
            * testsuite/20_util/function_objects/comparisons_pointer.cc: Add
            tests for type with overlaoded operators.

diff --git a/libstdc++-v3/include/bits/stl_function.h b/libstdc++-v3/include/bits/stl_function.h
index 0affaf7da3a..9e81ad3f20e 100644
--- a/libstdc++-v3/include/bits/stl_function.h
+++ b/libstdc++-v3/include/bits/stl_function.h
@@ -523,15 +523,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	      static_cast<const volatile void*>(std::forward<_Up>(__u)));
 	}
 
-      template<typename _Tp, typename _Up, typename = void>
-	struct __not_overloaded;
-
-      // False if we can call operator>(T,U)
-      template<typename _Tp, typename _Up>
-	struct __not_overloaded<_Tp, _Up, __void_t<
-	  decltype(operator>(std::declval<_Tp>(), std::declval<_Up>()))>>
-	: false_type { };
-
+      // True if there is no viable operator> member function.
       template<typename _Tp, typename _Up, typename = void>
 	struct __not_overloaded2 : true_type { };
 
@@ -541,8 +533,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  decltype(std::declval<_Tp>().operator>(std::declval<_Up>()))>>
 	: false_type { };
 
+      // True if there is no overloaded operator> for these operands.
+      template<typename _Tp, typename _Up, typename = void>
+	struct __not_overloaded : __not_overloaded2<_Tp, _Up> { };
+
+      // False if we can call operator>(T,U)
       template<typename _Tp, typename _Up>
-	struct __not_overloaded<_Tp, _Up> : __not_overloaded2<_Tp, _Up> { };
+	struct __not_overloaded<_Tp, _Up, __void_t<
+	  decltype(operator>(std::declval<_Tp>(), std::declval<_Up>()))>>
+	: false_type { };
 
       template<typename _Tp, typename _Up>
 	using __ptr_cmp = __and_<__not_overloaded<_Tp, _Up>,
@@ -586,15 +585,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	      static_cast<const volatile void*>(std::forward<_Up>(__u)));
 	}
 
-      template<typename _Tp, typename _Up, typename = void>
-	struct __not_overloaded;
-
-      // False if we can call operator<(T,U)
-      template<typename _Tp, typename _Up>
-	struct __not_overloaded<_Tp, _Up, __void_t<
-	  decltype(operator<(std::declval<_Tp>(), std::declval<_Up>()))>>
-	: false_type { };
-
+      // True if there is no viable operator< member function.
       template<typename _Tp, typename _Up, typename = void>
 	struct __not_overloaded2 : true_type { };
 
@@ -604,8 +595,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  decltype(std::declval<_Tp>().operator<(std::declval<_Up>()))>>
 	: false_type { };
 
+      // True if there is no overloaded operator< for these operands.
+      template<typename _Tp, typename _Up, typename = void>
+	struct __not_overloaded : __not_overloaded2<_Tp, _Up> { };
+
+      // False if we can call operator<(T,U)
       template<typename _Tp, typename _Up>
-	struct __not_overloaded<_Tp, _Up> : __not_overloaded2<_Tp, _Up> { };
+	struct __not_overloaded<_Tp, _Up, __void_t<
+	  decltype(operator<(std::declval<_Tp>(), std::declval<_Up>()))>>
+	: false_type { };
 
       template<typename _Tp, typename _Up>
 	using __ptr_cmp = __and_<__not_overloaded<_Tp, _Up>,
@@ -649,15 +647,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	      static_cast<const volatile void*>(std::forward<_Up>(__u)));
 	}
 
-      template<typename _Tp, typename _Up, typename = void>
-	struct __not_overloaded;
-
-      // False if we can call operator>=(T,U)
-      template<typename _Tp, typename _Up>
-	struct __not_overloaded<_Tp, _Up, __void_t<
-	  decltype(operator>=(std::declval<_Tp>(), std::declval<_Up>()))>>
-	: false_type { };
-
+      // True if there is no viable operator>= member function.
       template<typename _Tp, typename _Up, typename = void>
 	struct __not_overloaded2 : true_type { };
 
@@ -667,8 +657,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  decltype(std::declval<_Tp>().operator>=(std::declval<_Up>()))>>
 	: false_type { };
 
+      // True if there is no overloaded operator>= for these operands.
+      template<typename _Tp, typename _Up, typename = void>
+	struct __not_overloaded : __not_overloaded2<_Tp, _Up> { };
+
+      // False if we can call operator>=(T,U)
       template<typename _Tp, typename _Up>
-	struct __not_overloaded<_Tp, _Up> : __not_overloaded2<_Tp, _Up> { };
+	struct __not_overloaded<_Tp, _Up, __void_t<
+	  decltype(operator>=(std::declval<_Tp>(), std::declval<_Up>()))>>
+	: false_type { };
 
       template<typename _Tp, typename _Up>
 	using __ptr_cmp = __and_<__not_overloaded<_Tp, _Up>,
@@ -712,15 +709,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	      static_cast<const volatile void*>(std::forward<_Up>(__u)));
 	}
 
-      template<typename _Tp, typename _Up, typename = void>
-	struct __not_overloaded;
-
-      // False if we can call operator<=(T,U)
-      template<typename _Tp, typename _Up>
-	struct __not_overloaded<_Tp, _Up, __void_t<
-	  decltype(operator<=(std::declval<_Tp>(), std::declval<_Up>()))>>
-	: false_type { };
-
+      // True if there is no viable operator<= member function.
       template<typename _Tp, typename _Up, typename = void>
 	struct __not_overloaded2 : true_type { };
 
@@ -730,8 +719,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  decltype(std::declval<_Tp>().operator<=(std::declval<_Up>()))>>
 	: false_type { };
 
+      // True if there is no overloaded operator<= for these operands.
+      template<typename _Tp, typename _Up, typename = void>
+	struct __not_overloaded : __not_overloaded2<_Tp, _Up> { };
+
+      // False if we can call operator<=(T,U)
       template<typename _Tp, typename _Up>
-	struct __not_overloaded<_Tp, _Up> : __not_overloaded2<_Tp, _Up> { };
+	struct __not_overloaded<_Tp, _Up, __void_t<
+	  decltype(operator<=(std::declval<_Tp>(), std::declval<_Up>()))>>
+	: false_type { };
 
       template<typename _Tp, typename _Up>
 	using __ptr_cmp = __and_<__not_overloaded<_Tp, _Up>,
diff --git a/libstdc++-v3/testsuite/20_util/function_objects/comparisons_pointer.cc b/libstdc++-v3/testsuite/20_util/function_objects/comparisons_pointer.cc
index 474190cdf81..7cec294f8c2 100644
--- a/libstdc++-v3/testsuite/20_util/function_objects/comparisons_pointer.cc
+++ b/libstdc++-v3/testsuite/20_util/function_objects/comparisons_pointer.cc
@@ -195,6 +195,39 @@ test05()
 #endif
 }
 
+struct Overloaded {
+  bool operator>(int) { return true; }
+  bool operator<(int) { return false; }
+  bool operator>=(int) { return true; }
+  bool operator<=(int) { return false; }
+};
+bool operator>(Overloaded, Overloaded) { return false; }
+bool operator<(Overloaded, Overloaded) { return false; }
+bool operator>=(Overloaded, Overloaded) { return true; }
+bool operator<=(Overloaded, Overloaded) { return true; }
+
+void
+test06()
+{
+#if __cplusplus >= 201402L
+  std::greater<void> gt;
+  std::less<void> lt;
+  std::greater_equal<void> ge;
+  std::less_equal<void> le;
+
+  Overloaded o;
+  VERIFY( !gt(o, o) );
+  VERIFY( !lt(o, o) );
+  VERIFY( ge(o, o) );
+  VERIFY( le(o, o) );
+
+  VERIFY( gt(o, 1) );
+  VERIFY( !lt(o, 1) );
+  VERIFY( ge(o, 1) );
+  VERIFY( !le(o, 1) );
+#endif
+}
+
 int
 main()
 {
@@ -203,4 +236,5 @@ main()
   test03();
   test04();
   test05();
+  test06();
 }