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.