[COMMITTED] libstdc++: Make std::compare_three_way check if <=> is valid (PR 93479)

Message ID 20200129134719.GA161274@redhat.com
State New
Headers show
Series
  • [COMMITTED] libstdc++: Make std::compare_three_way check if <=> is valid (PR 93479)
Related show

Commit Message

Jonathan Wakely Jan. 29, 2020, 1:47 p.m.
Currently types that cannot be compared using <=> but which are
convertible to pointers will be compared by converting to pointers
first. They should not be comparable.

	PR libstdc++/93479
	* libsupc++/compare (__3way_builtin_ptr_cmp): Require <=> to be valid.
	* testsuite/18_support/comparisons/object/93479.cc: New test.

Tested powerpc64le-linux, committed to master.
commit 83b0201035cfdc1d4d80153f4e19ec98cf059941
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Jan 29 13:36:15 2020 +0000

    libstdc++: Make std::compare_three_way check if <=> is valid (PR 93479)
    
    Currently types that cannot be compared using <=> but which are
    convertible to pointers will be compared by converting to pointers
    first. They should not be comparable.
    
            PR libstdc++/93479
            * libsupc++/compare (__3way_builtin_ptr_cmp): Require <=> to be valid.
            * testsuite/18_support/comparisons/object/93479.cc: New test.

Comments

Jonathan Wakely Jan. 29, 2020, 2:10 p.m. | #1
On 29/01/20 13:47 +0000, Jonathan Wakely wrote:
>--- a/libstdc++-v3/libsupc++/compare

>+++ b/libstdc++-v3/libsupc++/compare

>@@ -525,7 +525,9 @@ namespace std

>     // BUILTIN-PTR-THREE-WAY(T, U)

>     template<typename _Tp, typename _Up>

>       concept __3way_builtin_ptr_cmp

>-	= convertible_to<_Tp, const volatile void*>

>+	= requires(_Tp&& __t, _Up&& __u)

>+	    { static_cast<_Tp&&>(__t) <=> static_cast<_Up&&>(__u); }


Doh. This should just use std::threeway_comparable_with, and then
subsumption makes it redundant to use this concept to constrain the
compare_threeway::operator() because we have (A || (A && B)) which is
just (A).

Tested powerpc64le-linux, committed to master.
commit f214ffb336d582a66149068a2a96b7fcf395b5de
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Jan 29 13:56:49 2020 +0000

    libstdc++: Simplify constraints on std::compare_three_way
    
    The __3way_builtin_ptr_cmp concept can use three_way_comparable_with to
    check whether <=> is valid. Doing that makes it obvious that the
    disjunction on compare_three_way::operator() is redundant, because
    the second constraint subsumes the first.
    
    The workaround for PR c++/91073 can also be removed as that bug is fixed
    now.
    
            * libsupc++/compare (__detail::__3way_builtin_ptr_cmp): Use
            three_way_comparable_with.
            (__detail::__3way_cmp_with): Remove workaround for fixed bug.
            (compare_three_way::operator()): Remove redundant constraint from
            requires-clause.
            (__detail::_Synth3way::operator()): Use three_way_comparable_with
            instead of workaround.
            * testsuite/18_support/comparisons/object/93479.cc: Prune extra
            output due to simplified constraints on compare_three_way::operator().

diff --git a/libstdc++-v3/libsupc++/compare b/libstdc++-v3/libsupc++/compare
index aabd0c56530..a7a29ef0440 100644
--- a/libstdc++-v3/libsupc++/compare
+++ b/libstdc++-v3/libsupc++/compare
@@ -525,26 +525,20 @@ namespace std
     // BUILTIN-PTR-THREE-WAY(T, U)
     template<typename _Tp, typename _Up>
       concept __3way_builtin_ptr_cmp
-	= requires(_Tp&& __t, _Up&& __u)
-	    { static_cast<_Tp&&>(__t) <=> static_cast<_Up&&>(__u); }
+	= three_way_comparable_with<_Tp, _Up>
 	  && convertible_to<_Tp, const volatile void*>
 	  && convertible_to<_Up, const volatile void*>
 	  && ! requires(_Tp&& __t, _Up&& __u)
 	     { operator<=>(static_cast<_Tp&&>(__t), static_cast<_Up&&>(__u)); }
 	  && ! requires(_Tp&& __t, _Up&& __u)
 	     { static_cast<_Tp&&>(__t).operator<=>(static_cast<_Up&&>(__u)); };
-
-    // FIXME: workaround for PR c++/91073
-    template<typename _Tp, typename _Up>
-      concept __3way_cmp_with = three_way_comparable_with<_Tp, _Up>;
   } // namespace __detail
 
   // [cmp.object], typename compare_three_way
   struct compare_three_way
   {
     template<typename _Tp, typename _Up>
-      requires (__detail::__3way_cmp_with<_Tp, _Up>
-	  || __detail::__3way_builtin_ptr_cmp<_Tp, _Up>)
+      requires three_way_comparable_with<_Tp, _Up>
       constexpr auto
       operator()(_Tp&& __t, _Up&& __u) const noexcept
       {
@@ -919,7 +913,7 @@ namespace std
 	  { __u < __t } -> convertible_to<bool>;
 	}
 	{
-	  if constexpr (__3way_cmp_with<_Tp, _Up>)
+	  if constexpr (three_way_comparable_with<_Tp, _Up>)
 	    return __t <=> __u;
 	  else
 	    {
diff --git a/libstdc++-v3/testsuite/18_support/comparisons/object/93479.cc b/libstdc++-v3/testsuite/18_support/comparisons/object/93479.cc
index 7da1df15848..f4f0a36685b 100644
--- a/libstdc++-v3/testsuite/18_support/comparisons/object/93479.cc
+++ b/libstdc++-v3/testsuite/18_support/comparisons/object/93479.cc
@@ -42,3 +42,5 @@ test02()
   std::compare_three_way{}(x, ""); // { dg-error "no match" }
   std::compare_three_way{}("", x); // { dg-error "no match" }
 }
+
+// { dg-prune-output "in requirements with" }

Patch

diff --git a/libstdc++-v3/libsupc++/compare b/libstdc++-v3/libsupc++/compare
index 117340ff184..aabd0c56530 100644
--- a/libstdc++-v3/libsupc++/compare
+++ b/libstdc++-v3/libsupc++/compare
@@ -525,7 +525,9 @@  namespace std
     // BUILTIN-PTR-THREE-WAY(T, U)
     template<typename _Tp, typename _Up>
       concept __3way_builtin_ptr_cmp
-	= convertible_to<_Tp, const volatile void*>
+	= requires(_Tp&& __t, _Up&& __u)
+	    { static_cast<_Tp&&>(__t) <=> static_cast<_Up&&>(__u); }
+	  && convertible_to<_Tp, const volatile void*>
 	  && convertible_to<_Up, const volatile void*>
 	  && ! requires(_Tp&& __t, _Up&& __u)
 	     { operator<=>(static_cast<_Tp&&>(__t), static_cast<_Up&&>(__u)); }
diff --git a/libstdc++-v3/testsuite/18_support/comparisons/object/93479.cc b/libstdc++-v3/testsuite/18_support/comparisons/object/93479.cc
new file mode 100644
index 00000000000..7da1df15848
--- /dev/null
+++ b/libstdc++-v3/testsuite/18_support/comparisons/object/93479.cc
@@ -0,0 +1,44 @@ 
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+#include <compare>
+
+void
+test01()
+{
+  // PR libstdc++/93479
+  int i[1];
+  int j[1];
+  std::compare_three_way{}(i, j); // { dg-error "no match" }
+}
+
+void
+test02()
+{
+  struct X
+  {
+    operator const char*() const noexcept { return "!"; }
+    void operator<=>(X) const = delete;
+    void operator<=>(const void*) const = delete;
+  } x;
+  std::compare_three_way{}(x, x); // { dg-error "no match" }
+  std::compare_three_way{}(x, ""); // { dg-error "no match" }
+  std::compare_three_way{}("", x); // { dg-error "no match" }
+}