diff mbox series

[committed] libstdc++: Make std::pointer_traits SFINAE-friendly [PR96416]

Message ID 20211125231243.1503399-1-jwakely@redhat.com
State New
Headers show
Series [committed] libstdc++: Make std::pointer_traits SFINAE-friendly [PR96416] | expand

Commit Message

Jonathan Wakely Nov. 25, 2021, 11:12 p.m. UTC
Tested x86_64-linux, pushed to trunk.



This implements the resolution I'm proposing for LWG 3545, to avoid hard
errors when using std::to_address for types that make pointer_traits
ill-formed.

Consistent with std::iterator_traits, instantiating std::pointer_traits
for a non-pointer type will be well-formed, but give an empty type with
no member types. This avoids the problematic cases for std::to_address.
Additionally, the pointer_to member is now only declared when the
element type is not cv void (and for C++20, when the function body would
be well-formed). The rebind member was already SFINAE-friendly in our
implementation.

libstdc++-v3/ChangeLog:

	PR libstdc++/96416
	* include/bits/ptr_traits.h (pointer_traits): Reimplement to be
	SFINAE-friendly (LWG 3545).
	* testsuite/20_util/pointer_traits/lwg3545.cc: New test.
	* testsuite/20_util/to_address/1_neg.cc: Adjust dg-error line.
	* testsuite/20_util/to_address/lwg3545.cc: New test.
---
 libstdc++-v3/include/bits/ptr_traits.h        | 167 +++++++++++++-----
 .../20_util/pointer_traits/lwg3545.cc         | 120 +++++++++++++
 .../testsuite/20_util/to_address/1_neg.cc     |   2 +-
 .../testsuite/20_util/to_address/lwg3545.cc   |  12 ++
 4 files changed, 251 insertions(+), 50 deletions(-)
 create mode 100644 libstdc++-v3/testsuite/20_util/pointer_traits/lwg3545.cc
 create mode 100644 libstdc++-v3/testsuite/20_util/to_address/lwg3545.cc
diff mbox series

Patch

diff --git a/libstdc++-v3/include/bits/ptr_traits.h b/libstdc++-v3/include/bits/ptr_traits.h
index 115b86d43e4..4987fa9942f 100644
--- a/libstdc++-v3/include/bits/ptr_traits.h
+++ b/libstdc++-v3/include/bits/ptr_traits.h
@@ -35,6 +35,7 @@ 
 #include <bits/move.h>
 
 #if __cplusplus > 201703L
+#include <concepts>
 #define __cpp_lib_constexpr_memory 201811L
 namespace __gnu_debug { struct _Safe_iterator_base; }
 #endif
@@ -45,55 +46,119 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   class __undefined;
 
-  // Given Template<T, ...> return T, otherwise invalid.
+  // For a specialization `SomeTemplate<T, Types...>` the member `type` is T,
+  // otherwise `type` is `__undefined`.
   template<typename _Tp>
     struct __get_first_arg
     { using type = __undefined; };
 
-  template<template<typename, typename...> class _Template, typename _Tp,
+  template<template<typename, typename...> class _SomeTemplate, typename _Tp,
            typename... _Types>
-    struct __get_first_arg<_Template<_Tp, _Types...>>
+    struct __get_first_arg<_SomeTemplate<_Tp, _Types...>>
     { using type = _Tp; };
 
-  template<typename _Tp>
-    using __get_first_arg_t = typename __get_first_arg<_Tp>::type;
-
-  // Given Template<T, ...> and U return Template<U, ...>, otherwise invalid.
+  // For a specialization `SomeTemplate<T, Args...>` and a type `U` the member
+  // `type` is `SomeTemplate<U, Args...>`, otherwise there is no member `type`.
   template<typename _Tp, typename _Up>
     struct __replace_first_arg
     { };
 
-  template<template<typename, typename...> class _Template, typename _Up,
+  template<template<typename, typename...> class _SomeTemplate, typename _Up,
            typename _Tp, typename... _Types>
-    struct __replace_first_arg<_Template<_Tp, _Types...>, _Up>
-    { using type = _Template<_Up, _Types...>; };
+    struct __replace_first_arg<_SomeTemplate<_Tp, _Types...>, _Up>
+    { using type = _SomeTemplate<_Up, _Types...>; };
 
-  template<typename _Tp, typename _Up>
-    using __replace_first_arg_t = typename __replace_first_arg<_Tp, _Up>::type;
-
-  template<typename _Tp>
-    using __make_not_void
-      = __conditional_t<is_void<_Tp>::value, __undefined, _Tp>;
-
-  /**
-   * @brief  Uniform interface to all pointer-like types
-   * @ingroup pointer_abstractions
-  */
+#if __cpp_concepts
+  // When concepts are supported detection of _Ptr::element_type is done
+  // by a requires-clause, so __ptr_traits_elem_t only needs to do this:
   template<typename _Ptr>
-    struct pointer_traits
+    using __ptr_traits_elem_t = typename __get_first_arg<_Ptr>::type;
+#else
+  // Detect the element type of a pointer-like type.
+  template<typename _Ptr, typename = void>
+    struct __ptr_traits_elem : __get_first_arg<_Ptr>
+    { };
+
+  // Use _Ptr::element_type if is a valid type.
+  template<typename _Ptr>
+    struct __ptr_traits_elem<_Ptr, __void_t<typename _Ptr::element_type>>
+    { using type = typename _Ptr::element_type; };
+
+  template<typename _Ptr>
+    using __ptr_traits_elem_t = typename __ptr_traits_elem<_Ptr>::type;
+#endif
+
+  // Define pointer_traits<P>::pointer_to.
+  template<typename _Ptr, typename _Elt, bool = is_void<_Elt>::value>
+    struct __ptr_traits_ptr_to
+    {
+      using pointer = _Ptr;
+      using element_type = _Elt;
+
+      /**
+       *  @brief  Obtain a pointer to an object
+       *  @param  __r  A reference to an object of type `element_type`
+       *  @return `pointer::pointer_to(__e)`
+       *  @pre `pointer::pointer_to(__e)` is a valid expression.
+      */
+      static pointer
+      pointer_to(element_type& __e)
+#if __cpp_lib_concepts
+      requires requires {
+	{ pointer::pointer_to(__e) } -> convertible_to<pointer>;
+      }
+#endif
+      { return pointer::pointer_to(__e); }
+    };
+
+  // Do not define pointer_traits<P>::pointer_to if element type is void.
+  template<typename _Ptr, typename _Elt>
+    struct __ptr_traits_ptr_to<_Ptr, _Elt, true>
+    { };
+
+  // Partial specialization defining pointer_traits<T*>::pointer_to(T&).
+  template<typename _Tp>
+    struct __ptr_traits_ptr_to<_Tp*, _Tp, false>
+    {
+      using pointer = _Tp*;
+      using element_type = _Tp;
+
+      /**
+       *  @brief  Obtain a pointer to an object
+       *  @param  __r  A reference to an object of type `element_type`
+       *  @return `addressof(__r)`
+      */
+      static _GLIBCXX20_CONSTEXPR pointer
+      pointer_to(element_type& __r) noexcept
+      { return std::addressof(__r); }
+    };
+
+  template<typename _Ptr, typename _Elt>
+    struct __ptr_traits_impl : __ptr_traits_ptr_to<_Ptr, _Elt>
     {
     private:
-      template<typename _Tp>
-	using __element_type = typename _Tp::element_type;
+      template<typename _Tp, typename = void>
+	struct __difference { using type = ptrdiff_t; };
 
       template<typename _Tp>
-	using __difference_type = typename _Tp::difference_type;
+#if __cpp_concepts
+	requires requires { typename _Tp::difference_type; }
+	struct __difference<_Tp>
+#else
+	struct __difference<_Tp, __void_t<typename _Tp::difference_type>>
+#endif
+	{ using type = typename _Tp::difference_type; };
 
       template<typename _Tp, typename _Up, typename = void>
 	struct __rebind : __replace_first_arg<_Tp, _Up> { };
 
       template<typename _Tp, typename _Up>
+#if __cpp_concepts
+	requires requires { typename _Tp::template rebind<_Up>; }
+	struct __rebind<_Tp, _Up>
+#else
 	struct __rebind<_Tp, _Up, __void_t<typename _Tp::template rebind<_Up>>>
+#endif
 	{ using type = typename _Tp::template rebind<_Up>; };
 
     public:
@@ -101,31 +166,45 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       using pointer = _Ptr;
 
       /// The type pointed to.
-      using element_type
-	= __detected_or_t<__get_first_arg_t<_Ptr>, __element_type, _Ptr>;
+      using element_type = _Elt;
 
       /// The type used to represent the difference between two pointers.
-      using difference_type
-	= __detected_or_t<ptrdiff_t, __difference_type, _Ptr>;
+      using difference_type = typename __difference<_Ptr>::type;
 
       /// A pointer to a different type.
       template<typename _Up>
         using rebind = typename __rebind<_Ptr, _Up>::type;
-
-      static _Ptr
-      pointer_to(__make_not_void<element_type>& __e)
-      { return _Ptr::pointer_to(__e); }
-
-      static_assert(!is_same<element_type, __undefined>::value,
-	  "pointer type defines element_type or is like SomePointer<T, Args>");
     };
 
+  // _GLIBCXX_RESOLVE_LIB_DEFECTS
+  // 3545. std::pointer_traits should be SFINAE-friendly
+  template<typename _Ptr>
+    struct __ptr_traits_impl<_Ptr, __undefined>
+    { };
+
+  /**
+   * @brief  Uniform interface to all pointer-like types
+   * @ingroup pointer_abstractions
+   * @since C++11
+  */
+  template<typename _Ptr>
+    struct pointer_traits : __ptr_traits_impl<_Ptr, __ptr_traits_elem_t<_Ptr>>
+    { };
+
+#if __cpp_concepts
+  template<typename _Ptr> requires requires { typename _Ptr::element_type; }
+    struct pointer_traits<_Ptr>
+    : __ptr_traits_impl<_Ptr, typename _Ptr::element_type>
+    { };
+#endif
+
   /**
    * @brief  Partial specialization for built-in pointers.
    * @ingroup pointer_abstractions
+   * @since C++11
   */
   template<typename _Tp>
-    struct pointer_traits<_Tp*>
+    struct pointer_traits<_Tp*> : __ptr_traits_ptr_to<_Tp*, _Tp>
     {
       /// The pointer type
       typedef _Tp* pointer;
@@ -133,18 +212,8 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       typedef _Tp  element_type;
       /// Type used to represent the difference between two pointers
       typedef ptrdiff_t difference_type;
-
-      template<typename _Up>
-        using rebind = _Up*;
-
-      /**
-       *  @brief  Obtain a pointer to an object
-       *  @param  __r  A reference to an object of type @c element_type
-       *  @return @c addressof(__r)
-      */
-      static _GLIBCXX20_CONSTEXPR pointer
-      pointer_to(__make_not_void<element_type>& __r) noexcept
-      { return std::addressof(__r); }
+      /// A pointer to a different type.
+      template<typename _Up> using rebind = _Up*;
     };
 
   /// Convenience alias for rebinding pointers.
diff --git a/libstdc++-v3/testsuite/20_util/pointer_traits/lwg3545.cc b/libstdc++-v3/testsuite/20_util/pointer_traits/lwg3545.cc
new file mode 100644
index 00000000000..8325cb66d08
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/pointer_traits/lwg3545.cc
@@ -0,0 +1,120 @@ 
+// { dg-do compile { target c++11 } }
+
+// LWG 3545. std::pointer_traits should be SFINAE-friendly
+
+#include <memory>
+
+using std::is_same;
+
+template<typename> using void_t = void;
+
+template<template<typename> class Probe, typename T, typename = void>
+  struct has_member
+  : std::false_type { };
+
+template<template<typename> class Probe, typename T>
+  struct has_member<Probe, T, void_t<Probe<T>>>
+  : std::true_type { };
+
+template<typename T>
+  using element_type = typename T::element_type;
+template<typename T>
+  using pointer = typename T::pointer;
+template<typename T>
+  using difference_type = typename T::difference_type;
+template<typename T>
+  using rebind = typename T::template rebind<short>;
+template<typename T>
+  using pointer_to = decltype(T::pointer_to(std::declval<element_type<T>&>()));
+
+using invalid = std::pointer_traits<int>;
+invalid i; // invalid instantiation is not ill-formed
+
+static_assert( !has_member<element_type, invalid>::value, "" );
+static_assert( !has_member<pointer, invalid>::value, "" );
+static_assert( !has_member<difference_type, invalid>::value, "" );
+static_assert( !has_member<rebind, invalid>::value, "" );
+static_assert( !has_member<pointer_to, invalid>::value, "" );
+
+struct I
+{
+  // These members should not be used by pointer_traits<P>::pointer.
+  using pointer = int;
+  using difference_type = int;
+  template<typename> using rebind = int;
+};
+
+using invalid2 = std::pointer_traits<I>;
+
+static_assert( !has_member<element_type, invalid2>::value, "" );
+static_assert( !has_member<pointer, invalid2>::value, "" );
+static_assert( !has_member<difference_type, invalid2>::value, "" );
+static_assert( !has_member<rebind, invalid2>::value, "" );
+static_assert( !has_member<pointer_to, invalid2>::value, "" );
+
+struct P
+{
+  using element_type = long;
+  struct pointer { }; // should not be used by pointer_traits<P>::pointer
+
+  P pointer_to(long&) const; // not static, should not be used.
+};
+using Ptraits = std::pointer_traits<P>;
+Ptraits p;
+
+static_assert( is_same<element_type<Ptraits>, long>::value, "" );
+static_assert( is_same<pointer<Ptraits>, P>::value, "" );
+static_assert( is_same<difference_type<Ptraits>, std::ptrdiff_t>::value, "" );
+static_assert( !has_member<rebind, Ptraits>::value, "" );
+#if __cplusplus >= 202002L
+// pointer_traits<P>::pointer_to(long&) is constrained in C++20 and later.
+static_assert( !has_member<pointer_to, Ptraits>::value, "" );
+#else
+static_assert( is_same<pointer_to<Ptraits>, P>::value, "" );
+#endif
+
+struct V { using element_type = const void; };
+using Vtraits = std::pointer_traits<V>;
+Vtraits v;
+
+static_assert( is_same<element_type<Vtraits>, const void>::value, "" );
+static_assert( is_same<pointer<Vtraits>, V>::value, "" );
+static_assert( is_same<difference_type<Vtraits>, std::ptrdiff_t>::value, "" );
+static_assert( !has_member<rebind, Vtraits>::value, "" );
+static_assert( !has_member<pointer_to, Vtraits>::value, "" );
+
+template<typename T>
+struct clever_ptr
+{
+  static T obj;
+
+  static clever_ptr pointer_to(T&) { return {}; }
+  constexpr T* operator->() const { return &obj; }
+};
+
+using Ctraits = std::pointer_traits<clever_ptr<char>>;
+
+static_assert( is_same<element_type<Ctraits>, char>::value, "" );
+static_assert( is_same<pointer<Ctraits>, clever_ptr<char>>::value, "" );
+static_assert( is_same<difference_type<Ctraits>, std::ptrdiff_t>::value, "" );
+static_assert( is_same<rebind<Ctraits>, clever_ptr<short>>::value, "" );
+static_assert( is_same<pointer_to<Ctraits>, clever_ptr<char>>::value, "" );
+
+#if __cplusplus >= 202002L
+static_assert( std::to_address(clever_ptr<char>{}) == &clever_ptr<char>::obj, "" );
+
+int the_int;
+
+template<>
+struct std::pointer_traits<clever_ptr<int>>
+{
+  using element_type = int;
+  using difference_type = std::ptrdiff_t;
+  using pointer = clever_ptr<int>;
+
+  static constexpr int* to_address(pointer p) { return &the_int; }
+};
+
+// Should use pointer_traits<clever_ptr<int>>::to_address
+static_assert( std::to_address(clever_ptr<int>{}) == &the_int, "" );
+#endif
diff --git a/libstdc++-v3/testsuite/20_util/to_address/1_neg.cc b/libstdc++-v3/testsuite/20_util/to_address/1_neg.cc
index 5030f89b345..65aff7a8a5e 100644
--- a/libstdc++-v3/testsuite/20_util/to_address/1_neg.cc
+++ b/libstdc++-v3/testsuite/20_util/to_address/1_neg.cc
@@ -17,7 +17,7 @@ 
 
 // { dg-options "-std=gnu++2a" }
 // { dg-do compile { target c++2a } }
-// { dg-error "not a function pointer" "" { target *-*-* } 158 }
+// { dg-error "not a function pointer" "" { target *-*-* } 0 }
 
 #include <memory>
 
diff --git a/libstdc++-v3/testsuite/20_util/to_address/lwg3545.cc b/libstdc++-v3/testsuite/20_util/to_address/lwg3545.cc
new file mode 100644
index 00000000000..4dc5fdcdde0
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/to_address/lwg3545.cc
@@ -0,0 +1,12 @@ 
+// { dg-do compile { target c++20 } }
+// { dg-options "-std=gnu++20" }
+
+#include <memory>
+
+template<typename T, bool> struct nttp_ptr
+{
+  T* operator->() const { return nullptr; }
+};
+
+// This gives an error in C++20, which the LWG 3545 resolution should fix:
+auto x = std::to_address( nttp_ptr<int, true>() );