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

["François Dumont" <frs.dumont@gmail.com>]

[-- 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());