* _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms @ 2024-03-11 20:07 Maciej Miera 2024-03-11 20:40 ` Jonathan Wakely 0 siblings, 1 reply; 19+ messages in thread From: Maciej Miera @ 2024-03-11 20:07 UTC (permalink / raw) To: libstdc++ 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms 2024-03-11 20:07 _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms Maciej Miera @ 2024-03-11 20:40 ` Jonathan Wakely 2024-03-12 0:54 ` Maciej Miera 0 siblings, 1 reply; 19+ messages in thread From: Jonathan Wakely @ 2024-03-11 20:40 UTC (permalink / raw) To: Maciej Miera; +Cc: libstdc++ 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms 2024-03-11 20:40 ` Jonathan Wakely @ 2024-03-12 0:54 ` Maciej Miera 2024-03-12 1:03 ` Jonathan Wakely 0 siblings, 1 reply; 19+ messages in thread From: Maciej Miera @ 2024-03-12 0:54 UTC (permalink / raw) To: libstdc++ [-- 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms 2024-03-12 0:54 ` Maciej Miera @ 2024-03-12 1:03 ` Jonathan Wakely 2024-03-12 9:52 ` Jonathan Wakely 0 siblings, 1 reply; 19+ messages in thread From: Jonathan Wakely @ 2024-03-12 1:03 UTC (permalink / raw) To: Maciej Miera; +Cc: libstdc++ 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms 2024-03-12 1:03 ` Jonathan Wakely @ 2024-03-12 9:52 ` Jonathan Wakely 2024-03-14 21:49 ` _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms [PR104316] François Dumont 0 siblings, 1 reply; 19+ messages in thread From: Jonathan Wakely @ 2024-03-12 9:52 UTC (permalink / raw) To: Maciej Miera; +Cc: libstdc++ 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! ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms [PR104316] 2024-03-12 9:52 ` Jonathan Wakely @ 2024-03-14 21:49 ` François Dumont 2024-03-16 12:16 ` François Dumont 0 siblings, 1 reply; 19+ messages in thread From: François Dumont @ 2024-03-14 21:49 UTC (permalink / raw) To: Jonathan Wakely, Maciej Miera; +Cc: libstdc++, gcc-patches 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! > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms [PR104316] 2024-03-14 21:49 ` _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms [PR104316] François Dumont @ 2024-03-16 12:16 ` François Dumont 2024-03-17 11:45 ` Jonathan Wakely 0 siblings, 1 reply; 19+ messages in thread From: François Dumont @ 2024-03-16 12:16 UTC (permalink / raw) To: Jonathan Wakely, Maciej Miera; +Cc: libstdc++, gcc-patches [-- 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; +} ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms [PR104316] 2024-03-16 12:16 ` François Dumont @ 2024-03-17 11:45 ` Jonathan Wakely 2024-03-17 16:52 ` François Dumont 0 siblings, 1 reply; 19+ messages in thread From: Jonathan Wakely @ 2024-03-17 11:45 UTC (permalink / raw) To: François Dumont Cc: Jonathan Wakely, Maciej Miera, libstdc++, gcc-patches [-- 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! > >> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms [PR104316] 2024-03-17 11:45 ` Jonathan Wakely @ 2024-03-17 16:52 ` François Dumont 2024-03-17 18:14 ` François Dumont 2024-03-18 7:45 ` Jonathan Wakely 0 siblings, 2 replies; 19+ messages in thread From: François Dumont @ 2024-03-17 16:52 UTC (permalink / raw) To: Jonathan Wakely; +Cc: Jonathan Wakely, libstdc++, gcc-patches [-- 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; +} ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms [PR104316] 2024-03-17 16:52 ` François Dumont @ 2024-03-17 18:14 ` François Dumont 2024-03-18 7:45 ` Jonathan Wakely 2024-03-18 7:45 ` Jonathan Wakely 1 sibling, 1 reply; 19+ messages in thread From: François Dumont @ 2024-03-17 18:14 UTC (permalink / raw) To: Jonathan Wakely; +Cc: Jonathan Wakely, libstdc++, gcc-patches [-- 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; +} ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms [PR104316] 2024-03-17 18:14 ` François Dumont @ 2024-03-18 7:45 ` Jonathan Wakely 2024-03-18 21:38 ` François Dumont 0 siblings, 1 reply; 19+ messages in thread From: Jonathan Wakely @ 2024-03-18 7:45 UTC (permalink / raw) To: François Dumont; +Cc: Jonathan Wakely, libstdc++, gcc-patches 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms [PR104316] 2024-03-18 7:45 ` Jonathan Wakely @ 2024-03-18 21:38 ` François Dumont 2024-03-19 9:31 ` Jonathan Wakely 0 siblings, 1 reply; 19+ messages in thread From: François Dumont @ 2024-03-18 21:38 UTC (permalink / raw) To: Jonathan Wakely; +Cc: Jonathan Wakely, libstdc++, gcc-patches 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms [PR104316] 2024-03-18 21:38 ` François Dumont @ 2024-03-19 9:31 ` Jonathan Wakely 2024-03-19 15:41 ` Jonathan Wakely 0 siblings, 1 reply; 19+ messages in thread From: Jonathan Wakely @ 2024-03-19 9:31 UTC (permalink / raw) To: François Dumont; +Cc: Jonathan Wakely, libstdc++, gcc-patches 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). ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms [PR104316] 2024-03-19 9:31 ` Jonathan Wakely @ 2024-03-19 15:41 ` Jonathan Wakely 2024-03-20 5:59 ` François Dumont 0 siblings, 1 reply; 19+ messages in thread From: Jonathan Wakely @ 2024-03-19 15:41 UTC (permalink / raw) To: François Dumont; +Cc: Jonathan Wakely, libstdc++, gcc-patches 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms [PR104316] 2024-03-19 15:41 ` Jonathan Wakely @ 2024-03-20 5:59 ` François Dumont 2024-03-20 9:02 ` Jonathan Wakely 0 siblings, 1 reply; 19+ messages in thread From: François Dumont @ 2024-03-20 5:59 UTC (permalink / raw) To: Jonathan Wakely; +Cc: Jonathan Wakely, libstdc++, gcc-patches [-- 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms [PR104316] 2024-03-20 5:59 ` François Dumont @ 2024-03-20 9:02 ` Jonathan Wakely 2024-03-20 18:10 ` François Dumont 0 siblings, 1 reply; 19+ messages in thread From: Jonathan Wakely @ 2024-03-20 9:02 UTC (permalink / raw) To: François Dumont; +Cc: Jonathan Wakely, libstdc++, gcc-patches 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms [PR104316] 2024-03-20 9:02 ` Jonathan Wakely @ 2024-03-20 18:10 ` François Dumont 2024-03-21 6:20 ` Jonathan Wakely 0 siblings, 1 reply; 19+ messages in thread From: François Dumont @ 2024-03-20 18:10 UTC (permalink / raw) To: Jonathan Wakely; +Cc: Jonathan Wakely, libstdc++, gcc-patches [-- 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms [PR104316] 2024-03-20 18:10 ` François Dumont @ 2024-03-21 6:20 ` Jonathan Wakely 0 siblings, 0 replies; 19+ messages in thread From: Jonathan Wakely @ 2024-03-21 6:20 UTC (permalink / raw) To: François Dumont; +Cc: Jonathan Wakely, libstdc++, gcc-patches 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. > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms [PR104316] 2024-03-17 16:52 ` François Dumont 2024-03-17 18:14 ` François Dumont @ 2024-03-18 7:45 ` Jonathan Wakely 1 sibling, 0 replies; 19+ messages in thread From: Jonathan Wakely @ 2024-03-18 7:45 UTC (permalink / raw) To: François Dumont; +Cc: Jonathan Wakely, libstdc++, gcc-patches 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2024-03-21 6:20 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-03-11 20:07 _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms Maciej Miera 2024-03-11 20:40 ` Jonathan Wakely 2024-03-12 0:54 ` Maciej Miera 2024-03-12 1:03 ` Jonathan Wakely 2024-03-12 9:52 ` Jonathan Wakely 2024-03-14 21:49 ` _LIBCXX_DEBUG value initialized singular iterators assert failures in std algorithms [PR104316] François Dumont 2024-03-16 12:16 ` François Dumont 2024-03-17 11:45 ` Jonathan Wakely 2024-03-17 16:52 ` François Dumont 2024-03-17 18:14 ` François Dumont 2024-03-18 7:45 ` Jonathan Wakely 2024-03-18 21:38 ` François Dumont 2024-03-19 9:31 ` Jonathan Wakely 2024-03-19 15:41 ` Jonathan Wakely 2024-03-20 5:59 ` François Dumont 2024-03-20 9:02 ` Jonathan Wakely 2024-03-20 18:10 ` François Dumont 2024-03-21 6:20 ` Jonathan Wakely 2024-03-18 7:45 ` Jonathan Wakely
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).