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

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

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


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

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.