[committed] libstdc++: Specialize std::pointer_traits<__normal_iterator<I,C>>

[committed] libstdc++: Specialize std::pointer_traits<__normal_iterator<I,C>>

[Jonathan Wakely]

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

This allows std::__to_address to be used with __normal_iterator in
C++11/14/17 modes. Without the partial specialization the deduced
pointer_traits::element_type is incorrect, and so the return type of
__to_address is wrong.

A similar partial specialization is probably needed for
__gnu_debug::_Safe_iterator.

Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

libstdc++-v3/ChangeLog:

	* include/bits/stl_iterator.h (pointer_traits): Define partial
	specialization for __normal_iterator.
	* testsuite/24_iterators/normal_iterator/to_address.cc: New test.

Tested x86_64-linux. Committed to trunk.


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

commit 82626be2d633a9802a8b08727ef51c627e37fee5
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Sep 28 15:26:46 2021

    libstdc++: Specialize std::pointer_traits<__normal_iterator<I,C>>
    
    This allows std::__to_address to be used with __normal_iterator in
    C++11/14/17 modes. Without the partial specialization the deduced
    pointer_traits::element_type is incorrect, and so the return type of
    __to_address is wrong.
    
    A similar partial specialization is probably needed for
    __gnu_debug::_Safe_iterator.
    
    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            * include/bits/stl_iterator.h (pointer_traits): Define partial
            specialization for __normal_iterator.
            * testsuite/24_iterators/normal_iterator/to_address.cc: New test.

diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h
index c5b02408c1c..004d767224d 100644
--- a/libstdc++-v3/include/bits/stl_iterator.h
+++ b/libstdc++-v3/include/bits/stl_iterator.h
@@ -1285,6 +1285,34 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     { return __it.base(); }
 
 #if __cplusplus >= 201103L
+
+  // Need to specialize pointer_traits because the primary template will
+  // deduce element_type of __normal_iterator<T*, C> as T* rather than T.
+  template<typename _Iterator, typename _Container>
+    struct pointer_traits<__gnu_cxx::__normal_iterator<_Iterator, _Container>>
+    {
+    private:
+      using _Base = pointer_traits<_Iterator>;
+
+    public:
+      using element_type = typename _Base::element_type;
+      using pointer = __gnu_cxx::__normal_iterator<_Iterator, _Container>;
+      using difference_type = typename _Base::difference_type;
+
+      template<typename _Tp>
+	using rebind = __gnu_cxx::__normal_iterator<_Tp, _Container>;
+
+      static pointer
+      pointer_to(element_type& __e) noexcept
+      { return pointer(_Base::pointer_to(__e)); }
+
+#if __cplusplus >= 202002L
+      static element_type*
+      to_address(pointer __p) noexcept
+      { return __p.base(); }
+#endif
+    };
+
   /**
    * @addtogroup iterators
    * @{
diff --git a/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc b/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc
new file mode 100644
index 00000000000..510d627435f
--- /dev/null
+++ b/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc
@@ -0,0 +1,6 @@
+// { dg-do compile { target { c++11 } } }
+#include <string>
+#include <memory>
+
+char* p = std::__to_address(std::string("1").begin());
+const char* q = std::__to_address(std::string("2").cbegin());

Re: [committed] libstdc++: Specialize std::pointer_traits<__normal_iterator<I,C>>

[François Dumont]

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

Here is the _Safe_iterator one.

Doing so I noticed that pointer_traits rebind for __normal_iterator was 
wrong and added tests on it.

For _Safe_iterator maybe I should specialize only when instantiated with 
__normal_iterator ? Or maybe limit to random_access_iterator_tag ?

Whatever the pointer_to implementation is problematic, we can only 
produce singular iterator as I did ifor now.

François

On 28/09/21 9:25 pm, Jonathan Wakely via Libstdc++ wrote:
> This allows std::__to_address to be used with __normal_iterator in
> C++11/14/17 modes. Without the partial specialization the deduced
> pointer_traits::element_type is incorrect, and so the return type of
> __to_address is wrong.
>
> A similar partial specialization is probably needed for
> __gnu_debug::_Safe_iterator.
>
> Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
>
> libstdc++-v3/ChangeLog:
>
> 	* include/bits/stl_iterator.h (pointer_traits): Define partial
> 	specialization for __normal_iterator.
> 	* testsuite/24_iterators/normal_iterator/to_address.cc: New test.
>
> Tested x86_64-linux. Committed to trunk.
>


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

diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h
index 004d767224d..f7e851718c1 100644
--- a/libstdc++-v3/include/bits/stl_iterator.h
+++ b/libstdc++-v3/include/bits/stl_iterator.h
@@ -1294,13 +1294,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     private:
       using _Base = pointer_traits<_Iterator>;
 
+      template<typename _Tp>
+	using __base_rebind = typename _Base::template rebind<_Tp>;
+
     public:
       using element_type = typename _Base::element_type;
       using pointer = __gnu_cxx::__normal_iterator<_Iterator, _Container>;
       using difference_type = typename _Base::difference_type;
 
       template<typename _Tp>
-	using rebind = __gnu_cxx::__normal_iterator<_Tp, _Container>;
+	using rebind =
+	  __gnu_cxx::__normal_iterator<__base_rebind<_Tp>, _Container>;
 
       static pointer
       pointer_to(element_type& __e) noexcept
diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h
index 5584d06de5a..5461d2b342f 100644
--- a/libstdc++-v3/include/debug/safe_iterator.h
+++ b/libstdc++-v3/include/debug/safe_iterator.h
@@ -1013,6 +1013,44 @@ namespace __gnu_debug
 
 } // namespace __gnu_debug
 
+#if __cplusplus >= 201103L
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+  template<typename _Iterator, typename _Sequence, typename _Category>
+    struct pointer_traits<__gnu_debug::_Safe_iterator<_Iterator, _Sequence,
+						      _Category>>
+    {
+    private:
+      using _Base = pointer_traits<_Iterator>;
+
+      template<typename _Tp>
+	using __base_rebind = typename _Base::template rebind<_Tp>;
+
+    public:
+      using element_type = typename _Base::element_type;
+      using pointer =
+	__gnu_debug::_Safe_iterator<_Iterator, _Sequence, _Category>;
+      using difference_type = typename _Base::difference_type;
+
+      template<typename _Tp>
+	using rebind =
+	  __gnu_debug::_Safe_iterator<__base_rebind<_Tp>, _Sequence, _Category>;
+
+      static pointer
+      pointer_to(element_type& __e) noexcept
+      { return pointer(_Base::pointer_to(__e), nullptr); }
+
+#if __cplusplus >= 202002L
+      static element_type*
+      to_address(pointer __p) noexcept
+      { return __p.base(); }
+#endif
+    };
+_GLIBCXX_END_NAMESPACE_VERSION
+} // namespace
+#endif
+
 #undef _GLIBCXX_DEBUG_VERIFY_DIST_OPERANDS
 #undef _GLIBCXX_DEBUG_VERIFY_REL_OPERANDS
 #undef _GLIBCXX_DEBUG_VERIFY_EQ_OPERANDS
diff --git a/libstdc++-v3/testsuite/20_util/pointer_traits/rebind.cc b/libstdc++-v3/testsuite/20_util/pointer_traits/rebind.cc
index 159ea8f5294..b78e974d777 100644
--- a/libstdc++-v3/testsuite/20_util/pointer_traits/rebind.cc
+++ b/libstdc++-v3/testsuite/20_util/pointer_traits/rebind.cc
@@ -18,6 +18,7 @@
 // { dg-do compile { target c++11 } }
 
 #include <memory>
+#include <vector>
 
 using std::is_same;
 
@@ -66,3 +67,13 @@ template<typename T, int = 0>
   };
 // PR libstdc++/72793 specialization of pointer_traits is still well-formed:
 std::pointer_traits<CannotRebind<int>>::element_type e;
+
+static_assert(is_same<typename std::pointer_traits<
+	      Rebind<typename std::vector<int>::iterator, long>>::element_type,
+	      long>::value,
+	      "iterator rebind");
+
+static_assert(is_same<typename std::pointer_traits<
+	      Rebind<typename std::vector<int>::const_iterator, long>>::element_type,
+	      long>::value,
+	      "const_iterator rebind");
diff --git a/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc b/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc
index 510d627435f..433c803beb1 100644
--- a/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc
+++ b/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc
@@ -1,6 +1,9 @@
 // { dg-do compile { target { c++11 } } }
 #include <string>
+#include <vector>
 #include <memory>
 
 char* p = std::__to_address(std::string("1").begin());
 const char* q = std::__to_address(std::string("2").cbegin());
+int* r = std::__to_address(std::vector<int>(1, 1).begin());
+const int* s = std::__to_address(std::vector<int>(1, 1).cbegin());

Re: [committed] libstdc++: Specialize std::pointer_traits<__normal_iterator<I, C>>

[Jonathan Wakely]

On Thu, 30 Sept 2021 at 21:27, François Dumont via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> Here is the _Safe_iterator one.
>
> Doing so I noticed that pointer_traits rebind for __normal_iterator was
> wrong and added tests on it.

Oops, thanks!

> For _Safe_iterator maybe I should specialize only when instantiated with
> __normal_iterator ? Or maybe limit to random_access_iterator_tag ?

Yes, I think that's a good idea, maybe only for __normal_iterator<T*,
C>. I think that should cover all the cases we actually need to
support.

And could you change the existing pointer_traits<__normal_iterator<I,
C>> partial specialization to only be valid for
__normal_iterator<T*,C> as well? i.e. only for __normal_iterator that
wraps a pointer (which is true for string::iterator, vector::iterator
etc.)

> Whatever the pointer_to implementation is problematic, we can only
> produce singular iterator as I did ifor now.

Hmm, yes. I think that's OK, because I don't think anybody is actually
going to use that function. What matters is that it works with
std::__to_address for C++17 mode, and that doesn't depend on
pointer_to, so it's OK if it doesn't work usefully.

Thanks!

Re: [committed] libstdc++: Specialize std::pointer_traits<__normal_iterator<I,C>>

[François Dumont]

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

On 02/10/21 12:29 am, Jonathan Wakely wrote:
> On Thu, 30 Sept 2021 at 21:27, François Dumont via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
>> Here is the _Safe_iterator one.
>>
>> Doing so I noticed that pointer_traits rebind for __normal_iterator was
>> wrong and added tests on it.
> Oops, thanks!
>
>> For _Safe_iterator maybe I should specialize only when instantiated with
>> __normal_iterator ? Or maybe limit to random_access_iterator_tag ?
> Yes, I think that's a good idea, maybe only for __normal_iterator<T*,
> C>. I think that should cover all the cases we actually need to
> support.
>
> And could you change the existing pointer_traits<__normal_iterator<I,
> C>> partial specialization to only be valid for
> __normal_iterator<T*,C> as well? i.e. only for __normal_iterator that
> wraps a pointer (which is true for string::iterator, vector::iterator
> etc.)

Is the attached patch what you had in mind ?

At least it is passing tests.


>
>> Whatever the pointer_to implementation is problematic, we can only
>> produce singular iterator as I did ifor now.
> Hmm, yes. I think that's OK, because I don't think anybody is actually
> going to use that function. What matters is that it works with
> std::__to_address for C++17 mode, and that doesn't depend on
> pointer_to, so it's OK if it doesn't work usefully.
>
This is why I think that my patch of the unordered containers to support 
custom pointers is not so bad because it avoids usage of 
pointer_traits::to_pointer :-)



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

diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h
index 8afd6756613..f6dbe562505 100644
--- a/libstdc++-v3/include/bits/stl_iterator.h
+++ b/libstdc++-v3/include/bits/stl_iterator.h
@@ -1314,23 +1314,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   // Need to specialize pointer_traits because the primary template will
   // deduce element_type of __normal_iterator<T*, C> as T* rather than T.
-  template<typename _Iterator, typename _Container>
-    struct pointer_traits<__gnu_cxx::__normal_iterator<_Iterator, _Container>>
+  template<typename _Tp, typename _Container>
+    struct pointer_traits<__gnu_cxx::__normal_iterator<_Tp*, _Container>>
     {
-    private:
-      using _Base = pointer_traits<_Iterator>;
-
-    public:
-      using element_type = typename _Base::element_type;
-      using pointer = __gnu_cxx::__normal_iterator<_Iterator, _Container>;
-      using difference_type = typename _Base::difference_type;
+      using element_type = _Tp;
+      using pointer = __gnu_cxx::__normal_iterator<_Tp*, _Container>;
+      using difference_type = ptrdiff_t;
 
-      template<typename _Tp>
-	using rebind = __gnu_cxx::__normal_iterator<_Tp, _Container>;
+      template<typename _Up>
+	using rebind = __gnu_cxx::__normal_iterator<_Up*, _Container>;
 
-      static pointer
-      pointer_to(element_type& __e) noexcept
-      { return pointer(_Base::pointer_to(__e)); }
+      static _GLIBCXX20_CONSTEXPR pointer
+      pointer_to(__make_not_void<element_type>& __r) noexcept
+      { return pointer(std::addressof(__r)); }
 
 #if __cplusplus >= 202002L
       static element_type*
diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h
index 5584d06de5a..d2469574240 100644
--- a/libstdc++-v3/include/debug/safe_iterator.h
+++ b/libstdc++-v3/include/debug/safe_iterator.h
@@ -1013,6 +1013,45 @@ namespace __gnu_debug
 
 } // namespace __gnu_debug
 
+#if __cplusplus >= 201103L
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+  template<typename _Tp, typename _Container, typename _Sequence>
+    struct pointer_traits<__gnu_debug::_Safe_iterator<
+      __gnu_cxx::__normal_iterator<_Tp*, _Container>, _Sequence,
+      std::random_access_iterator_tag>>
+    {
+    private:
+      using __base_ptr =
+	__gnu_cxx::__normal_iterator<_Tp*, _Container>;
+
+    public:
+      using element_type = _Tp;
+      using pointer = __gnu_debug::_Safe_iterator<
+	__gnu_cxx::__normal_iterator<_Tp*, _Container>, _Sequence,
+	std::random_access_iterator_tag>;
+      using difference_type = ptrdiff_t;
+
+      template<typename _Up>
+	using rebind = __gnu_debug::_Safe_iterator<
+	  __gnu_cxx::__normal_iterator<_Up*, _Container>, _Sequence,
+	  std::random_access_iterator_tag>;
+
+      static pointer
+      pointer_to(element_type& __e) noexcept
+      { return { __base_ptr(std::addressof(__e)), nullptr }; }
+
+#if __cplusplus >= 202002L
+      static element_type*
+      to_address(const pointer& __p) noexcept
+      { return __p.base().base(); }
+#endif
+    };
+_GLIBCXX_END_NAMESPACE_VERSION
+} // namespace
+#endif
+
 #undef _GLIBCXX_DEBUG_VERIFY_DIST_OPERANDS
 #undef _GLIBCXX_DEBUG_VERIFY_REL_OPERANDS
 #undef _GLIBCXX_DEBUG_VERIFY_EQ_OPERANDS
diff --git a/libstdc++-v3/testsuite/20_util/pointer_traits/rebind.cc b/libstdc++-v3/testsuite/20_util/pointer_traits/rebind.cc
index 159ea8f5294..b78e974d777 100644
--- a/libstdc++-v3/testsuite/20_util/pointer_traits/rebind.cc
+++ b/libstdc++-v3/testsuite/20_util/pointer_traits/rebind.cc
@@ -18,6 +18,7 @@
 // { dg-do compile { target c++11 } }
 
 #include <memory>
+#include <vector>
 
 using std::is_same;
 
@@ -66,3 +67,13 @@ template<typename T, int = 0>
   };
 // PR libstdc++/72793 specialization of pointer_traits is still well-formed:
 std::pointer_traits<CannotRebind<int>>::element_type e;
+
+static_assert(is_same<typename std::pointer_traits<
+	      Rebind<typename std::vector<int>::iterator, long>>::element_type,
+	      long>::value,
+	      "iterator rebind");
+
+static_assert(is_same<typename std::pointer_traits<
+	      Rebind<typename std::vector<int>::const_iterator, long>>::element_type,
+	      long>::value,
+	      "const_iterator rebind");
diff --git a/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc b/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc
index 510d627435f..433c803beb1 100644
--- a/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc
+++ b/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc
@@ -1,6 +1,9 @@
 // { dg-do compile { target { c++11 } } }
 #include <string>
+#include <vector>
 #include <memory>
 
 char* p = std::__to_address(std::string("1").begin());
 const char* q = std::__to_address(std::string("2").cbegin());
+int* r = std::__to_address(std::vector<int>(1, 1).begin());
+const int* s = std::__to_address(std::vector<int>(1, 1).cbegin());

Re: [committed] libstdc++: Specialize std::pointer_traits<__normal_iterator<I, C>>

[Jonathan Wakely]

On Sat, 2 Oct 2021 at 14:08, François Dumont <frs.dumont@gmail.com> wrote:
>
> On 02/10/21 12:29 am, Jonathan Wakely wrote:
> > On Thu, 30 Sept 2021 at 21:27, François Dumont via Libstdc++
> > <libstdc++@gcc.gnu.org> wrote:
> >> Here is the _Safe_iterator one.
> >>
> >> Doing so I noticed that pointer_traits rebind for __normal_iterator was
> >> wrong and added tests on it.
> > Oops, thanks!
> >
> >> For _Safe_iterator maybe I should specialize only when instantiated with
> >> __normal_iterator ? Or maybe limit to random_access_iterator_tag ?
> > Yes, I think that's a good idea, maybe only for __normal_iterator<T*,
> > C>. I think that should cover all the cases we actually need to
> > support.
> >
> > And could you change the existing pointer_traits<__normal_iterator<I,
> > C>> partial specialization to only be valid for
> > __normal_iterator<T*,C> as well? i.e. only for __normal_iterator that
> > wraps a pointer (which is true for string::iterator, vector::iterator
> > etc.)
>
> Is the attached patch what you had in mind ?

Yes, although I don't think we need to us __make_not_void for
to_pointer, because I don't think we would ever have
__normal_iterator<void*, C>. I don't think it will even compile,
because void* is not an iterator type, and iterator_traits<void*> is
invalid (in C++20 it's constrained with is_object_v<T> and before
C++20 it will just give an error, because void& is ill-formed).

Re: [committed] libstdc++: Specialize std::pointer_traits<__normal_iterator<I,C>>

[François Dumont]

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

I would like to propose this alternative approach.

In this patch I make __normal_iterator and random iterator 
_Safe_iterator compatible for pointer_traits primary template.

Regarding pointer_traits I wonder if it shouldn't check for the 
to_pointer method availability and use per default: return { 
std::addressof(__e) }; otherwise. This way we wouldn't have to provide a 
pointer_to method on __normal_iterator.

François

On 02/10/21 12:29 am, Jonathan Wakely wrote:
> On Thu, 30 Sept 2021 at 21:27, François Dumont via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
>> Here is the _Safe_iterator one.
>>
>> Doing so I noticed that pointer_traits rebind for __normal_iterator was
>> wrong and added tests on it.
> Oops, thanks!
>
>> For _Safe_iterator maybe I should specialize only when instantiated with
>> __normal_iterator ? Or maybe limit to random_access_iterator_tag ?
> Yes, I think that's a good idea, maybe only for __normal_iterator<T*,
> C>. I think that should cover all the cases we actually need to
> support.
>
> And could you change the existing pointer_traits<__normal_iterator<I,
> C>> partial specialization to only be valid for
> __normal_iterator<T*,C> as well? i.e. only for __normal_iterator that
> wraps a pointer (which is true for string::iterator, vector::iterator
> etc.)
>
>> Whatever the pointer_to implementation is problematic, we can only
>> produce singular iterator as I did ifor now.
> Hmm, yes. I think that's OK, because I don't think anybody is actually
> going to use that function. What matters is that it works with
> std::__to_address for C++17 mode, and that doesn't depend on
> pointer_to, so it's OK if it doesn't work usefully.
>
> Thanks!
>


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

diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h
index 8afd6756613..594d4d5b65f 100644
--- a/libstdc++-v3/include/bits/stl_iterator.h
+++ b/libstdc++-v3/include/bits/stl_iterator.h
@@ -1017,6 +1017,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template<typename _Iterator, typename _Container>
     class __normal_iterator
     {
+#if __cplusplus >= 201103L
+      using __ite_ptr_traits = std::pointer_traits<_Iterator>;
+
+      template<typename _Tp>
+	using __ite_rebind = typename __ite_ptr_traits::template rebind<_Tp>;
+#endif
+
     protected:
       _Iterator _M_current;
 
@@ -1036,6 +1043,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       typedef typename __traits_type::reference 	reference;
       typedef typename __traits_type::pointer   	pointer;
 
+#if __cplusplus >= 201103L
+      using element_type = typename __ite_ptr_traits::element_type;
+
+      template<typename _Up>
+	using rebind = __normal_iterator<__ite_rebind<_Up>, _Container>;
+
+      static __normal_iterator
+      pointer_to(element_type& __e)
+      { return { __ite_ptr_traits::pointer_to(__e) }; }
+#endif
+
 #if __cplusplus > 201703L && __cpp_lib_concepts
       using iterator_concept = std::__detail::__iter_concept<_Iterator>;
 #endif
@@ -1311,34 +1329,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     { return __it.base(); }
 
 #if __cplusplus >= 201103L
-
-  // Need to specialize pointer_traits because the primary template will
-  // deduce element_type of __normal_iterator<T*, C> as T* rather than T.
-  template<typename _Iterator, typename _Container>
-    struct pointer_traits<__gnu_cxx::__normal_iterator<_Iterator, _Container>>
-    {
-    private:
-      using _Base = pointer_traits<_Iterator>;
-
-    public:
-      using element_type = typename _Base::element_type;
-      using pointer = __gnu_cxx::__normal_iterator<_Iterator, _Container>;
-      using difference_type = typename _Base::difference_type;
-
-      template<typename _Tp>
-	using rebind = __gnu_cxx::__normal_iterator<_Tp, _Container>;
-
-      static pointer
-      pointer_to(element_type& __e) noexcept
-      { return pointer(_Base::pointer_to(__e)); }
-
-#if __cplusplus >= 202002L
-      static element_type*
-      to_address(pointer __p) noexcept
-      { return __p.base(); }
-#endif
-    };
-
   /**
    * @addtogroup iterators
    * @{
diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h
index 5584d06de5a..b175c9e4a31 100644
--- a/libstdc++-v3/include/debug/safe_iterator.h
+++ b/libstdc++-v3/include/debug/safe_iterator.h
@@ -653,6 +653,13 @@ namespace __gnu_debug
     : public _Safe_iterator<_Iterator, _Sequence,
 			    std::bidirectional_iterator_tag>
     {
+#if __cplusplus >= 201103L
+      using __ite_ptr_traits = std::pointer_traits<_Iterator>;
+
+      template<typename _Tp>
+	using __ite_rebind = typename __ite_ptr_traits::template rebind<_Tp>;
+#endif
+
       typedef _Safe_iterator<_Iterator, _Sequence,
 			     std::bidirectional_iterator_tag> _Safe_base;
       typedef typename _Safe_base::_OtherIterator _OtherIterator;
@@ -672,6 +679,19 @@ namespace __gnu_debug
       typedef typename _Safe_base::difference_type	difference_type;
       typedef typename _Safe_base::reference		reference;
 
+
+#if __cplusplus >= 201103L
+      using element_type = typename __ite_ptr_traits::element_type;
+
+      template<typename _Up>
+	using rebind = _Safe_iterator<__ite_rebind<_Up>, _Sequence,
+				      std::random_access_iterator_tag>;
+
+      static _Safe_iterator
+      pointer_to(element_type& __e)
+      { return { __ite_ptr_traits::pointer_to(__e), nullptr }; }
+#endif
+
       /// @post the iterator is singular and unattached
       _Safe_iterator() _GLIBCXX_NOEXCEPT { }
 
diff --git a/libstdc++-v3/testsuite/20_util/pointer_traits/rebind.cc b/libstdc++-v3/testsuite/20_util/pointer_traits/rebind.cc
index 159ea8f5294..b78e974d777 100644
--- a/libstdc++-v3/testsuite/20_util/pointer_traits/rebind.cc
+++ b/libstdc++-v3/testsuite/20_util/pointer_traits/rebind.cc
@@ -18,6 +18,7 @@
 // { dg-do compile { target c++11 } }
 
 #include <memory>
+#include <vector>
 
 using std::is_same;
 
@@ -66,3 +67,13 @@ template<typename T, int = 0>
   };
 // PR libstdc++/72793 specialization of pointer_traits is still well-formed:
 std::pointer_traits<CannotRebind<int>>::element_type e;
+
+static_assert(is_same<typename std::pointer_traits<
+	      Rebind<typename std::vector<int>::iterator, long>>::element_type,
+	      long>::value,
+	      "iterator rebind");
+
+static_assert(is_same<typename std::pointer_traits<
+	      Rebind<typename std::vector<int>::const_iterator, long>>::element_type,
+	      long>::value,
+	      "const_iterator rebind");
diff --git a/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc b/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc
index 510d627435f..433c803beb1 100644
--- a/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc
+++ b/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc
@@ -1,6 +1,9 @@
 // { dg-do compile { target { c++11 } } }
 #include <string>
+#include <vector>
 #include <memory>
 
 char* p = std::__to_address(std::string("1").begin());
 const char* q = std::__to_address(std::string("2").cbegin());
+int* r = std::__to_address(std::vector<int>(1, 1).begin());
+const int* s = std::__to_address(std::vector<int>(1, 1).cbegin());

Re: [committed] libstdc++: Specialize std::pointer_traits<__normal_iterator<I, C>>

[Jonathan Wakely]

On Sat, 2 Oct 2021 at 18:27, François Dumont wrote:
>
> I would like to propose this alternative approach.
>
> In this patch I make __normal_iterator and random iterator
> _Safe_iterator compatible for pointer_traits primary template.
>
> Regarding pointer_traits I wonder if it shouldn't check for the
> to_pointer method availability and use per default: return {
> std::addressof(__e) }; otherwise. This way we wouldn't have to provide a
> pointer_to method on __normal_iterator.

But I would rather not have these members present in vector::iterator
and string::iterator, in case users accidentally start to rely on them
being present.

Another option would be to overload std::__to_address so it knows how
to get the address from __normal_iterator and _Safe_iterator.

Re: [committed] libstdc++: Specialize std::pointer_traits<__normal_iterator<I,C>>

[François Dumont]

On 02/10/21 10:24 pm, Jonathan Wakely wrote:
> On Sat, 2 Oct 2021 at 18:27, François Dumont wrote:
>> I would like to propose this alternative approach.
>>
>> In this patch I make __normal_iterator and random iterator
>> _Safe_iterator compatible for pointer_traits primary template.
>>
>> Regarding pointer_traits I wonder if it shouldn't check for the
>> to_pointer method availability and use per default: return {
>> std::addressof(__e) }; otherwise. This way we wouldn't have to provide a
>> pointer_to method on __normal_iterator.
> But I would rather not have these members present in vector::iterator
> and string::iterator, in case users accidentally start to rely on them
> being present.

Making pointer_traits friends would help but I do not like it neither.


>
> Another option would be to overload std::__to_address so it knows how
> to get the address from __normal_iterator and _Safe_iterator.
>
> .

I start thinking that rather than proposing not-useful and even 
incorrect code in the case of the _Safe_iterator<> it might be a better 
approach.

Even the rebind for __normal_iterator is a little strange because when 
doing rebind on std::vector<int>::iterator for long it produces 
__normal_iterator<long*, std::vector<int>>, quite inconsistent even if 
useless.

But there's something that I'm missing, what is the relation between 
__addressof and std::pointer_traits ? Is it that __builtin_addressof is 
using it ?

Re: [committed] libstdc++: Specialize std::pointer_traits<__normal_iterator<I,C>>

[François Dumont]

On 04/10/21 10:05 pm, François Dumont wrote:
> On 02/10/21 10:24 pm, Jonathan Wakely wrote:
>> On Sat, 2 Oct 2021 at 18:27, François Dumont wrote:
>>> I would like to propose this alternative approach.
>>>
>>> In this patch I make __normal_iterator and random iterator
>>> _Safe_iterator compatible for pointer_traits primary template.
>>>
>>> Regarding pointer_traits I wonder if it shouldn't check for the
>>> to_pointer method availability and use per default: return {
>>> std::addressof(__e) }; otherwise. This way we wouldn't have to 
>>> provide a
>>> pointer_to method on __normal_iterator.
>> But I would rather not have these members present in vector::iterator
>> and string::iterator, in case users accidentally start to rely on them
>> being present.
>
> Making pointer_traits friends would help but I do not like it neither.
>
>
>>
>> Another option would be to overload std::__to_address so it knows how
>> to get the address from __normal_iterator and _Safe_iterator.
>>
>> .
>
> I start thinking that rather than proposing not-useful and even 
> incorrect code in the case of the _Safe_iterator<> it might be a 
> better approach.
>
> Even the rebind for __normal_iterator is a little strange because when 
> doing rebind on std::vector<int>::iterator for long it produces 
> __normal_iterator<long*, std::vector<int>>, quite inconsistent even if 
> useless.
>
> But there's something that I'm missing, what is the relation between 
> __addressof and std::pointer_traits ? Is it that __builtin_addressof 
> is using it ?
>
Ignore this last question, I realized that we are talking about 
__to_address, not __addressof.

Re: [committed] libstdc++: Specialize std::pointer_traits<__normal_iterator<I, C>>

[Jonathan Wakely]

On Mon, 4 Oct 2021 at 21:28, François Dumont via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> On 04/10/21 10:05 pm, François Dumont wrote:
> > On 02/10/21 10:24 pm, Jonathan Wakely wrote:
> >> On Sat, 2 Oct 2021 at 18:27, François Dumont wrote:
> >>> I would like to propose this alternative approach.
> >>>
> >>> In this patch I make __normal_iterator and random iterator
> >>> _Safe_iterator compatible for pointer_traits primary template.
> >>>
> >>> Regarding pointer_traits I wonder if it shouldn't check for the
> >>> to_pointer method availability and use per default: return {
> >>> std::addressof(__e) }; otherwise. This way we wouldn't have to
> >>> provide a
> >>> pointer_to method on __normal_iterator.
> >> But I would rather not have these members present in vector::iterator
> >> and string::iterator, in case users accidentally start to rely on them
> >> being present.
> >
> > Making pointer_traits friends would help but I do not like it neither.
> >
> >
> >>
> >> Another option would be to overload std::__to_address so it knows how
> >> to get the address from __normal_iterator and _Safe_iterator.
> >>
> >> .
> >
> > I start thinking that rather than proposing not-useful and even
> > incorrect code in the case of the _Safe_iterator<> it might be a
> > better approach.
> >
> > Even the rebind for __normal_iterator is a little strange because when
> > doing rebind on std::vector<int>::iterator for long it produces
> > __normal_iterator<long*, std::vector<int>>, quite inconsistent even if
> > useless.
> >
> > But there's something that I'm missing, what is the relation between
> > __addressof and std::pointer_traits ? Is it that __builtin_addressof
> > is using it ?
> >
> Ignore this last question, I realized that we are talking about
> __to_address, not __addressof.

Yes, see the definition of std::__to_address in <bits/ptr_traits.h>
and also PR 96416.

Re: [committed] libstdc++: Specialize std::pointer_traits<__normal_iterator<I,C>>

[François Dumont]

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

Here is another proposal with the __to_address overload.

I preferred to let it open to any kind of __normal_iterator 
instantiation cause afaics std::vector supports fancy pointer types. It 
is better if __to_address works fine also in this case, no ?

     libstdc++: Overload std::__to_address for __gnu_cxx::__normal_iterator.

     Prefer to overload __to_address to partially specialize 
std::pointer_traits because
     std::pointer_traits would be mostly useless. In the case of 
__gnu_debug::_Safe_iterator
     the to_pointer method is even impossible to implement correctly 
because we are missing
     the parent container to associate the iterator to.

     libstdc++-v3/ChangeLog:

             * include/bits/stl_iterator.h
(std::pointer_traits<__gnu_cxx::__normal_iterator<>>): Remove.
             (std::__to_address(const __gnu_cxx::__normal_iterator<>&)): 
New.
             * include/debug/safe_iterator.h
             (std::__to_address(const 
__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<>, _Sequence>&)):
             New.
             * testsuite/24_iterators/normal_iterator/to_address.cc: Add 
check on std::vector::iterator
             to validate both __gnu_cxx::__normal_iterator<> 
__to_address overload in normal mode and the

Tested under Linux x86_64.

Ok to commit ?

François


On 04/10/21 10:30 pm, Jonathan Wakely wrote:
> On Mon, 4 Oct 2021 at 21:28, François Dumont via Libstdc++
> <libstdc++@gcc.gnu.org>  wrote:
>> On 04/10/21 10:05 pm, François Dumont wrote:
>>> On 02/10/21 10:24 pm, Jonathan Wakely wrote:
>>>> On Sat, 2 Oct 2021 at 18:27, François Dumont wrote:
>>>>> I would like to propose this alternative approach.
>>>>>
>>>>> In this patch I make __normal_iterator and random iterator
>>>>> _Safe_iterator compatible for pointer_traits primary template.
>>>>>
>>>>> Regarding pointer_traits I wonder if it shouldn't check for the
>>>>> to_pointer method availability and use per default: return {
>>>>> std::addressof(__e) }; otherwise. This way we wouldn't have to
>>>>> provide a
>>>>> pointer_to method on __normal_iterator.
>>>> But I would rather not have these members present in vector::iterator
>>>> and string::iterator, in case users accidentally start to rely on them
>>>> being present.
>>> Making pointer_traits friends would help but I do not like it neither.
>>>
>>>
>>>> Another option would be to overload std::__to_address so it knows how
>>>> to get the address from __normal_iterator and _Safe_iterator.
>>>>
>>>> .
>>> I start thinking that rather than proposing not-useful and even
>>> incorrect code in the case of the _Safe_iterator<> it might be a
>>> better approach.
>>>
>>> Even the rebind for __normal_iterator is a little strange because when
>>> doing rebind on std::vector<int>::iterator for long it produces
>>> __normal_iterator<long*, std::vector<int>>, quite inconsistent even if
>>> useless.
>>>
>>>

[-- Attachment #2: to_address.patch --]
[-- Type: text/x-patch, Size: 3255 bytes --]

diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h
index 8afd6756613..6b915ec011c 100644
--- a/libstdc++-v3/include/bits/stl_iterator.h
+++ b/libstdc++-v3/include/bits/stl_iterator.h
@@ -1312,32 +1312,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 #if __cplusplus >= 201103L
 
-  // Need to specialize pointer_traits because the primary template will
-  // deduce element_type of __normal_iterator<T*, C> as T* rather than T.
+  // Need to overload __to_address because the pointer_traits primary template
+  // will deduce element_type of __normal_iterator<T*, C> as T* rather than T.
   template<typename _Iterator, typename _Container>
-    struct pointer_traits<__gnu_cxx::__normal_iterator<_Iterator, _Container>>
-    {
-    private:
-      using _Base = pointer_traits<_Iterator>;
-
-    public:
-      using element_type = typename _Base::element_type;
-      using pointer = __gnu_cxx::__normal_iterator<_Iterator, _Container>;
-      using difference_type = typename _Base::difference_type;
-
-      template<typename _Tp>
-	using rebind = __gnu_cxx::__normal_iterator<_Tp, _Container>;
-
-      static pointer
-      pointer_to(element_type& __e) noexcept
-      { return pointer(_Base::pointer_to(__e)); }
-
-#if __cplusplus >= 202002L
-      static element_type*
-      to_address(pointer __p) noexcept
-      { return __p.base(); }
-#endif
-    };
+    constexpr auto
+    __to_address(const __gnu_cxx::__normal_iterator<_Iterator,
+						    _Container>& __it) noexcept
+    -> decltype(std::__to_address(__it.base()))
+    { return std::__to_address(__it.base()); }
 
   /**
    * @addtogroup iterators
diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h
index 5584d06de5a..09e35f79067 100644
--- a/libstdc++-v3/include/debug/safe_iterator.h
+++ b/libstdc++-v3/include/debug/safe_iterator.h
@@ -1018,6 +1018,23 @@ namespace __gnu_debug
 #undef _GLIBCXX_DEBUG_VERIFY_EQ_OPERANDS
 #undef _GLIBCXX_DEBUG_VERIFY_OPERANDS
 
+#if __cplusplus >= 201103L
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+  template<typename _Iterator, typename _Container, typename _Sequence>
+    constexpr auto
+    __to_address(const __gnu_debug::_Safe_iterator<
+		 __gnu_cxx::__normal_iterator<_Iterator, _Container>,
+		 _Sequence>& __it) noexcept
+    -> decltype(std::__to_address(__it.base()))
+    { return std::__to_address(__it.base()); }
+
+_GLIBCXX_END_NAMESPACE_VERSION
+}
+#endif
+
 #include <debug/safe_iterator.tcc>
 
 #endif
diff --git a/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc b/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc
index 510d627435f..433c803beb1 100644
--- a/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc
+++ b/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc
@@ -1,6 +1,9 @@
 // { dg-do compile { target { c++11 } } }
 #include <string>
+#include <vector>
 #include <memory>
 
 char* p = std::__to_address(std::string("1").begin());
 const char* q = std::__to_address(std::string("2").cbegin());
+int* r = std::__to_address(std::vector<int>(1, 1).begin());
+const int* s = std::__to_address(std::vector<int>(1, 1).cbegin());

Re: [committed] libstdc++: Specialize std::pointer_traits<__normal_iterator<I,C>>

[François Dumont]

I forgot to ask if with this patch this overload:

   template<typename _Ptr, typename... _None>
     constexpr auto
     __to_address(const _Ptr& __ptr, _None...) noexcept
     {
       if constexpr (is_base_of_v<__gnu_debug::_Safe_iterator_base, _Ptr>)
     return std::__to_address(__ptr.base().operator->());
       else
     return std::__to_address(__ptr.operator->());
     }

should be removed ?

Or perhaps just the _Safe_iterator_base branch in it ?

On 06/10/21 7:18 pm, François Dumont wrote:
> Here is another proposal with the __to_address overload.
>
> I preferred to let it open to any kind of __normal_iterator 
> instantiation cause afaics std::vector supports fancy pointer types. 
> It is better if __to_address works fine also in this case, no ?
>
>     libstdc++: Overload std::__to_address for 
> __gnu_cxx::__normal_iterator.
>
>     Prefer to overload __to_address to partially specialize 
> std::pointer_traits because
>     std::pointer_traits would be mostly useless. In the case of 
> __gnu_debug::_Safe_iterator
>     the to_pointer method is even impossible to implement correctly 
> because we are missing
>     the parent container to associate the iterator to.
>
>     libstdc++-v3/ChangeLog:
>
>             * include/bits/stl_iterator.h
> (std::pointer_traits<__gnu_cxx::__normal_iterator<>>): Remove.
>             (std::__to_address(const 
> __gnu_cxx::__normal_iterator<>&)): New.
>             * include/debug/safe_iterator.h
>             (std::__to_address(const 
> __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<>, 
> _Sequence>&)):
>             New.
>             * testsuite/24_iterators/normal_iterator/to_address.cc: 
> Add check on std::vector::iterator
>             to validate both __gnu_cxx::__normal_iterator<> 
> __to_address overload in normal mode and the
>
> Tested under Linux x86_64.
>
> Ok to commit ?
>
> François
>
>
> On 04/10/21 10:30 pm, Jonathan Wakely wrote:
>> On Mon, 4 Oct 2021 at 21:28, François Dumont via Libstdc++
>> <libstdc++@gcc.gnu.org>  wrote:
>>> On 04/10/21 10:05 pm, François Dumont wrote:
>>>> On 02/10/21 10:24 pm, Jonathan Wakely wrote:
>>>>> On Sat, 2 Oct 2021 at 18:27, François Dumont wrote:
>>>>>> I would like to propose this alternative approach.
>>>>>>
>>>>>> In this patch I make __normal_iterator and random iterator
>>>>>> _Safe_iterator compatible for pointer_traits primary template.
>>>>>>
>>>>>> Regarding pointer_traits I wonder if it shouldn't check for the
>>>>>> to_pointer method availability and use per default: return {
>>>>>> std::addressof(__e) }; otherwise. This way we wouldn't have to
>>>>>> provide a
>>>>>> pointer_to method on __normal_iterator.
>>>>> But I would rather not have these members present in vector::iterator
>>>>> and string::iterator, in case users accidentally start to rely on 
>>>>> them
>>>>> being present.
>>>> Making pointer_traits friends would help but I do not like it neither.
>>>>
>>>>
>>>>> Another option would be to overload std::__to_address so it knows how
>>>>> to get the address from __normal_iterator and _Safe_iterator.
>>>>>
>>>>> .
>>>> I start thinking that rather than proposing not-useful and even
>>>> incorrect code in the case of the _Safe_iterator<> it might be a
>>>> better approach.
>>>>
>>>> Even the rebind for __normal_iterator is a little strange because when
>>>> doing rebind on std::vector<int>::iterator for long it produces
>>>> __normal_iterator<long*, std::vector<int>>, quite inconsistent even if
>>>> useless.
>>>>
>>>>

Re: [committed] libstdc++: Specialize std::pointer_traits<__normal_iterator<I,C>>

[François Dumont]

Hi

     Any conclusion regarding this thread ?

François


On 06/10/21 7:25 pm, François Dumont wrote:
> I forgot to ask if with this patch this overload:
>
>   template<typename _Ptr, typename... _None>
>     constexpr auto
>     __to_address(const _Ptr& __ptr, _None...) noexcept
>     {
>       if constexpr (is_base_of_v<__gnu_debug::_Safe_iterator_base, _Ptr>)
>     return std::__to_address(__ptr.base().operator->());
>       else
>     return std::__to_address(__ptr.operator->());
>     }
>
> should be removed ?
>
> Or perhaps just the _Safe_iterator_base branch in it ?
>
> On 06/10/21 7:18 pm, François Dumont wrote:
>> Here is another proposal with the __to_address overload.
>>
>> I preferred to let it open to any kind of __normal_iterator 
>> instantiation cause afaics std::vector supports fancy pointer types. 
>> It is better if __to_address works fine also in this case, no ?
>>
>>     libstdc++: Overload std::__to_address for 
>> __gnu_cxx::__normal_iterator.
>>
>>     Prefer to overload __to_address to partially specialize 
>> std::pointer_traits because
>>     std::pointer_traits would be mostly useless. In the case of 
>> __gnu_debug::_Safe_iterator
>>     the to_pointer method is even impossible to implement correctly 
>> because we are missing
>>     the parent container to associate the iterator to.
>>
>>     libstdc++-v3/ChangeLog:
>>
>>             * include/bits/stl_iterator.h
>> (std::pointer_traits<__gnu_cxx::__normal_iterator<>>): Remove.
>>             (std::__to_address(const 
>> __gnu_cxx::__normal_iterator<>&)): New.
>>             * include/debug/safe_iterator.h
>>             (std::__to_address(const 
>> __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<>, 
>> _Sequence>&)):
>>             New.
>>             * testsuite/24_iterators/normal_iterator/to_address.cc: 
>> Add check on std::vector::iterator
>>             to validate both __gnu_cxx::__normal_iterator<> 
>> __to_address overload in normal mode and the
>>
>> Tested under Linux x86_64.
>>
>> Ok to commit ?
>>
>> François
>>
>>
>> On 04/10/21 10:30 pm, Jonathan Wakely wrote:
>>> On Mon, 4 Oct 2021 at 21:28, François Dumont via Libstdc++
>>> <libstdc++@gcc.gnu.org>  wrote:
>>>> On 04/10/21 10:05 pm, François Dumont wrote:
>>>>> On 02/10/21 10:24 pm, Jonathan Wakely wrote:
>>>>>> On Sat, 2 Oct 2021 at 18:27, François Dumont wrote:
>>>>>>> I would like to propose this alternative approach.
>>>>>>>
>>>>>>> In this patch I make __normal_iterator and random iterator
>>>>>>> _Safe_iterator compatible for pointer_traits primary template.
>>>>>>>
>>>>>>> Regarding pointer_traits I wonder if it shouldn't check for the
>>>>>>> to_pointer method availability and use per default: return {
>>>>>>> std::addressof(__e) }; otherwise. This way we wouldn't have to
>>>>>>> provide a
>>>>>>> pointer_to method on __normal_iterator.
>>>>>> But I would rather not have these members present in 
>>>>>> vector::iterator
>>>>>> and string::iterator, in case users accidentally start to rely on 
>>>>>> them
>>>>>> being present.
>>>>> Making pointer_traits friends would help but I do not like it 
>>>>> neither.
>>>>>
>>>>>
>>>>>> Another option would be to overload std::__to_address so it knows 
>>>>>> how
>>>>>> to get the address from __normal_iterator and _Safe_iterator.
>>>>>>
>>>>>> .
>>>>> I start thinking that rather than proposing not-useful and even
>>>>> incorrect code in the case of the _Safe_iterator<> it might be a
>>>>> better approach.
>>>>>
>>>>> Even the rebind for __normal_iterator is a little strange because 
>>>>> when
>>>>> doing rebind on std::vector<int>::iterator for long it produces
>>>>> __normal_iterator<long*, std::vector<int>>, quite inconsistent 
>>>>> even if
>>>>> useless.
>>>>>
>>>>>
>

Re: [committed] libstdc++: Specialize std::pointer_traits<__normal_iterator<I, C>>

[Jonathan Wakely]

On Tue, 14 Dec 2021 at 06:53, François Dumont wrote:

> Hi
>
>      Any conclusion regarding this thread ?
>
> François
>
>
> On 06/10/21 7:25 pm, François Dumont wrote:
> > I forgot to ask if with this patch this overload:
> >
> >   template<typename _Ptr, typename... _None>
> >     constexpr auto
> >     __to_address(const _Ptr& __ptr, _None...) noexcept
> >     {
> >       if constexpr (is_base_of_v<__gnu_debug::_Safe_iterator_base, _Ptr>)
> >     return std::__to_address(__ptr.base().operator->());
> >       else
> >     return std::__to_address(__ptr.operator->());
> >     }
> >
> > should be removed ?
>
>
No, definitely not.

That is the default overload for types that do not have a
pointer_traits::to_address specialization. If you remove it, __to_address
won't work for fancy pointers or any other pointer-like types. That would
completely break it.

The purpose of C++20's std::to_address is to get a real pointer from a
pointer-like type. Using it with iterators is not the primary use case, but
it needs to work with contiguous iterators because those are pointer-like.
I made it work correctly with __normal_iterator because that was necessary
to support the uses of std::__to_address in <regex> and <filesystem>, but I
removed those uses in:

https://gcc.gnu.org/g:247bac507e63b32d4dc23ef1c55f300aafea24c6
https://gcc.gnu.org/g:b83b810ac440f72e7551b6496539e60ac30c0d8a

So now we don't really need the C++17 version of std::__to_address to work
with __normal_iterator at all.

I think it's OK to add the overload for __normal_iterator though, but only
for C++11/14/17, because the default std::__to_address handles
__normal_iterator correctly in C++20.


> Or perhaps just the _Safe_iterator_base branch in it ?
>

Yes, you can just remove that branch, because your new overload handles it.


>

> > On 06/10/21 7:18 pm, François Dumont wrote:
> >> Here is another proposal with the __to_address overload.
> >>
> >> I preferred to let it open to any kind of __normal_iterator
> >> instantiation cause afaics std::vector supports fancy pointer types.
> >> It is better if __to_address works fine also in this case, no ?
>

 If we intend to support that, then we should verify it in the testsuite,
using __gnu_test::CustomPointerAlloc.


>>     libstdc++: Overload std::__to_address for
> >> __gnu_cxx::__normal_iterator.
> >>
> >>     Prefer to overload __to_address to partially specialize
> >> std::pointer_traits because
> >>     std::pointer_traits would be mostly useless. In the case of
> >> __gnu_debug::_Safe_iterator
> >>     the to_pointer method is even impossible to implement correctly
> >> because we are missing
> >>     the parent container to associate the iterator to.
>

To record additional rationale in the git history, please add that the
partial specialization of pointer_traits<__normal_iterator<P, C>> fails to
rebind C, so you get incorrect types like __normal_iterator<long*,
vector<int>>.


>>
> >>     libstdc++-v3/ChangeLog:
> >>
> >>             * include/bits/stl_iterator.h
> >> (std::pointer_traits<__gnu_cxx::__normal_iterator<>>): Remove.
>

OK to remove this (it's broken anyway).

>>             (std::__to_address(const
> >> __gnu_cxx::__normal_iterator<>&)): New.
>

Please make this only defined for C++11, 14 and 17.


> >>             * include/debug/safe_iterator.h
> >>             (std::__to_address(const
> >> __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<>,
> >> _Sequence>&)):
> >>             New.
>

OK to add this (including for C++20), and remove the _Safe_iterator branch
from the C++20 std::__to_address in <bits/ptr_traits.h>.

I think this new overload could return
std::__to_address(__it.base().base()) though. That saves a function call,
by going directly to the value stored in the __normal_iterator.



> >>             * testsuite/24_iterators/normal_iterator/to_address.cc:
> >> Add check on std::vector::iterator
> >>             to validate both __gnu_cxx::__normal_iterator<>
> >> __to_address overload in normal mode and the
>

Add similar checks for vector<int, __gnu_test::CustomPointerAlloc<int>>.

OK with those changes, thanks.

Re: [committed] libstdc++: Specialize std::pointer_traits<__normal_iterator<I,C>>

[François Dumont]

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

Here is what I eventually would like to commit.

I was not able to remove the _Safe_iterator_base branch in ptr_traits.h. 
When adding the _Safe_iterator overload in C++20 and removing the branch 
the 20_util/to_address/debug.cc test started to fail because it was not 
calling my overload. I tried to declare the overload in ptr_traits.h 
directly so it is known at the time it is used in std::to_address but 
then it failed to match it with the implementation in safe_iterator.h. 
The declaration was not easy to do and I guess I had it wrong.

But it does not matter cause I think this version is the simplest one 
(as it does not change a lot of code).

     libstdc++: Overload std::__to_address for __gnu_cxx::__normal_iterator.

     Prefer to overload __to_address to partially specialize 
std::pointer_traits because
     std::pointer_traits would be mostly useless. Moreover partial 
specialization of
     pointer_traits<__normal_iterator<P, C>> fails to rebind C, so you 
get incorrect types
     like __normal_iterator<long*, vector<int>>. In the case of 
__gnu_debug::_Safe_iterator
     the to_pointer method is impossible to implement correctly because 
we are missing
     the parent container to associate the iterator to.

     libstdc++-v3/ChangeLog:

             * include/bits/stl_iterator.h
(std::pointer_traits<__gnu_cxx::__normal_iterator<>>): Remove.
             (std::__to_address(const __gnu_cxx::__normal_iterator<>&)): 
New for C++11 to C++17.
             * include/debug/safe_iterator.h
             (std::__to_address(const 
__gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<>, _Sequence>&)):
             New for C++11 to C++17.
             * testsuite/24_iterators/normal_iterator/to_address.cc: Add 
check on std::vector::iterator
             to validate both __gnu_cxx::__normal_iterator<> 
__to_address overload in normal mode and
             __gnu_debug::_Safe_iterator in _GLIBCXX_DEBUG mode.

Tested under Linux x86_64 normal and _GLIBCXX_DEBUG modes for 
C++11/C++14/C++17/C++20.

Ok to commit ?

François


On 14/12/21 2:12 pm, Jonathan Wakely wrote:
> On Tue, 14 Dec 2021 at 06:53, François Dumont wrote:
>
>     Hi
>
>          Any conclusion regarding this thread ?
>
>     François
>
>
>     On 06/10/21 7:25 pm, François Dumont wrote:
>     > I forgot to ask if with this patch this overload:
>     >
>     >   template<typename _Ptr, typename... _None>
>     >     constexpr auto
>     >     __to_address(const _Ptr& __ptr, _None...) noexcept
>     >     {
>     >       if constexpr
>     (is_base_of_v<__gnu_debug::_Safe_iterator_base, _Ptr>)
>     >     return std::__to_address(__ptr.base().operator->());
>     >       else
>     >     return std::__to_address(__ptr.operator->());
>     >     }
>     >
>     > should be removed ?
>
>
> No, definitely not.
>
> That is the default overload for types that do not have a 
> pointer_traits::to_address specialization. If you remove it, 
> __to_address won't work for fancy pointers or any other pointer-like 
> types. That would completely break it.
>
> The purpose of C++20's std::to_address is to get a real pointer from a 
> pointer-like type. Using it with iterators is not the primary use 
> case, but it needs to work with contiguous iterators because those are 
> pointer-like. I made it work correctly with __normal_iterator because 
> that was necessary to support the uses of std::__to_address in <regex> 
> and <filesystem>, but I removed those uses in:
>
> https://gcc.gnu.org/g:247bac507e63b32d4dc23ef1c55f300aafea24c6 
> <https://gcc.gnu.org/g:247bac507e63b32d4dc23ef1c55f300aafea24c6>
> https://gcc.gnu.org/g:b83b810ac440f72e7551b6496539e60ac30c0d8a 
> <https://gcc.gnu.org/g:b83b810ac440f72e7551b6496539e60ac30c0d8a>
>
> So now we don't really need the C++17 version of std::__to_address to 
> work with __normal_iterator at all.
>
> I think it's OK to add the overload for __normal_iterator though, but 
> only for C++11/14/17, because the default std::__to_address handles 
> __normal_iterator correctly in C++20.
>
>
>     > Or perhaps just the _Safe_iterator_base branch in it ?
>
>
> Yes, you can just remove that branch, because your new overload 
> handles it.
>
>
> >
>
>     > On 06/10/21 7:18 pm, François Dumont wrote:
>     >> Here is another proposal with the __to_address overload.
>     >>
>     >> I preferred to let it open to any kind of __normal_iterator
>     >> instantiation cause afaics std::vector supports fancy pointer
>     types.
>     >> It is better if __to_address works fine also in this case, no ?
>
>
>  If we intend to support that, then we should verify it in the 
> testsuite, using __gnu_test::CustomPointerAlloc.
>
>
>     >>     libstdc++: Overload std::__to_address for
>     >> __gnu_cxx::__normal_iterator.
>     >>
>     >>     Prefer to overload __to_address to partially specialize
>     >> std::pointer_traits because
>     >>     std::pointer_traits would be mostly useless. In the case of
>     >> __gnu_debug::_Safe_iterator
>     >>     the to_pointer method is even impossible to implement
>     correctly
>     >> because we are missing
>     >>     the parent container to associate the iterator to.
>
>
> To record additional rationale in the git history, please add that the 
> partial specialization of pointer_traits<__normal_iterator<P, C>> 
> fails to rebind C, so you get incorrect types like 
> __normal_iterator<long*, vector<int>>.
>
>
>     >>
>     >>     libstdc++-v3/ChangeLog:
>     >>
>     >>             * include/bits/stl_iterator.h
>     >> (std::pointer_traits<__gnu_cxx::__normal_iterator<>>): Remove.
>
>
> OK to remove this (it's broken anyway).
>
>     >>             (std::__to_address(const
>     >> __gnu_cxx::__normal_iterator<>&)): New.
>
>
> Please make this only defined for C++11, 14 and 17.
>
>     >>             * include/debug/safe_iterator.h
>     >>             (std::__to_address(const
>     >> __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<>,
>     >> _Sequence>&)):
>     >>             New.
>
>
> OK to add this (including for C++20), and remove the _Safe_iterator 
> branch from the C++20 std::__to_address in <bits/ptr_traits.h>.
>
> I think this new overload could return 
> std::__to_address(__it.base().base()) though. That saves a function 
> call, by going directly to the value stored in the __normal_iterator.
>
>     >>             *
>     testsuite/24_iterators/normal_iterator/to_address.cc:
>     >> Add check on std::vector::iterator
>     >>             to validate both __gnu_cxx::__normal_iterator<>
>     >> __to_address overload in normal mode and the
>
>
> Add similar checks for vector<int, __gnu_test::CustomPointerAlloc<int>>.
>
> OK with those changes, thanks.



[-- Attachment #2: to_address.patch --]
[-- Type: text/x-patch, Size: 3876 bytes --]

diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h
index 6bd860b803e..ac9342112f4 100644
--- a/libstdc++-v3/include/bits/stl_iterator.h
+++ b/libstdc++-v3/include/bits/stl_iterator.h
@@ -1349,32 +1349,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 #if __cplusplus >= 201103L
 
-  // Need to specialize pointer_traits because the primary template will
-  // deduce element_type of __normal_iterator<T*, C> as T* rather than T.
+#if __cplusplus <= 201703L
+  // Need to overload __to_address because the pointer_traits primary template
+  // will deduce element_type of __normal_iterator<T*, C> as T* rather than T.
   template<typename _Iterator, typename _Container>
-    struct pointer_traits<__gnu_cxx::__normal_iterator<_Iterator, _Container>>
-    {
-    private:
-      using _Base = pointer_traits<_Iterator>;
-
-    public:
-      using element_type = typename _Base::element_type;
-      using pointer = __gnu_cxx::__normal_iterator<_Iterator, _Container>;
-      using difference_type = typename _Base::difference_type;
-
-      template<typename _Tp>
-	using rebind = __gnu_cxx::__normal_iterator<_Tp, _Container>;
-
-      static pointer
-      pointer_to(element_type& __e) noexcept
-      { return pointer(_Base::pointer_to(__e)); }
-
-#if __cplusplus >= 202002L
-      static element_type*
-      to_address(pointer __p) noexcept
-      { return __p.base(); }
+    constexpr auto
+    __to_address(const __gnu_cxx::__normal_iterator<_Iterator,
+						    _Container>& __it) noexcept
+    -> decltype(std::__to_address(__it.base()))
+    { return std::__to_address(__it.base()); }
 #endif
-    };
 
   /**
    * @addtogroup iterators
diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h
index 5584d06de5a..9c821c82e17 100644
--- a/libstdc++-v3/include/debug/safe_iterator.h
+++ b/libstdc++-v3/include/debug/safe_iterator.h
@@ -1013,6 +1013,23 @@ namespace __gnu_debug
 
 } // namespace __gnu_debug
 
+#if __cplusplus >= 201103L && __cplusplus <= 201703L
+namespace std _GLIBCXX_VISIBILITY(default)
+{
+_GLIBCXX_BEGIN_NAMESPACE_VERSION
+
+  template<typename _Iterator, typename _Container, typename _Sequence>
+    constexpr auto
+    __to_address(const __gnu_debug::_Safe_iterator<
+		 __gnu_cxx::__normal_iterator<_Iterator, _Container>,
+		 _Sequence>& __it) noexcept
+    -> decltype(std::__to_address(__it.base().base()))
+    { return std::__to_address(__it.base().base()); }
+
+_GLIBCXX_END_NAMESPACE_VERSION
+}
+#endif
+
 #undef _GLIBCXX_DEBUG_VERIFY_DIST_OPERANDS
 #undef _GLIBCXX_DEBUG_VERIFY_REL_OPERANDS
 #undef _GLIBCXX_DEBUG_VERIFY_EQ_OPERANDS
diff --git a/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc b/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc
index 510d627435f..6afc6540609 100644
--- a/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc
+++ b/libstdc++-v3/testsuite/24_iterators/normal_iterator/to_address.cc
@@ -1,6 +1,19 @@
 // { dg-do compile { target { c++11 } } }
 #include <string>
+#include <vector>
 #include <memory>
 
-char* p = std::__to_address(std::string("1").begin());
-const char* q = std::__to_address(std::string("2").cbegin());
+#include <testsuite_allocator.h>
+
+char* p __attribute__((unused))
+  = std::__to_address(std::string("1").begin());
+const char* q __attribute__((unused))
+  = std::__to_address(std::string("2").cbegin());
+int* r __attribute__((unused))
+  = std::__to_address(std::vector<int>(1, 1).begin());
+const int* s __attribute__((unused))
+  = std::__to_address(std::vector<int>(1, 1).cbegin());
+int* t __attribute__((unused))
+  = std::__to_address(std::vector<int, __gnu_test::CustomPointerAlloc<int>>(1, 1).begin());
+const int* u __attribute__((unused))
+  = std::__to_address(std::vector<int, __gnu_test::CustomPointerAlloc<int>>(1, 1).cbegin());

Re: [committed] libstdc++: Specialize std::pointer_traits<__normal_iterator<I, C>>

[Jonathan Wakely]

On Wed, 15 Dec 2021 at 21:16, François Dumont <frs.dumont@gmail.com> wrote:

> Here is what I eventually would like to commit.
>
> I was not able to remove the _Safe_iterator_base branch in ptr_traits.h.
> When adding the _Safe_iterator overload in C++20 and removing the branch
> the 20_util/to_address/debug.cc test started to fail because it was not
> calling my overload. I tried to declare the overload in ptr_traits.h
> directly so it is known at the time it is used in std::to_address but then
> it failed to match it with the implementation in safe_iterator.h. The
> declaration was not easy to do and I guess I had it wrong.
>
> But it does not matter cause I think this version is the simplest one (as
> it does not change a lot of code).
>
>     libstdc++: Overload std::__to_address for __gnu_cxx::__normal_iterator.
>
>     Prefer to overload __to_address to partially specialize
> std::pointer_traits because
>     std::pointer_traits would be mostly useless. Moreover partial
> specialization of
>     pointer_traits<__normal_iterator<P, C>> fails to rebind C, so you get
> incorrect types
>     like __normal_iterator<long*, vector<int>>. In the case of
> __gnu_debug::_Safe_iterator
>     the to_pointer method is impossible to implement correctly because we
> are missing
>     the parent container to associate the iterator to.
>
>     libstdc++-v3/ChangeLog:
>
>             * include/bits/stl_iterator.h
>             (std::pointer_traits<__gnu_cxx::__normal_iterator<>>): Remove.
>             (std::__to_address(const __gnu_cxx::__normal_iterator<>&)):
> New for C++11 to C++17.
>             * include/debug/safe_iterator.h
>             (std::__to_address(const
> __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<>, _Sequence>&)):
>             New for C++11 to C++17.
>             * testsuite/24_iterators/normal_iterator/to_address.cc: Add
> check on std::vector::iterator
>             to validate both __gnu_cxx::__normal_iterator<> __to_address
> overload in normal mode and
>             __gnu_debug::_Safe_iterator in _GLIBCXX_DEBUG mode.
>
> Tested under Linux x86_64 normal and _GLIBCXX_DEBUG modes for
> C++11/C++14/C++17/C++20.
>
> Ok to commit ?
>

OK, thanks!



> François
>
>
> On 14/12/21 2:12 pm, Jonathan Wakely wrote:
>
> On Tue, 14 Dec 2021 at 06:53, François Dumont wrote:
>
>> Hi
>>
>>      Any conclusion regarding this thread ?
>>
>> François
>>
>>
>> On 06/10/21 7:25 pm, François Dumont wrote:
>> > I forgot to ask if with this patch this overload:
>> >
>> >   template<typename _Ptr, typename... _None>
>> >     constexpr auto
>> >     __to_address(const _Ptr& __ptr, _None...) noexcept
>> >     {
>> >       if constexpr (is_base_of_v<__gnu_debug::_Safe_iterator_base,
>> _Ptr>)
>> >     return std::__to_address(__ptr.base().operator->());
>> >       else
>> >     return std::__to_address(__ptr.operator->());
>> >     }
>> >
>> > should be removed ?
>>
>>
> No, definitely not.
>
> That is the default overload for types that do not have a
> pointer_traits::to_address specialization. If you remove it, __to_address
> won't work for fancy pointers or any other pointer-like types. That would
> completely break it.
>
> The purpose of C++20's std::to_address is to get a real pointer from a
> pointer-like type. Using it with iterators is not the primary use case, but
> it needs to work with contiguous iterators because those are pointer-like.
> I made it work correctly with __normal_iterator because that was necessary
> to support the uses of std::__to_address in <regex> and <filesystem>, but I
> removed those uses in:
>
> https://gcc.gnu.org/g:247bac507e63b32d4dc23ef1c55f300aafea24c6
> https://gcc.gnu.org/g:b83b810ac440f72e7551b6496539e60ac30c0d8a
>
> So now we don't really need the C++17 version of std::__to_address to work
> with __normal_iterator at all.
>
> I think it's OK to add the overload for __normal_iterator though, but only
> for C++11/14/17, because the default std::__to_address handles
> __normal_iterator correctly in C++20.
>
>
> > Or perhaps just the _Safe_iterator_base branch in it ?
>>
>
> Yes, you can just remove that branch, because your new overload handles it.
>
>
> >
>
>> > On 06/10/21 7:18 pm, François Dumont wrote:
>> >> Here is another proposal with the __to_address overload.
>> >>
>> >> I preferred to let it open to any kind of __normal_iterator
>> >> instantiation cause afaics std::vector supports fancy pointer types.
>> >> It is better if __to_address works fine also in this case, no ?
>>
>
>  If we intend to support that, then we should verify it in the testsuite,
> using __gnu_test::CustomPointerAlloc.
>
>
> >>     libstdc++: Overload std::__to_address for
>> >> __gnu_cxx::__normal_iterator.
>> >>
>> >>     Prefer to overload __to_address to partially specialize
>> >> std::pointer_traits because
>> >>     std::pointer_traits would be mostly useless. In the case of
>> >> __gnu_debug::_Safe_iterator
>> >>     the to_pointer method is even impossible to implement correctly
>> >> because we are missing
>> >>     the parent container to associate the iterator to.
>>
>
> To record additional rationale in the git history, please add that the
> partial specialization of pointer_traits<__normal_iterator<P, C>> fails to
> rebind C, so you get incorrect types like __normal_iterator<long*,
> vector<int>>.
>
>
> >>
>> >>     libstdc++-v3/ChangeLog:
>> >>
>> >>             * include/bits/stl_iterator.h
>> >> (std::pointer_traits<__gnu_cxx::__normal_iterator<>>): Remove.
>>
>
> OK to remove this (it's broken anyway).
>
> >>             (std::__to_address(const
>> >> __gnu_cxx::__normal_iterator<>&)): New.
>>
>
> Please make this only defined for C++11, 14 and 17.
>
>
>> >>             * include/debug/safe_iterator.h
>> >>             (std::__to_address(const
>> >> __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<>,
>> >> _Sequence>&)):
>> >>             New.
>>
>
> OK to add this (including for C++20), and remove the _Safe_iterator branch
> from the C++20 std::__to_address in <bits/ptr_traits.h>.
>
> I think this new overload could return
> std::__to_address(__it.base().base()) though. That saves a function call,
> by going directly to the value stored in the __normal_iterator.
>
>
>
>> >>             * testsuite/24_iterators/normal_iterator/to_address.cc:
>> >> Add check on std::vector::iterator
>> >>             to validate both __gnu_cxx::__normal_iterator<>
>> >> __to_address overload in normal mode and the
>>
>
> Add similar checks for vector<int, __gnu_test::CustomPointerAlloc<int>>.
>
> OK with those changes, thanks.
>
>
>
>
>