Re: _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms

[Jonathan Wakely <jwakely@redhat.com>]

On Tue, 12 Mar 2024 at 01:03, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On Tue, 12 Mar 2024 at 00:55, Maciej Miera <maciej.miera@gmail.com> wrote:
> >
> >
> >
> > Wiadomość napisana przez Jonathan Wakely <jwakely@redhat.com> w dniu 11.03.2024, o godz. 21:40:
> >
> > On Mon, 11 Mar 2024 at 20:07, Maciej Miera <maciej.miera@gmail.com> wrote:
> >
> >
> > Hello,
> >
> > I have tried to introduce an extra level of safety to my codebase and utilize _GLIBCXX_DEBUG in my test builds in order to catch faulty iterators.
> > However, I have encountered the following problem: I would like to utilize singular, value-initialized iterators as an arbitrary "null range”.
> > However, this leads to failed assertions in std:: algorithms taking such range.
> >
> > Consider the following code sample with find_if:
> >
> > #include <map>
> > #include <algorithm>
> > #include <iterator>
> >
> > #ifndef __cpp_lib_null_iterators
> > #warning "Not standard compliant"
> > #endif
> >
> > int main()
> > {
> >    std::multimap<char, int>::iterator it1{};
> >    std::multimap<char, int>::iterator it2{};
> >
> >    (void) (it1==it2); // OK
> >    (void) std::find_if(
> >        it1, it2, [](const auto& el) { return el.second == 8;});
> > }
> >
> > Compiled with -std=c++20 and -D_GLIBCXX_DEBUG it produces the warning "Not standard compliant"
> > and the execution results in the following assert failure:
> >
> > /opt/compiler-explorer/gcc-12.2.0/include/c++/12.2.0/bits/stl_algo.h:3875:
> > In function:
> >    constexpr _IIter std::find_if(_IIter, _IIter, _Predicate) [with _IIter =
> >    gnu_debug::_Safe_iterator<_Rb_tree_iterator<pair<const char, int> >,
> >    debug::multimap<char, int>, bidirectional_iterator_tag>; _Predicate =
> >    main()::<lambda(const auto:16&)>]
> >
> > The question is though: is it by design, or is it just a mere oversight? The warning actually suggest the first option.
> > If it is an intentional design choice, could you provide some rationale behind it, please?
> >
> >
> > The macro was not defined because the C++14 rule wasn't implemented
> > for debug mode, but that should have been fixed for GCC 11, according
> > to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98466 and
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70303
> > So we should be able to define macro now, except maybe it wasn't fixed
> > for the RB tree containers.
> >
> >
> >
> > Just to make sure there are no misunderstandings: comparison via == works fine. The feature check macro without _GLIBCXX_DEBUG and with <iterator> included is also fine. Maybe the need to include a header is the issue, but that’s not the core of the problem anyway.
>
> No, it has nothing to do with the headers included. The feature test
> macro is defined like so:
>
> # if (__cplusplus >= 201402L) && (!defined(_GLIBCXX_DEBUG))
> #  define __glibcxx_null_iterators 201304L
>
> It's a very deliberate choice to not define it when _GLIBCXX_DEBUG is
> defined. But as I said, I think we should have changed that.
>
> >
> > The actual question is though, whether passing singular iterators to std algorithms (like find_if) should make the asserts at the beginning of the algo function fail when compiled with _GLIBCXX_DEBUG. IMHO, intuitively it should not, as comparing iterators equal would just ensure early exit and return of the same singular iterator.
> > This seems not to be the case though. The actual message is this:
> > Error: the function requires a valid iterator range [first, last).
> > What bothers me is whether the empty virtual range limited by two same singular iterators is actually valid or not.
>
> Yes, it's valid. So the bug is in the __glibcxx_requires_valid_range macro.

Thanks for the bugzilla report:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114316
We'll get it fixed!