diff mbox

Implement std::pointer_traits::to_address as per P0653R0

Message ID 20170720165340.GK15340@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely July 20, 2017, 4:53 p.m. UTC
On 16/07/17 17:54 -0400, Glen Fernandes wrote:
>diff --git a/libstdc++-v3/include/bits/allocated_ptr.h b/libstdc++-v3/include/bits/allocated_ptr.h
>index 773b3f5..72e0179 100644
>--- a/libstdc++-v3/include/bits/allocated_ptr.h
>+++ b/libstdc++-v3/include/bits/allocated_ptr.h
>@@ -82,16 +82,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       }
>
>       /// Get the address that the owned pointer refers to.
>-      value_type* get() { return _S_raw_ptr(_M_ptr); }
>+      value_type* get()
>+      { return std::pointer_traits<pointer>::to_address(_M_ptr); }

Much nicer :-)

However, it's unfortunately not that simple. The proposal hasn't been
approved yet, so we can't add "to_address" as a new identifier, and we
certainly can't add it to C++17 or earlier modes, otherwise this valid
C++17 program won't compile:

#define to_address(iter) (&*iter)
#include <memory>
int main() { }

Realistically, I don't think we should be adding this (in this form)
to libstdc++ before it's been approved by the committee. But more on
this at the end of the email.

>diff --git a/libstdc++-v3/include/bits/ptr_traits.h b/libstdc++-v3/include/bits/ptr_traits.h
>index 797e7fc..93e95ad 100644
>--- a/libstdc++-v3/include/bits/ptr_traits.h
>+++ b/libstdc++-v3/include/bits/ptr_traits.h
>@@ -111,6 +111,22 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       pointer_to(__make_not_void<element_type>& __e)
>       { return _Ptr::pointer_to(__e); }
>
>+    private:
>+      template<typename _Tp>
>+      static element_type*
>+      __to_address(_Tp __p) noexcept
>+      { return pointer_traits<_Tp>::to_address(__p); }

There should be a blank line after this function.

>+    public:
>+      /**
>+       *  @brief  Obtain address referenced by a pointer to an object
>+       *  @param  __p  A pointer to an object
>+       *  @return @c pointer_traits<decltype(Expr)>::to_address(Expr)
>+                  where @c Expr is @c __p.operator->()
>+      */
>+      static element_type*
>+      to_address(pointer __p) noexcept
>+      { return __to_address(__p.operator->()); }

This would need to be guarded by #if __cplusplus > 201707L so it's not
present for C++17 and lower.

>+
>       static_assert(!is_same<element_type, __undefined>::value,
> 	  "pointer type defines element_type or is like SomePointer<T, Args>");
>     };
>@@ -140,6 +156,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       static pointer
>       pointer_to(__make_not_void<element_type>& __r) noexcept
>       { return std::addressof(__r); }
>+
>+      /**
>+       *  @brief  Obtain address referenced by a pointer to an object
>+       *  @param  __p  A pointer to an object
>+       *  @return @c __p
>+      */
>+      static element_type*
>+      to_address(pointer __p) noexcept { return __p; }

#if __cplusplus > 201707L

>diff --git a/libstdc++-v3/include/ext/pointer.h b/libstdc++-v3/include/ext/pointer.h
>index 8432da0..d7ab4e2 100644
>--- a/libstdc++-v3/include/ext/pointer.h
>+++ b/libstdc++-v3/include/ext/pointer.h
>@@ -584,6 +584,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
>       static pointer pointer_to(typename pointer::reference __r) noexcept
>       { return pointer(std::addressof(__r)); }
>+
>+      static element_type* to_address(pointer __p) noexcept
>+      { return __p.operator->(); }

#if __cplusplus > 201707L


>diff --git a/libstdc++-v3/testsuite/20_util/pointer_traits/to_address.cc b/libstdc++-v3/testsuite/20_util/pointer_traits/to_address.cc
>new file mode 100644
>index 0000000..3c8cfbd
>--- /dev/null
>+++ b/libstdc++-v3/testsuite/20_util/pointer_traits/to_address.cc
>@@ -0,0 +1,69 @@
>+// { dg-do run { target c++11 } }

We can't test a C++20 feature when the effective-target is c++11.

We have a more general problem with this, which is that if it's only
available for C++2a mode then we can't use the new feature in most of
the library. Which would be very unfortunate. I want to use this!

In order to clean up the various places in the library that
could/should use to_address I think we need our own version of it.
Something like our std::__addressof which can be used even for C++98,
and is used by the implementation of the C++11 std::addressof.

The attached patch does something like that. I think I'll commit this,
as it's a small improvement over what we have today. As the design of
P0653 evolves (*) we can revisit the definition of __to_address, to
use pointer_traits, or find user-defined overloads by ADL, or
something else.

(*) there's an active discussion on the LEWG reflector, and Glen said
he's revising the paper.

Comments

Glen Fernandes July 20, 2017, 6:31 p.m. UTC | #1
On Thu, Jul 20, 2017 at 12:53 PM, Jonathan Wakely wrote:
> We have a more general problem with this, which is that if it's only
> available for C++2a mode then we can't use the new feature in most of
> the library. Which would be very unfortunate. I want to use this!
>
> In order to clean up the various places in the library that
> could/should use to_address I think we need our own version of it.
> Something like our std::__addressof which can be used even for C++98,
> and is used by the implementation of the C++11 std::addressof.
>
> The attached patch does something like that. I think I'll commit this,
> as it's a small improvement over what we have today. As the design of
> P0653 evolves (*) we can revisit the definition of __to_address, to
> use pointer_traits, or find user-defined overloads by ADL, or
> something else.
>
> (*) there's an active discussion on the LEWG reflector, and Glen said
> he's revising the paper.

Sounds good - thanks Jonathan.

I'll revise the patch when P0653 evolves (and approaches acceptance),
and when I do, I'll follow the suggestions and style that you've
mentioned above.

Glen
diff mbox

Patch

commit da6aa2c4ba47f91127c3caeb7a851d086b16f3bc
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Jul 20 16:19:53 2017 +0100

    Define std::__to_address helper
    
    	* include/bits/allocated_ptr.h (__allocated_ptr::get): Use
    	__to_address.
    	(__allocated_ptr::_S_raw_ptr): Remove.
    	* include/bits/ptr_traits.h (__to_address): Define new function
    	template.
    	* include/bits/stl_vector.h [__cplusplus >= 201103l]
    	(vector::_M_data_ptr): Use __to_address.

diff --git a/libstdc++-v3/include/bits/allocated_ptr.h b/libstdc++-v3/include/bits/allocated_ptr.h
index 773b3f5..96d7841 100644
--- a/libstdc++-v3/include/bits/allocated_ptr.h
+++ b/libstdc++-v3/include/bits/allocated_ptr.h
@@ -82,16 +82,9 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       }
 
       /// Get the address that the owned pointer refers to.
-      value_type* get() { return _S_raw_ptr(_M_ptr); }
+      value_type* get() { return std::__to_address(_M_ptr); }
 
     private:
-      static value_type* _S_raw_ptr(value_type* __ptr) { return __ptr; }
-
-      template<typename _Ptr>
-	static auto
-	_S_raw_ptr(_Ptr __ptr) -> decltype(_S_raw_ptr(__ptr.operator->()))
-	{ return _S_raw_ptr(__ptr.operator->()); }
-
       _Alloc* _M_alloc;
       pointer _M_ptr;
     };
diff --git a/libstdc++-v3/include/bits/ptr_traits.h b/libstdc++-v3/include/bits/ptr_traits.h
index 797e7fc..51c4a20 100644
--- a/libstdc++-v3/include/bits/ptr_traits.h
+++ b/libstdc++-v3/include/bits/ptr_traits.h
@@ -146,6 +146,17 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template<typename _Ptr, typename _Tp>
     using __ptr_rebind = typename pointer_traits<_Ptr>::template rebind<_Tp>;
 
+  template<typename _Tp>
+    constexpr _Tp*
+    __to_address(_Tp* __ptr) noexcept
+    { return __ptr; }
+
+  template<typename _Ptr>
+    inline _GLIBCXX14_CONSTEXPR
+    typename std::pointer_traits<_Ptr>::element_type*
+    __to_address(const _Ptr& __ptr)
+    { return std::__to_address(__ptr.operator->()); }
+
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace std
 
diff --git a/libstdc++-v3/include/bits/stl_vector.h b/libstdc++-v3/include/bits/stl_vector.h
index 7ee3ce9..49f6974 100644
--- a/libstdc++-v3/include/bits/stl_vector.h
+++ b/libstdc++-v3/include/bits/stl_vector.h
@@ -1561,7 +1561,7 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       template<typename _Ptr>
 	typename std::pointer_traits<_Ptr>::element_type*
 	_M_data_ptr(_Ptr __ptr) const
-	{ return empty() ? nullptr : std::__addressof(*__ptr); }
+	{ return empty() ? nullptr : std::__to_address(__ptr); }
 #else
       template<typename _Up>
 	_Up*