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 <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!
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;
+}
