public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* PR libstdc++/78420 Make std::less etc. yield total order for pointers
@ 2018-03-09  0:41 Jonathan Wakely
  2018-03-13 16:30 ` Jonathan Wakely
  0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Wakely @ 2018-03-09  0:41 UTC (permalink / raw)
  To: libstdc++, gcc-patches; +Cc: Jason Merrill, Jakub Jelinek

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

In order to meet the total order requirements of [comparisons] p2 we
need to cast unrelated pointers to uintptr_t before comparing them.
Those casts aren't allowed in constant expressions, so only cast
when __builtin_constant_p says the result of the comparison is not a
compile-time constant (because the arguments are not constants, or the
result of the comparison is unspecified). When the result is constant
just compare the pointers directly without casting.

This ensures that the function can be called in constant expressions
with suitable arguments, but still yields a total order even for
otherwise unspecified pointer comparisons.

        PR libstdc++/78420
        * include/bits/stl_function.h (__ptr_rel_ops): New struct providing
        relational operators defining total order on pointers.
        (greater<_Tp*>, less<_Tp*>, greater_equal<_Tp*>, less_equal<_Tp>*):
        New partial specializations for pointers, using __ptr_rel_ops.
        (greater<void>, less<void>, greater_equal<void>, less_equal<void*):
        Dispatch to __ptr_rel_ops when both arguments are pointers.
        * testsuite/20_util/function_objects/comparisons_pointer.cc: New.

Tested powerpc64le-linux. I intend to commit this to trunk and
backport to the active branches, unless anybody sees a problem with
this solution, or has any suggestions for improvement.

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

commit e03605434a5c378c55e1961402319a20af75a909
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Mar 8 20:27:04 2018 +0000

    PR libstdc++/78420 Make std::less etc. yield total order for pointers
    
    In order to meet the total order requirements of [comparisons] p2 we
    need to cast unrelated pointers to uintptr_t before comparing them.
    Those casts aren't allowed in constant expressions, so only cast
    when __builtin_constant_p says the result of the comparison is not a
    compile-time constant (because the arguments are not constants, or the
    result of the comparison is unspecified). When the result is constant
    just compare the pointers directly without casting.
    
    This ensures that the function can be called in constant expressions
    with suitable arguments, but still yields a total order even for
    otherwise unspecified pointer comparisons.
    
            PR libstdc++/78420
            * include/bits/stl_function.h (__ptr_rel_ops): New struct providing
            relational operators defining total order on pointers.
            (greater<_Tp*>, less<_Tp*>, greater_equal<_Tp*>, less_equal<_Tp>*):
            New partial specializations for pointers, using __ptr_rel_ops.
            (greater<void>, less<void>, greater_equal<void>, less_equal<void*):
            Dispatch to __ptr_rel_ops when both arguments are pointers.
            * testsuite/20_util/function_objects/comparisons_pointer.cc: New.

diff --git a/libstdc++-v3/include/bits/stl_function.h b/libstdc++-v3/include/bits/stl_function.h
index f5f98b25395..27739ad7089 100644
--- a/libstdc++-v3/include/bits/stl_function.h
+++ b/libstdc++-v3/include/bits/stl_function.h
@@ -406,14 +406,91 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       { return __x <= __y; }
     };
 
-#if __cplusplus > 201103L
+  struct __ptr_rel_ops
+  {
+    __ptr_rel_ops() = delete;
+    ~__ptr_rel_ops() = delete;
+
+    template<typename _Tp, typename _Up>
+      static _GLIBCXX14_CONSTEXPR bool
+      _S_greater(_Tp* __x, _Up* __y)
+      {
+	if (__builtin_constant_p (__x > __y))
+	  return __x > __y;
+	return (__UINTPTR_TYPE__)__x > (__UINTPTR_TYPE__)__y;
+      }
+
+    template<typename _Tp, typename _Up>
+      static _GLIBCXX14_CONSTEXPR bool
+      _S_less(_Tp* __x, _Up* __y)
+      {
+	if (__builtin_constant_p (__x < __y))
+	  return __x < __y;
+	return (__UINTPTR_TYPE__)__x < (__UINTPTR_TYPE__)__y;
+      }
+
+    template<typename _Tp, typename _Up>
+      static _GLIBCXX14_CONSTEXPR bool
+      _S_greater_equal(_Tp* __x, _Up* __y)
+      {
+	if (__builtin_constant_p (__x >= __y))
+	  return __x >= __y;
+	return (__UINTPTR_TYPE__)__x >= (__UINTPTR_TYPE__)__y;
+      }
+
+    template<typename _Tp, typename _Up>
+      static _GLIBCXX14_CONSTEXPR bool
+      _S_less_equal(_Tp* __x, _Up* __y)
+      {
+	if (__builtin_constant_p (__x <= __y))
+	  return __x <= __y;
+	return (__UINTPTR_TYPE__)__x <= (__UINTPTR_TYPE__)__y;
+      }
+  };
+
+  // Partial specialization of std::greater for pointers.
+  template<typename _Tp>
+    struct greater<_Tp*> : public binary_function<_Tp*, _Tp*, bool>
+    {
+      _GLIBCXX14_CONSTEXPR bool
+      operator()(_Tp* const& __x, _Tp* const& __y) const _GLIBCXX_NOTHROW
+      { return __ptr_rel_ops::_S_greater(__x, __y); }
+    };
+
+  // Partial specialization of std::less for pointers.
+  template<typename _Tp>
+    struct less<_Tp*> : public binary_function<_Tp*, _Tp*, bool>
+    {
+      _GLIBCXX14_CONSTEXPR bool
+      operator()(_Tp* const& __x, _Tp* const& __y) const _GLIBCXX_NOTHROW
+      { return __ptr_rel_ops::_S_less(__x, __y); }
+    };
+
+  // Partial specialization of std::greater_equal for pointers.
+  template<typename _Tp>
+    struct greater_equal<_Tp*> : public binary_function<_Tp*, _Tp*, bool>
+    {
+      _GLIBCXX14_CONSTEXPR bool
+      operator()(_Tp* const& __x, _Tp* const& __y) const _GLIBCXX_NOTHROW
+      { return __ptr_rel_ops::_S_greater_equal(__x, __y); }
+    };
+
+  // Partial specialization of std::less_equal for pointers.
+  template<typename _Tp>
+    struct less_equal<_Tp*> : public binary_function<_Tp*, _Tp*, bool>
+    {
+      _GLIBCXX14_CONSTEXPR bool
+      operator()(_Tp* const& __x, _Tp* const& __y) const _GLIBCXX_NOTHROW
+      { return __ptr_rel_ops::_S_less_equal(__x, __y); }
+    };
+
+#if __cplusplus >= 201402L
   /// One of the @link comparison_functors comparison functors@endlink.
   template<>
     struct equal_to<void>
     {
       template <typename _Tp, typename _Up>
-	_GLIBCXX14_CONSTEXPR
-	auto
+	constexpr auto
 	operator()(_Tp&& __t, _Up&& __u) const
 	noexcept(noexcept(std::forward<_Tp>(__t) == std::forward<_Up>(__u)))
 	-> decltype(std::forward<_Tp>(__t) == std::forward<_Up>(__u))
@@ -427,8 +504,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     struct not_equal_to<void>
     {
       template <typename _Tp, typename _Up>
-	_GLIBCXX14_CONSTEXPR
-	auto
+	constexpr auto
 	operator()(_Tp&& __t, _Up&& __u) const
 	noexcept(noexcept(std::forward<_Tp>(__t) != std::forward<_Up>(__u)))
 	-> decltype(std::forward<_Tp>(__t) != std::forward<_Up>(__u))
@@ -442,14 +518,27 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     struct greater<void>
     {
       template <typename _Tp, typename _Up>
-	_GLIBCXX14_CONSTEXPR
-	auto
+	constexpr auto
 	operator()(_Tp&& __t, _Up&& __u) const
 	noexcept(noexcept(std::forward<_Tp>(__t) > std::forward<_Up>(__u)))
 	-> decltype(std::forward<_Tp>(__t) > std::forward<_Up>(__u))
-	{ return std::forward<_Tp>(__t) > std::forward<_Up>(__u); }
+	{
+	  using _Ptrs = __and_<is_pointer<decay_t<_Tp>>, is_pointer<decay_t<_Up>>>;
+	  return _S_cmp(std::forward<_Tp>(__t), std::forward<_Up>(__u), _Ptrs());
+	}
 
       typedef __is_transparent is_transparent;
+
+    private:
+      template<typename _Tp, typename _Up>
+	static constexpr decltype(auto)
+	_S_cmp(_Tp&& __t, _Up&& __u, false_type)
+	{ return std::forward<_Tp>(__t) > std::forward<_Up>(__u); }
+
+      template<typename _Tp, typename _Up>
+	static constexpr bool
+	_S_cmp(_Tp* __t, _Up* __u, true_type) noexcept
+	{ return __ptr_rel_ops::_S_greater(__t, __u); }
     };
 
   /// One of the @link comparison_functors comparison functors@endlink.
@@ -457,14 +546,27 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     struct less<void>
     {
       template <typename _Tp, typename _Up>
-	_GLIBCXX14_CONSTEXPR
-	auto
+	constexpr auto
 	operator()(_Tp&& __t, _Up&& __u) const
 	noexcept(noexcept(std::forward<_Tp>(__t) < std::forward<_Up>(__u)))
 	-> decltype(std::forward<_Tp>(__t) < std::forward<_Up>(__u))
-	{ return std::forward<_Tp>(__t) < std::forward<_Up>(__u); }
+	{
+	  using _Ptrs = __and_<is_pointer<decay_t<_Tp>>, is_pointer<decay_t<_Up>>>;
+	  return _S_cmp(std::forward<_Tp>(__t), std::forward<_Up>(__u), _Ptrs());
+	}
 
       typedef __is_transparent is_transparent;
+
+    private:
+      template<typename _Tp, typename _Up>
+	static constexpr decltype(auto)
+	_S_cmp(_Tp&& __t, _Up&& __u, false_type)
+	{ return std::forward<_Tp>(__t) < std::forward<_Up>(__u); }
+
+      template<typename _Tp, typename _Up>
+	static constexpr bool
+	_S_cmp(_Tp* __t, _Up* __u, true_type) noexcept
+	{ return __ptr_rel_ops::_S_less(__t, __u); }
     };
 
   /// One of the @link comparison_functors comparison functors@endlink.
@@ -472,14 +574,27 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     struct greater_equal<void>
     {
       template <typename _Tp, typename _Up>
-	_GLIBCXX14_CONSTEXPR
-	auto
+	constexpr auto
 	operator()(_Tp&& __t, _Up&& __u) const
 	noexcept(noexcept(std::forward<_Tp>(__t) >= std::forward<_Up>(__u)))
 	-> decltype(std::forward<_Tp>(__t) >= std::forward<_Up>(__u))
-	{ return std::forward<_Tp>(__t) >= std::forward<_Up>(__u); }
+	{
+	  using _Ptrs = __and_<is_pointer<decay_t<_Tp>>, is_pointer<decay_t<_Up>>>;
+	  return _S_cmp(std::forward<_Tp>(__t), std::forward<_Up>(__u), _Ptrs());
+	}
 
       typedef __is_transparent is_transparent;
+
+    private:
+      template<typename _Tp, typename _Up>
+	static constexpr decltype(auto)
+	_S_cmp(_Tp&& __t, _Up&& __u, false_type)
+	{ return std::forward<_Tp>(__t) >= std::forward<_Up>(__u); }
+
+      template<typename _Tp, typename _Up>
+	static constexpr bool
+	_S_cmp(_Tp* __t, _Up* __u, true_type) noexcept
+	{ return __ptr_rel_ops::_S_greater_equal(__t, __u); }
     };
 
   /// One of the @link comparison_functors comparison functors@endlink.
@@ -487,16 +602,29 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     struct less_equal<void>
     {
       template <typename _Tp, typename _Up>
-	_GLIBCXX14_CONSTEXPR
-	auto
+	constexpr auto
 	operator()(_Tp&& __t, _Up&& __u) const
 	noexcept(noexcept(std::forward<_Tp>(__t) <= std::forward<_Up>(__u)))
 	-> decltype(std::forward<_Tp>(__t) <= std::forward<_Up>(__u))
-	{ return std::forward<_Tp>(__t) <= std::forward<_Up>(__u); }
+	{
+	  using _Ptrs = __and_<is_pointer<decay_t<_Tp>>, is_pointer<decay_t<_Up>>>;
+	  return _S_cmp(std::forward<_Tp>(__t), std::forward<_Up>(__u), _Ptrs());
+	}
 
       typedef __is_transparent is_transparent;
+
+    private:
+      template<typename _Tp, typename _Up>
+	static constexpr decltype(auto)
+	_S_cmp(_Tp&& __t, _Up&& __u, false_type)
+	{ return std::forward<_Tp>(__t) <= std::forward<_Up>(__u); }
+
+      template<typename _Tp, typename _Up>
+	static constexpr bool
+	_S_cmp(_Tp* __t, _Up* __u, true_type) noexcept
+	{ return __ptr_rel_ops::_S_less_equal(__t, __u); }
     };
-#endif
+#endif // C++14
   /** @}  */
 
   // 20.3.4 logical operations
diff --git a/libstdc++-v3/testsuite/20_util/function_objects/comparisons_pointer.cc b/libstdc++-v3/testsuite/20_util/function_objects/comparisons_pointer.cc
new file mode 100644
index 00000000000..317e42d8f11
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/function_objects/comparisons_pointer.cc
@@ -0,0 +1,166 @@
+// Copyright (C) 2018 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-do run }
+
+#include <functional>
+#include <sstream>
+#include <testsuite_hooks.h>
+
+int b[8];
+int a[8];
+
+void
+test01()
+{
+  auto p = a + 8;
+  std::greater<int*> gt;
+
+  std::stringstream ss;
+  ss << gt(p, b) << ' ' << gt(b, p) << ' ' << (!gt(p, b) && !gt(b, p));
+  int sum = 0, n = 0;
+  while (ss >> n)
+    sum += n;
+  VERIFY( sum == 1 );
+
+#if __cplusplus >= 201402L
+  static_assert( gt(a+1, a), "constexpr greater<int*>" );
+  static_assert( !gt(a, a+1), "constexpr greater<int*>" );
+
+  ss.str("");
+  ss.clear();
+  sum = 0;
+  auto p2 = a + 8;
+  std::greater<> gt2;
+  ss << gt2(p2, b) << ' ' << gt2(b, p2) << ' ' << (!gt2(p2, b) && !gt2(b, p2));
+  while (ss >> n)
+    sum += n;
+  VERIFY( sum == 1 );
+
+  static_assert( gt2(a+1, a), "constexpr greater<>" );
+  static_assert( !gt2(a, a+1), "constexpr greater<>" );
+#endif
+}
+
+void
+test02()
+{
+  auto p = a + 8;
+  std::less<int*> lt;
+
+  std::stringstream ss;
+  ss << lt(p, b) << ' ' << lt(b, p) << ' ' << (!lt(p, b) && !lt(b, p));
+  int sum = 0, n = 0;
+  while (ss >> n)
+    sum += n;
+  VERIFY( sum == 1 );
+
+#if __cplusplus >= 201402L
+  static_assert( lt(a, a+1), "constexpr less<int*>" );
+  static_assert( !lt(a+1, a), "constexpr less<int*>" );
+
+  ss.str("");
+  ss.clear();
+  sum = 0;
+  auto p2 = a + 8;
+  std::less<> lt2;
+  ss << lt2(p2, b) << ' ' << lt2(b, p2) << ' ' << (!lt2(p2, b) && !lt2(b, p2));
+  while (ss >> n)
+    sum += n;
+  VERIFY( sum == 1 );
+
+  static_assert( lt2(a, a+1), "constexpr less<>" );
+  static_assert( !lt2(a+1, a), "constexpr less<>" );
+#endif
+}
+
+void
+test03()
+{
+  auto p = a + 8;
+  std::greater_equal<int*> ge;
+
+  std::stringstream ss;
+  ss << !ge(p, b) << ' ' << !ge(b, p) << ' ' << (ge(p, b) && ge(b, p));
+  int sum = 0, n = 0;
+  while (ss >> n)
+    sum += n;
+  VERIFY( sum == 1 );
+
+#if __cplusplus >= 201402L
+  static_assert( !ge(a, a+1), "constexpr greater_equal<int*>" );
+  static_assert( ge(a, a), "constexpr greater_equal<int*>" );
+  static_assert( ge(a+1, a), "constexpr greater_equal<int*>" );
+
+  ss.str("");
+  ss.clear();
+  sum = 0;
+  auto p2 = a + 8;
+  std::greater_equal<> ge2;
+  ss << !ge2(p2, b) << ' ' << !ge2(b, p2) << ' ' << (ge2(p2, b) && ge2(b, p2));
+  while (ss >> n)
+    sum += n;
+  VERIFY( sum == 1 );
+
+  static_assert( !ge2(a, a+1), "constexpr greater_equal<>" );
+  static_assert( ge2(a, a), "constexpr greater_equal<>" );
+  static_assert( ge2(a+1, a), "constexpr greater_equal<>" );
+#endif
+}
+
+void
+test04()
+{
+  auto p = a + 8;
+  std::less_equal<int*> le;
+
+  std::stringstream ss;
+  ss << !le(p, b) << ' ' << !le(b, p) << ' ' << (le(p, b) && le(b, p));
+  int sum = 0, n = 0;
+  while (ss >> n)
+    sum += n;
+  VERIFY( sum == 1 );
+
+#if __cplusplus >= 201402L
+  static_assert( !le(a+1, a), "constexpr less_equal<int*>" );
+  static_assert( le(a, a), "constexpr less_equal<int*>" );
+  static_assert( le(a, a+1), "constexpr less_equal<int*>" );
+
+  ss.str("");
+  ss.clear();
+  sum = 0;
+  auto p2 = a + 8;
+  std::less_equal<> le2;
+  ss << !le2(p2, b) << ' ' << !le2(b, p2) << ' ' << (le2(p2, b) && le2(b, p2));
+  while (ss >> n)
+    sum += n;
+  VERIFY( sum == 1 );
+
+  static_assert( !le2(a+1, a), "constexpr less_equal<>" );
+  static_assert( le2(a, a), "constexpr less_equal<>" );
+  static_assert( le2(a, a+1), "constexpr less_equal<>" );
+#endif
+}
+
+int
+main()
+{
+  test01();
+  test02();
+  test03();
+  test04();
+}

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-03-22 14:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-09  0:41 PR libstdc++/78420 Make std::less etc. yield total order for pointers Jonathan Wakely
2018-03-13 16:30 ` Jonathan Wakely
2018-03-14 23:27   ` Jonathan Wakely
2018-03-18 10:27     ` Jonathan Wakely
2018-03-22 14:27     ` Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).