Re: [PATCH][_GLIBCXX_DEBUG] Fix std::__niter_base behavior

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

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

Thanks for the link, tested and committed.

On 15/02/2024 19:40, Jonathan Wakely wrote:
>
>
> On Thu, 15 Feb 2024 at 18:38, François Dumont <frs.dumont@gmail.com> 
> wrote:
>
>
>     On 15/02/2024 14:17, Jonathan Wakely wrote:
>>
>>
>>     On Wed, 14 Feb 2024 at 21:48, François Dumont
>>     <frs.dumont@gmail.com> wrote:
>>
>>
>>         On 14/02/2024 20:44, Jonathan Wakely wrote:
>>>
>>>
>>>         On Wed, 14 Feb 2024 at 18:39, François Dumont
>>>         <frs.dumont@gmail.com> wrote:
>>>
>>>             libstdc++: [_GLIBCXX_DEBUG] Fix std::__niter_base behavior
>>>
>>>             std::__niter_base is used in _GLIBCXX_DEBUG mode to
>>>             remove _Safe_iterator<>
>>>             wrapper on random access iterators. But doing so it
>>>             should also preserve
>>>             original
>>>             behavior to remove __normal_iterator wrapper.
>>>
>>>             libstdc++-v3/ChangeLog:
>>>
>>>                  * include/bits/stl_algobase.h (std::__niter_base):
>>>             Redefine the
>>>             overload
>>>                  definitions for __gnu_debug::_Safe_iterator.
>>>                  * include/debug/safe_iterator.tcc
>>>             (std::__niter_base): Adapt
>>>             declarations.
>>>
>>>             Ok to commit once all tests completed (still need to
>>>             check pre-c++11) ?
>>>
>>>
>>>
>>>         The declaration in include/bits/stl_algobase.h has a
>>>         noexcept-specifier but the definition in
>>>         include/debug/safe_iterator.tcc does not have one - that
>>>         seems wrong (I'm surprised it even compiles).
>>
>>         It does !
>>
>>
>>     The diagnostic is suppressed without -Wsystem-headers:
>>
>>     /home/jwakely/gcc/14/include/c++/14.0.1/debug/safe_iterator.tcc:255:5:warning:
>>     declaration of 'template<class _Ite, class _Seq> constexpr
>>     decltype (std::__
>>     niter_base(declval<_Ite>())) std::__niter_base(const
>>     __gnu_debug::_Safe_iterator<_Iterator, _Sequence,
>>     random_access_iterator_tag>&)' has a different except
>>     ion specifier [-Wsystem-headers]
>>      255 | __niter_base(const ::__gnu_debug::_Safe_iterator<_Ite, _Seq,
>>          | ^~~~~~~~~~~~
>>     /home/jwakely/gcc/14/include/c++/14.0.1/bits/stl_algobase.h:335:5:note:
>>     from previous declaration 'template<class _Ite, class _Seq>
>>     constexpr decltype (std
>>     ::__niter_base(declval<_Ite>())) std::__niter_base(const
>>     __gnu_debug::_Safe_iterator<_Iterator, _Sequence,
>>     random_access_iterator_tag>&) noexcept (noexcept
>>     (is_nothrow_copy_constructible<decltype
>>     (std::__niter_base(declval<_Ite>()))>::value))'
>>      335 | __niter_base(const ::__gnu_debug::_Safe_iterator<_Ite, _Seq,
>>          | ^~~~~~~~~~~~
>>
>>
>>     It's a hard error with Clang though:
>>
>>     deb.cc:7:10: error: call to '__niter_base' is ambiguous
>>
>>
>     Yes, I eventually got the error too, I hadn't run enough tests yet.
>
>
>>
>>
>>         I thought it was only necessary at declaration, and I also
>>         had troubles doing it right at definition because of the
>>         interaction with the auto and ->.
>>
>>
>>     The trailing-return-type has to come after the noexcept-specifier.
>>
>>         Now simplified and consistent in this new proposal.
>>
>>
>>>         Just using std::is_nothrow_copy_constructible<_Ite> seems
>>>         simpler, that will be true for __normal_iterator<I, C> if
>>>         is_nothrow_copy_constructible<I> is true.
>>>
>>         Ok
>>
>>
>>>         The definition in include/debug/safe_iterator.tcc should use
>>>         std::declval<_Ite>() not declval<_Ite>(). Is there any
>>>         reason why the definition uses a late-specified-return-type
>>>         (i.e. auto and ->) when the declaration doesn't?
>>>
>>>
>>         I initially plan to use '->
>>         std::decltype(std::__niter_base(__it.base()))' but this did
>>         not compile, ambiguity issue. So I resort to using
>>         std::declval and I could have then done it the same way as
>>         declaration, done now.
>>
>>         Attached is what I'm testing, ok to commit once fully tested ?
>>
>>
>>     OK, thanks.
>>
>     Thanks for validation but I have a problem to test for c++98.
>
>     When I do:
>
>     make CXXFLAGS=-std=c++98 check-debug
>
>
> That doesn't work any more, see 
> https://gcc.gnu.org/onlinedocs/libstdc++/manual/test.html#test.run.permutations
>
>     I see in debug/libstdc++.log for example:
>
>     Executing on host: /home/fdumont/dev/gcc/build/./gcc/xg++
>     -shared-libgcc ... -mshstk -std=c++98 -g -O2 -DLOCALEDIR="."
>     -nostdinc++ -I/home/fdumont/dev/gcc/...
>     /home/fdumont/dev/gcc/git/libstdc++-v3/testsuite/25_algorithms/copy/3.cc
>     -D_GLIBCXX_DEBUG   -std=gnu++17  -include bits/stdc++.h ...  -lm 
>     -o ./3.exe    (timeout = 360)
>
>     The -std=c++98 is there but later comes the -std=gnu++17 so I
>     think it runs in C++17, no ?
>
>     I also tried the documented alternative:
>
>     make check 'RUNTESTFLAGS=--target_board=unix/-O3\"{-std=gnu++98,-std=gnu++11,-std=gnu++14}\"'
>
>     but same problem, -std=gnu++17 comes last.
>
>     I'll try to rebuild all from scratch but I won't commit soon then.
>
>