Re: [PATCH] libstdc++/98466 Fix _GLIBCXX_DEBUG N3644 integration

[Jonathan Wakely via Gcc-patches]

On 14/01/21 19:33 +0100, François Dumont wrote:

On 14/01/21 6:10 pm, Jonathan Wakely wrote:

On 01/01/21 18:51 +0100, François Dumont via Libstdc++ wrote:

I think the PR is not limited to unordered containers iterator, it impacts all _GLIBCXX_DEBUG iterators.

However unordered containers local_iterator was more complicated to handle. Because of c++/65816 I prefer to review _Node_iterator_default constructor to set _M_cur to nullptr even if in principle it is not necessary except for the _Local_iterator_base constructor when hash code is not cached.

    libstdc++: Implement N3644 for _GLIBCXX_DEBUG iterators

    libstdc++-v3/ChangeLog

            PR libstdc++/98466

            * include/bits/hashtable_policy.h (_Node_iterator_base()): Set _M_cur to nullptr.

            (_Node_iterator()): Make default.
            (_Node_const_iterator()): Make 
default.

            * include/debug/macros.h (__glibcxx_check_erae_range_after): Add _M_singular

            iterator checks.
            * include/debug/safe_iterator.h

            (_GLIBCXX_DEBUG_VERIFY_OPERANDS): Accept if both iterator are value initialized.             * include/debug/safe_local_iterator.h (_GLIBCXX_DEBUG_VERIFY_OPERANDS):

            Likewise.

            * include/debug/safe_iterator.tcc (_Safe_iterator<>::_M_valid_range): Add

            _M_singular checks on input 
iterators.

            * src/c++11/debug.cc (_Safe_iterator_base::_M_can_compare): Remove _M_singular

            checks.

            * testsuite/23_containers/deque/debug/98466.cc: New test.             * testsuite/23_containers/unordered_map/debug/98466.cc: New test.

Tested under Linux x86_64 normal and debug mode.

Ok to commit ?

Yes, thanks.

One question about the deque test ...

diff --git a/libstdc++-v3/testsuite/23_containers/deque/debug/98466.cc b/libstdc++-v3/testsuite/23_containers/deque/debug/98466.cc

new file mode 100644
index 00000000000..720977e5622
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/deque/debug/98466.cc
@@ -0,0 +1,38 @@
+// Copyright (C) 2021 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+

+// You should have received a copy of the GNU General Public License along

+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do run { target c++11 } }

Does this need to be limited to c++11 and later? Could it just use
{ dg-do run } instead?

Good point, a bad copy/paste from the unordered test I guess.

But I try to remove it and it complained about invalid '{}' syntax in C++98 for the iterator value initialization. I try to replace with '()' but then ambiguity with a function declaration, I gave up !

As N3644 is talking about value initialization it doesn't sound that bad to limit it to C++11, isn't it a C++11 concept ?

No, it was added in C++03.

You can do this to avoid it being treated as a function declaration:

__gnu_debug::deque<int>::iterator it = __gnu_debug::deque<int>::iterator();


OK to commit with that change if it passes in C++98 mode.