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> fails to rebind C, so you get incorrect types     like __normal_iterator>. 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 > >     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 > and , 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> > fails to rebind C, so you get incorrect types like > __normal_iterator>. > > > >> > >>     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 . > > 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>. > > OK with those changes, thanks.