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

[Jonathan Wakely <jwakely@redhat.com>]

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

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






> 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.