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 <jwak...@redhat.com> wrote:
On Tue, 12 Mar 2024 at 00:55, Maciej Miera <maciej.mi...@gmail.com> wrote:
Wiadomość napisana przez Jonathan Wakely <jwak...@redhat.com> w dniu
11.03.2024, o godz. 21:40:
On Mon, 11 Mar 2024 at 20:07, Maciej Miera <maciej.mi...@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!
