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

[François Dumont]

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.dum...@gmail.com> wrote:

    On 15/02/2024 14:17, Jonathan Wakely wrote:

    On Wed, 14 Feb 2024 at 21:48, François Dumont
    <frs.dum...@gmail.com> wrote:


        On 14/02/2024 20:44, Jonathan Wakely wrote:

        On Wed, 14 Feb 2024 at 18:39, François Dumont
        <frs.dum...@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.