_LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms

_LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms

[Maciej Miera]

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?

Compiler explorer link with demo below:
https://godbolt.org/z/e8xa5Eoqn

Best regards,
A M. Miera

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

[Jonathan Wakely]

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.

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

[Maciej Miera]

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



> 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 <mailto: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 <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98466> and
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70303 <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.

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.
This occurs with various std containers, I did check vector, multimap and unordered_map; I can check all the containers but I daresay it’s a pretty general thing.

Best regards,
A. M. Miera

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

[Jonathan Wakely]

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.

> This occurs with various std containers, I did check vector, multimap and unordered_map; I can check all the containers but I daresay it’s a pretty general thing.
>
> Best regards,
> A. M. Miera

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

[Jonathan Wakely]

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!

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

[François Dumont]

Hi

This is what I started to do.

For now I haven't touch to __cpp_lib_null_iterators definition as 
_Safe_local_iterator still need some work.

libstdc++: Implement N3644 on _Safe_iterator<> [PR114316]

Consider range of value-initialized iterators as valid and empty.

libstdc++-v3/ChangeLog:

         PR libstdc++/114316
         * include/debug/safe_iterator.tcc 
(_Safe_iterator<>::_M_valid_range):
         First check if both iterators are value-initialized before 
checking if
         singular.
         * testsuite/23_containers/set/debug/114316.cc: New test case.
         * testsuite/23_containers/vector/debug/114316.cc: New test case.

Tested under Linux x86_64, ok to commit ?

François


On 12/03/2024 10:52, Jonathan Wakely wrote:
> 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!
>

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

[François Dumont]

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

With the patch, sorry.

On 14/03/2024 22:49, François Dumont wrote:
> Hi
>
> This is what I started to do.
>
> For now I haven't touch to __cpp_lib_null_iterators definition as 
> _Safe_local_iterator still need some work.
>
> libstdc++: Implement N3644 on _Safe_iterator<> [PR114316]
>
> Consider range of value-initialized iterators as valid and empty.
>
> libstdc++-v3/ChangeLog:
>
>         PR libstdc++/114316
>         * include/debug/safe_iterator.tcc 
> (_Safe_iterator<>::_M_valid_range):
>         First check if both iterators are value-initialized before 
> checking if
>         singular.
>         * testsuite/23_containers/set/debug/114316.cc: New test case.
>         * testsuite/23_containers/vector/debug/114316.cc: New test case.
>
> Tested under Linux x86_64, ok to commit ?
>
> François
>
>
> On 12/03/2024 10:52, Jonathan Wakely wrote:
>> 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!
>>

[-- Attachment #2: pr104316.txt --]
[-- Type: text/plain, Size: 2121 bytes --]

diff --git a/libstdc++-v3/include/debug/safe_iterator.tcc b/libstdc++-v3/include/debug/safe_iterator.tcc
index a8b24233e85..4b2baf2980e 100644
--- a/libstdc++-v3/include/debug/safe_iterator.tcc
+++ b/libstdc++-v3/include/debug/safe_iterator.tcc
@@ -194,6 +194,12 @@ namespace __gnu_debug
 		   std::pair<difference_type, _Distance_precision>& __dist,
 		   bool __check_dereferenceable) const
     {
+      if (_M_value_initialized() && __rhs._M_value_initialized())
+	{
+	  __dist = std::make_pair(0, __dp_exact);
+	  return true;
+	}
+
       if (_M_singular() || __rhs._M_singular() || !_M_can_compare(__rhs))
 	return false;
 
@@ -218,6 +224,12 @@ namespace __gnu_debug
 		   std::pair<difference_type,
 			     _Distance_precision>& __dist) const
     {
+      if (this->_M_value_initialized() && __rhs._M_value_initialized())
+	{
+	  __dist = std::make_pair(0, __dp_exact);
+	  return true;
+	}
+
       if (this->_M_singular() || __rhs._M_singular()
 	  || !this->_M_can_compare(__rhs))
 	return false;
diff --git a/libstdc++-v3/testsuite/23_containers/set/debug/114316.cc b/libstdc++-v3/testsuite/23_containers/set/debug/114316.cc
new file mode 100644
index 00000000000..126ec89b5e0
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/set/debug/114316.cc
@@ -0,0 +1,16 @@
+// { dg-do run { target c++11 } }
+// { dg-require-debug-mode "" }
+
+// PR libstdc++/114316
+
+#include <set>
+#include <algorithm>
+
+#include <testsuite_hooks.h>
+
+int main()
+{
+  std::set<int>::iterator it{};
+  VERIFY( std::find(it, it, 0) == it );
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/23_containers/vector/debug/114316.cc b/libstdc++-v3/testsuite/23_containers/vector/debug/114316.cc
new file mode 100644
index 00000000000..f211cf67b4c
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/vector/debug/114316.cc
@@ -0,0 +1,16 @@
+// { dg-do run { target c++11 } }
+// { dg-require-debug-mode "" }
+
+// PR libstdc++/114316
+
+#include <vector>
+#include <algorithm>
+
+#include <testsuite_hooks.h>
+
+int main()
+{
+  std::vector<int>::iterator it{};
+  VERIFY( std::find(it, it, 0) == it );
+  return 0;
+}

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

[Jonathan Wakely]

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

On Sat, 16 Mar 2024, 12:16 François Dumont, <frs.dumont@gmail.com> wrote:

> With the patch, sorry.
>
> On 14/03/2024 22:49, François Dumont wrote:
> > Hi
> >
> > This is what I started to do.
> >
> > For now I haven't touch to __cpp_lib_null_iterators definition as
> > _Safe_local_iterator still need some work.
> >
> > libstdc++: Implement N3644 on _Safe_iterator<> [PR114316]
> >
> > Consider range of value-initialized iterators as valid and empty.
> >
> > libstdc++-v3/ChangeLog:
> >
> >         PR libstdc++/114316
> >         * include/debug/safe_iterator.tcc
> > (_Safe_iterator<>::_M_valid_range):
> >         First check if both iterators are value-initialized before
> > checking if
> >         singular.
> >         * testsuite/23_containers/set/debug/114316.cc: New test case.
> >         * testsuite/23_containers/vector/debug/114316.cc: New test case.
> >
> > Tested under Linux x86_64, ok to commit ?
>

OK for trunk, thanks!

I think this is OK to backport to 13 too.

Maybe after this we can define the __cpp_lib_null_itetators macro for debug
mode?


>
> > François
> >
> >
> > On 12/03/2024 10:52, Jonathan Wakely wrote:
> >> 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!
> >>

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

[François Dumont]

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


>
> OK for trunk, thanks!
>
> I think this is OK to backport to 13 too.
>
> Maybe after this we can define the __cpp_lib_null_itetators macro for 
> debug mode?
>
After this fix of local_iterator I think we can indeed.

In fact the added 11316.cc was already passing for 
unordered_set<>::local_iterator but simply because we were missing the 
singular check. Both issues solved with this patch.

I found the version.def file to cleanup but no idea how to regenerate 
version.h from it so I'll let you do it, ok ?

     libstdc++: Fix _Safe_local_iterator<>::_M_valid_range

     Unordered container local_iterator range shall not contain any singular
     iterator unless both iterators are value-initialized.

     libstdc++-v3/ChangeLog:

             * include/debug/safe_local_iterator.tcc
             (_Safe_local_iterator::_M_valid_range): Add 
_M_value_initialized and
             _M_singular checks.
             * testsuite/23_containers/unordered_set/debug/114316.cc: 
New test case.


Ok to commit ?

François

[-- Attachment #2: safe_local_iterator.txt --]
[-- Type: text/plain, Size: 1491 bytes --]

diff --git a/libstdc++-v3/include/debug/safe_local_iterator.tcc b/libstdc++-v3/include/debug/safe_local_iterator.tcc
index 90e60e37c32..6d546ec040c 100644
--- a/libstdc++-v3/include/debug/safe_local_iterator.tcc
+++ b/libstdc++-v3/include/debug/safe_local_iterator.tcc
@@ -78,7 +78,13 @@ namespace __gnu_debug
     _M_valid_range(const _Safe_local_iterator& __rhs,
 		std::pair<difference_type, _Distance_precision>& __dist) const
     {
-      if (!_M_can_compare(__rhs))
+      if (_M_value_initialized() && __rhs._M_value_initialized())
+	{
+	  __dist = { 0, __dp_exact };
+	  return true;
+	}
+
+      if (_M_singular() || __rhs._M_singular() || !_M_can_compare(__rhs))
 	return false;
 
       if (bucket() != __rhs.bucket())
diff --git a/libstdc++-v3/testsuite/23_containers/unordered_set/debug/114316.cc b/libstdc++-v3/testsuite/23_containers/unordered_set/debug/114316.cc
new file mode 100644
index 00000000000..41b649a9cbd
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/unordered_set/debug/114316.cc
@@ -0,0 +1,28 @@
+// { dg-do run { target c++11 } }
+// { dg-require-debug-mode "" }
+
+// PR libstdc++/114316
+
+#include <unordered_set>
+#include <algorithm>
+
+#include <testsuite_hooks.h>
+
+void test01()
+{
+  std::unordered_set<int>::iterator it{};
+  VERIFY( std::find(it, it, 0) == it );
+}
+
+void test02()
+{
+  std::unordered_set<int>::local_iterator it{};
+  VERIFY( std::find(it, it, 0) == it );
+}
+
+int main()
+{
+  test01();
+  test02();
+  return 0;
+}

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

[François Dumont]

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

I was a little bit too confident below. After review of all _M_singular 
usages I found another necessary fix.

After this one for sure we will be able to define 
__cpp_lib_null_iterators even in Debug mode.

     libstdc++: Fix N3344 behavior on _Safe_iterator::_M_can_advance

     We shall be able to advance from a 0 offset a value-initialized 
iterator.

     libstdc++-v3/ChangeLog:

             * include/debug/safe_iterator.tcc 
(_Safe_iterator<>::_M_can_advance):
             Accept 0 offset advance on value-initialized iterator.
             * testsuite/23_containers/vector/debug/n3644.cc: New test case.

Ok to commit ?

François


On 17/03/2024 17:52, François Dumont wrote:
>
>>
>> OK for trunk, thanks!
>>
>> I think this is OK to backport to 13 too.
>>
>> Maybe after this we can define the __cpp_lib_null_itetators macro for 
>> debug mode?
>>
> After this fix of local_iterator I think we can indeed.
>
> In fact the added 11316.cc was already passing for 
> unordered_set<>::local_iterator but simply because we were missing the 
> singular check. Both issues solved with this patch.
>
> I found the version.def file to cleanup but no idea how to regenerate 
> version.h from it so I'll let you do it, ok ?
>
>     libstdc++: Fix _Safe_local_iterator<>::_M_valid_range
>
>     Unordered container local_iterator range shall not contain any 
> singular
>     iterator unless both iterators are value-initialized.
>
>     libstdc++-v3/ChangeLog:
>
>             * include/debug/safe_local_iterator.tcc
>             (_Safe_local_iterator::_M_valid_range): Add 
> _M_value_initialized and
>             _M_singular checks.
>             * testsuite/23_containers/unordered_set/debug/114316.cc: 
> New test case.
>
>
> Ok to commit ?
>
> François

[-- Attachment #2: n3344_patch.txt --]
[-- Type: text/plain, Size: 1088 bytes --]

diff --git a/libstdc++-v3/include/debug/safe_iterator.tcc b/libstdc++-v3/include/debug/safe_iterator.tcc
index 4b2baf2980e..deaa84d0a1f 100644
--- a/libstdc++-v3/include/debug/safe_iterator.tcc
+++ b/libstdc++-v3/include/debug/safe_iterator.tcc
@@ -86,6 +86,9 @@ namespace __gnu_debug
     _Safe_iterator<_Iterator, _Sequence, _Category>::
     _M_can_advance(difference_type __n, bool __strict) const
     {
+      if (this->_M_value_initialized() && __n == 0)
+	return true;
+
       if (this->_M_singular())
 	return false;
 
diff --git a/libstdc++-v3/testsuite/23_containers/vector/debug/n3644.cc b/libstdc++-v3/testsuite/23_containers/vector/debug/n3644.cc
new file mode 100644
index 00000000000..052c52f26b7
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/vector/debug/n3644.cc
@@ -0,0 +1,16 @@
+// { dg-do run { target c++11 } }
+// { dg-require-debug-mode "" }
+
+#include <vector>
+#include <algorithm>
+
+#include <testsuite_hooks.h>
+
+int main()
+{
+  std::vector<int>::iterator it{};
+  auto cpy = it;
+  std::advance(it, 0);
+  VERIFY( it == cpy );
+  return 0;
+}

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

[Jonathan Wakely]

On Sun, 17 Mar 2024 at 18:14, François Dumont <frs.dumont@gmail.com> wrote:
>
> I was a little bit too confident below. After review of all _M_singular
> usages I found another necessary fix.
>
> After this one for sure we will be able to define
> __cpp_lib_null_iterators even in Debug mode.
>
>      libstdc++: Fix N3344 behavior on _Safe_iterator::_M_can_advance
>
>      We shall be able to advance from a 0 offset a value-initialized
> iterator.
>
>      libstdc++-v3/ChangeLog:
>
>              * include/debug/safe_iterator.tcc
> (_Safe_iterator<>::_M_can_advance):
>              Accept 0 offset advance on value-initialized iterator.
>              * testsuite/23_containers/vector/debug/n3644.cc: New test case.
>
> Ok to commit ?


OK, thanks.


> François
>
>
> On 17/03/2024 17:52, François Dumont wrote:
> >
> >>
> >> OK for trunk, thanks!
> >>
> >> I think this is OK to backport to 13 too.
> >>
> >> Maybe after this we can define the __cpp_lib_null_itetators macro for
> >> debug mode?
> >>
> > After this fix of local_iterator I think we can indeed.
> >
> > In fact the added 11316.cc was already passing for
> > unordered_set<>::local_iterator but simply because we were missing the
> > singular check. Both issues solved with this patch.
> >
> > I found the version.def file to cleanup but no idea how to regenerate
> > version.h from it so I'll let you do it, ok ?
> >
> >     libstdc++: Fix _Safe_local_iterator<>::_M_valid_range
> >
> >     Unordered container local_iterator range shall not contain any
> > singular
> >     iterator unless both iterators are value-initialized.
> >
> >     libstdc++-v3/ChangeLog:
> >
> >             * include/debug/safe_local_iterator.tcc
> >             (_Safe_local_iterator::_M_valid_range): Add
> > _M_value_initialized and
> >             _M_singular checks.
> >             * testsuite/23_containers/unordered_set/debug/114316.cc:
> > New test case.
> >
> >
> > Ok to commit ?
> >
> > François

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

[François Dumont]

Both committed now.

Just to confirm, those 2 last patches should be backported to gcc-13 
branch, right ?

I might have a try to update version.h but if you want to do it before 
don't hesitate.

François

On 18/03/2024 08:45, Jonathan Wakely wrote:
> On Sun, 17 Mar 2024 at 18:14, François Dumont <frs.dumont@gmail.com> wrote:
>> I was a little bit too confident below. After review of all _M_singular
>> usages I found another necessary fix.
>>
>> After this one for sure we will be able to define
>> __cpp_lib_null_iterators even in Debug mode.
>>
>>       libstdc++: Fix N3344 behavior on _Safe_iterator::_M_can_advance
>>
>>       We shall be able to advance from a 0 offset a value-initialized
>> iterator.
>>
>>       libstdc++-v3/ChangeLog:
>>
>>               * include/debug/safe_iterator.tcc
>> (_Safe_iterator<>::_M_can_advance):
>>               Accept 0 offset advance on value-initialized iterator.
>>               * testsuite/23_containers/vector/debug/n3644.cc: New test case.
>>
>> Ok to commit ?
>
> OK, thanks.
>
>
>> François
>>
>>
>> On 17/03/2024 17:52, François Dumont wrote:
>>>> OK for trunk, thanks!
>>>>
>>>> I think this is OK to backport to 13 too.
>>>>
>>>> Maybe after this we can define the __cpp_lib_null_itetators macro for
>>>> debug mode?
>>>>
>>> After this fix of local_iterator I think we can indeed.
>>>
>>> In fact the added 11316.cc was already passing for
>>> unordered_set<>::local_iterator but simply because we were missing the
>>> singular check. Both issues solved with this patch.
>>>
>>> I found the version.def file to cleanup but no idea how to regenerate
>>> version.h from it so I'll let you do it, ok ?
>>>
>>>      libstdc++: Fix _Safe_local_iterator<>::_M_valid_range
>>>
>>>      Unordered container local_iterator range shall not contain any
>>> singular
>>>      iterator unless both iterators are value-initialized.
>>>
>>>      libstdc++-v3/ChangeLog:
>>>
>>>              * include/debug/safe_local_iterator.tcc
>>>              (_Safe_local_iterator::_M_valid_range): Add
>>> _M_value_initialized and
>>>              _M_singular checks.
>>>              * testsuite/23_containers/unordered_set/debug/114316.cc:
>>> New test case.
>>>
>>>
>>> Ok to commit ?
>>>
>>> François

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

[Jonathan Wakely]

On Mon, 18 Mar 2024 at 21:38, François Dumont wrote:
>
> Both committed now.
>
> Just to confirm, those 2 last patches should be backported to gcc-13
> branch, right ?

Yes please.

>
> I might have a try to update version.h but if you want to do it before
> don't hesitate.

You'll need to have 'autogen' installed to do it (I'm going to update
the docs to describe that today).

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

[Jonathan Wakely]

On Tue, 19 Mar 2024 at 09:31, Jonathan Wakely wrote:
>
> On Mon, 18 Mar 2024 at 21:38, François Dumont wrote:
> >
> > Both committed now.
> >
> > Just to confirm, those 2 last patches should be backported to gcc-13
> > branch, right ?
>
> Yes please.
>
> >
> > I might have a try to update version.h but if you want to do it before
> > don't hesitate.
>
> You'll need to have 'autogen' installed to do it (I'm going to update
> the docs to describe that today).

I've documented it in a new "Generated files" section at the end of
https://gcc.gnu.org/onlinedocs/libstdc++/manual/appendix_porting.html#build_hacking.generated

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

[François Dumont]

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

Thanks to you doc:

     libstdc++: [_GLIBCXX_DEBUG] Define __[glibcxx,cpp_lib]_null_iterators

     _GLIBCXX_DEBUG has now fully N3344 compliant iterator checks, we 
can define
     __glibcxx_null_iterators and __cpp_lib_null_iterators macros like 
the normal
     mode.

     libstdc++-v3/ChangeLog:

             * version.def (null_iterators): Remove extra_cond.
             * version.h: Regenerate.

Ok to commit ?

I already noticed that GCC 13 has no version.h file so no backport question.

François

On 19/03/2024 16:41, Jonathan Wakely wrote:
> On Tue, 19 Mar 2024 at 09:31, Jonathan Wakely wrote:
>> On Mon, 18 Mar 2024 at 21:38, François Dumont wrote:
>>> Both committed now.
>>>
>>> Just to confirm, those 2 last patches should be backported to gcc-13
>>> branch, right ?
>> Yes please.
>>
>>> I might have a try to update version.h but if you want to do it before
>>> don't hesitate.
>> You'll need to have 'autogen' installed to do it (I'm going to update
>> the docs to describe that today).
> I've documented it in a new "Generated files" section at the end of
> https://gcc.gnu.org/onlinedocs/libstdc++/manual/appendix_porting.html#build_hacking.generated
>

[-- Attachment #2: version_patch.txt --]
[-- Type: text/plain, Size: 927 bytes --]

diff --git a/libstdc++-v3/include/bits/version.def b/libstdc++-v3/include/bits/version.def
index be5af18e818..26e62c6a9b2 100644
--- a/libstdc++-v3/include/bits/version.def
+++ b/libstdc++-v3/include/bits/version.def
@@ -209,7 +209,6 @@ ftms = {
   values = {
     v = 201304;
     cxxmin = 14;
-    extra_cond = "!defined(_GLIBCXX_DEBUG)";
   };
 };
 
diff --git a/libstdc++-v3/include/bits/version.h b/libstdc++-v3/include/bits/version.h
index 9107b45a484..23c8c09ab4b 100644
--- a/libstdc++-v3/include/bits/version.h
+++ b/libstdc++-v3/include/bits/version.h
@@ -214,7 +214,7 @@
 #undef __glibcxx_want_make_reverse_iterator
 
 #if !defined(__cpp_lib_null_iterators)
-# if (__cplusplus >= 201402L) && (!defined(_GLIBCXX_DEBUG))
+# if (__cplusplus >= 201402L)
 #  define __glibcxx_null_iterators 201304L
 #  if defined(__glibcxx_want_all) || defined(__glibcxx_want_null_iterators)
 #   define __cpp_lib_null_iterators 201304L

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

[Jonathan Wakely]

On Wed, 20 Mar 2024 at 05:59, François Dumont wrote:
>
> Thanks to you doc:
>
>      libstdc++: [_GLIBCXX_DEBUG] Define __[glibcxx,cpp_lib]_null_iterators
>
>      _GLIBCXX_DEBUG has now fully N3344 compliant iterator checks, we
> can define
>      __glibcxx_null_iterators and __cpp_lib_null_iterators macros like
> the normal
>      mode.
>
>      libstdc++-v3/ChangeLog:
>
>              * version.def (null_iterators): Remove extra_cond.
>              * version.h: Regenerate.
>
> Ok to commit ?

Please don't bother talking about __glibcxx_null_iterators in the
commit message, that's an implementation detail that always mirrors
the standard-defined __cpp_lib_null_iterators one. The first line of
the commit will be much easier to read without that.

OK with that change, thanks.

> I already noticed that GCC 13 has no version.h file so no backport question.

It has no version.h but it still has the macros:

include/std/iterator:# define __cpp_lib_null_iterators 201304L
include/std/version:# define __cpp_lib_null_iterators 201304L

Those definitions can be made to not depend on _GLIBCXX_DEBUG.

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

[François Dumont]

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

As proposed below I also updated gcc-13 branch.

     libstdc++: [_GLIBCXX_DEBUG] Define __cpp_lib_null_iterators

     _GLIBCXX_DEBUG has now fully N3344 compliant iterator checks, we 
can define
     __cpp_lib_null_iterators macros like the normal mode.

     libstdc++-v3/ChangeLog:

             * include/std/iterator (__cpp_lib_null_iterators): Define 
regardless of
             _GLIBCXX_DEBUG.
             * include/std/version (__cpp_lib_null_iterators): Likewise.

François


On 20/03/2024 10:02, Jonathan Wakely wrote:
> On Wed, 20 Mar 2024 at 05:59, François Dumont wrote:
>> Thanks to you doc:
>>
>>       libstdc++: [_GLIBCXX_DEBUG] Define __[glibcxx,cpp_lib]_null_iterators
>>
>>       _GLIBCXX_DEBUG has now fully N3344 compliant iterator checks, we
>> can define
>>       __glibcxx_null_iterators and __cpp_lib_null_iterators macros like
>> the normal
>>       mode.
>>
>>       libstdc++-v3/ChangeLog:
>>
>>               * version.def (null_iterators): Remove extra_cond.
>>               * version.h: Regenerate.
>>
>> Ok to commit ?
> Please don't bother talking about __glibcxx_null_iterators in the
> commit message, that's an implementation detail that always mirrors
> the standard-defined __cpp_lib_null_iterators one. The first line of
> the commit will be much easier to read without that.
>
> OK with that change, thanks.
>
>> I already noticed that GCC 13 has no version.h file so no backport question.
> It has no version.h but it still has the macros:
>
> include/std/iterator:# define __cpp_lib_null_iterators 201304L
> include/std/version:# define __cpp_lib_null_iterators 201304L
>
> Those definitions can be made to not depend on _GLIBCXX_DEBUG.
>

[-- Attachment #2: version_patch.txt --]
[-- Type: text/plain, Size: 1073 bytes --]

diff --git a/libstdc++-v3/include/std/iterator b/libstdc++-v3/include/std/iterator
index 695e18e2c47..a0a8eac570b 100644
--- a/libstdc++-v3/include/std/iterator
+++ b/libstdc++-v3/include/std/iterator
@@ -67,7 +67,7 @@
 #endif
 #include <bits/range_access.h>
 
-#if __cplusplus >= 201402L && ! defined _GLIBCXX_DEBUG // PR libstdc++/70303
+#if __cplusplus >= 201402L
 # define __cpp_lib_null_iterators 201304L
 #endif
 
diff --git a/libstdc++-v3/include/std/version b/libstdc++-v3/include/std/version
index bd1bee0190d..ee515c4e66c 100644
--- a/libstdc++-v3/include/std/version
+++ b/libstdc++-v3/include/std/version
@@ -81,9 +81,7 @@
 #define __cpp_lib_integral_constant_callable 201304L
 #define __cpp_lib_is_final 201402L
 #define __cpp_lib_make_reverse_iterator 201402L
-#ifndef _GLIBCXX_DEBUG // PR libstdc++/70303
-# define __cpp_lib_null_iterators 201304L
-#endif
+#define __cpp_lib_null_iterators 201304L
 #define __cpp_lib_robust_nonmodifying_seq_ops 201304L
 #define __cpp_lib_transformation_trait_aliases 201304L
 #define __cpp_lib_transparent_operators 201510L

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

[Jonathan Wakely]

On Wed, 20 Mar 2024 at 18:11, François Dumont wrote:
>
> As proposed below I also updated gcc-13 branch.

Great, thanks.

>
>      libstdc++: [_GLIBCXX_DEBUG] Define __cpp_lib_null_iterators
>
>      _GLIBCXX_DEBUG has now fully N3344 compliant iterator checks, we
> can define
>      __cpp_lib_null_iterators macros like the normal mode.
>
>      libstdc++-v3/ChangeLog:
>
>              * include/std/iterator (__cpp_lib_null_iterators): Define
> regardless of
>              _GLIBCXX_DEBUG.
>              * include/std/version (__cpp_lib_null_iterators): Likewise.
>
> François
>
>
> On 20/03/2024 10:02, Jonathan Wakely wrote:
> > On Wed, 20 Mar 2024 at 05:59, François Dumont wrote:
> >> Thanks to you doc:
> >>
> >>       libstdc++: [_GLIBCXX_DEBUG] Define __[glibcxx,cpp_lib]_null_iterators
> >>
> >>       _GLIBCXX_DEBUG has now fully N3344 compliant iterator checks, we
> >> can define
> >>       __glibcxx_null_iterators and __cpp_lib_null_iterators macros like
> >> the normal
> >>       mode.
> >>
> >>       libstdc++-v3/ChangeLog:
> >>
> >>               * version.def (null_iterators): Remove extra_cond.
> >>               * version.h: Regenerate.
> >>
> >> Ok to commit ?
> > Please don't bother talking about __glibcxx_null_iterators in the
> > commit message, that's an implementation detail that always mirrors
> > the standard-defined __cpp_lib_null_iterators one. The first line of
> > the commit will be much easier to read without that.
> >
> > OK with that change, thanks.
> >
> >> I already noticed that GCC 13 has no version.h file so no backport question.
> > It has no version.h but it still has the macros:
> >
> > include/std/iterator:# define __cpp_lib_null_iterators 201304L
> > include/std/version:# define __cpp_lib_null_iterators 201304L
> >
> > Those definitions can be made to not depend on _GLIBCXX_DEBUG.
> >

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

[Jonathan Wakely]

On Sun, 17 Mar 2024 at 16:52, François Dumont <frs.dumont@gmail.com> wrote:
>
>
> >
> > OK for trunk, thanks!
> >
> > I think this is OK to backport to 13 too.
> >
> > Maybe after this we can define the __cpp_lib_null_itetators macro for
> > debug mode?
> >
> After this fix of local_iterator I think we can indeed.
>
> In fact the added 11316.cc was already passing for
> unordered_set<>::local_iterator but simply because we were missing the
> singular check. Both issues solved with this patch.
>
> I found the version.def file to cleanup but no idea how to regenerate
> version.h from it so I'll let you do it, ok ?

Sure, I can do that. To regenerate it run 'make update-version' in the
libstdc++-v3/include build directory.

>
>      libstdc++: Fix _Safe_local_iterator<>::_M_valid_range
>
>      Unordered container local_iterator range shall not contain any singular
>      iterator unless both iterators are value-initialized.
>
>      libstdc++-v3/ChangeLog:
>
>              * include/debug/safe_local_iterator.tcc
>              (_Safe_local_iterator::_M_valid_range): Add
> _M_value_initialized and
>              _M_singular checks.
>              * testsuite/23_containers/unordered_set/debug/114316.cc:
> New test case.
>
>
> Ok to commit ?

OK.