public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] libstdc++/98466 Fix _GLIBCXX_DEBUG N3644 integration
@ 2021-01-01 17:51 François Dumont
  2021-01-14 17:10 ` Jonathan Wakely
  0 siblings, 1 reply; 6+ messages in thread
From: François Dumont @ 2021-01-01 17:51 UTC (permalink / raw)
  To: libstdc++, gcc-patches

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

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 ?

François


[-- Attachment #2: n3644.patch --]
[-- Type: text/x-patch, Size: 7749 bytes --]

diff --git a/libstdc++-v3/include/bits/hashtable_policy.h b/libstdc++-v3/include/bits/hashtable_policy.h
index 28372979c87..96d87076ba5 100644
--- a/libstdc++-v3/include/bits/hashtable_policy.h
+++ b/libstdc++-v3/include/bits/hashtable_policy.h
@@ -290,7 +290,7 @@ namespace __detail
 
       __node_type* _M_cur;
 
-      _Node_iterator_base() = default;
+      _Node_iterator_base() : _M_cur(nullptr) { }
       _Node_iterator_base(__node_type* __p) noexcept
       : _M_cur(__p) { }
 
@@ -331,8 +331,7 @@ namespace __detail
       using reference = typename std::conditional<__constant_iterators,
 				  const value_type&, value_type&>::type;
 
-      _Node_iterator() noexcept
-      : __base_type(nullptr) { }
+      _Node_iterator() = default;
 
       explicit
       _Node_iterator(__node_type* __p) noexcept
@@ -379,8 +378,7 @@ namespace __detail
       typedef const value_type*				pointer;
       typedef const value_type&				reference;
 
-      _Node_const_iterator() noexcept
-      : __base_type(nullptr) { }
+      _Node_const_iterator() = default;
 
       explicit
       _Node_const_iterator(__node_type* __p) noexcept
diff --git a/libstdc++-v3/include/debug/macros.h b/libstdc++-v3/include/debug/macros.h
index a69310d9a12..ba19dbead66 100644
--- a/libstdc++-v3/include/debug/macros.h
+++ b/libstdc++-v3/include/debug/macros.h
@@ -244,6 +244,11 @@ _GLIBCXX_DEBUG_VERIFY(_First._M_attached_to(this),			\
  *  valid iterator range within this sequence.
 */
 #define __glibcxx_check_erase_range_after(_First,_Last)			\
+_GLIBCXX_DEBUG_VERIFY(!_First._M_singular() && !_Last._M_singular(),	\
+		      _M_message(__gnu_debug::__msg_erase_different)	\
+		      ._M_sequence(*this, "this")			\
+		      ._M_iterator(_First, #_First)			\
+		      ._M_iterator(_Last, #_Last));			\
 _GLIBCXX_DEBUG_VERIFY(_First._M_can_compare(_Last),			\
 		      _M_message(__gnu_debug::__msg_erase_different)	\
 		      ._M_sequence(*this, "this")			\
diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h
index 9b77fac6478..ffd8b23d06d 100644
--- a/libstdc++-v3/include/debug/safe_iterator.h
+++ b/libstdc++-v3/include/debug/safe_iterator.h
@@ -40,7 +40,9 @@
 #endif
 
 #define _GLIBCXX_DEBUG_VERIFY_OPERANDS(_Lhs, _Rhs, _BadMsgId, _DiffMsgId) \
-  _GLIBCXX_DEBUG_VERIFY(!_Lhs._M_singular() && !_Rhs._M_singular(),	\
+  _GLIBCXX_DEBUG_VERIFY(!_Lhs._M_singular() && !_Rhs._M_singular()	\
+			|| (_Lhs.base() == _Iterator()			\
+			    && _Rhs.base() == _Iterator()),		\
 			_M_message(_BadMsgId)				\
 			._M_iterator(_Lhs, #_Lhs)			\
 			._M_iterator(_Rhs, #_Rhs));			\
diff --git a/libstdc++-v3/include/debug/safe_iterator.tcc b/libstdc++-v3/include/debug/safe_iterator.tcc
index f694e514239..b9f9e39847b 100644
--- a/libstdc++-v3/include/debug/safe_iterator.tcc
+++ b/libstdc++-v3/include/debug/safe_iterator.tcc
@@ -186,7 +186,7 @@ namespace __gnu_debug
 		   std::pair<difference_type, _Distance_precision>& __dist,
 		   bool __check_dereferenceable) const
     {
-      if (!_M_can_compare(__rhs))
+      if (_M_singular() || __rhs._M_singular() || !_M_can_compare(__rhs))
 	return false;
 
       /* Determine iterators order */
@@ -217,7 +217,8 @@ namespace __gnu_debug
 		   std::pair<difference_type,
 			     _Distance_precision>& __dist) const
     {
-      if (!this->_M_can_compare(__rhs))
+      if (this->_M_singular() || __rhs._M_singular()
+	  || !this->_M_can_compare(__rhs))
 	return false;
 
       /* Determine iterators order */
diff --git a/libstdc++-v3/include/debug/safe_local_iterator.h b/libstdc++-v3/include/debug/safe_local_iterator.h
index 5b051d0ebf9..ea1c1e8dd75 100644
--- a/libstdc++-v3/include/debug/safe_local_iterator.h
+++ b/libstdc++-v3/include/debug/safe_local_iterator.h
@@ -32,7 +32,9 @@
 #include <debug/safe_unordered_base.h>
 
 #define _GLIBCXX_DEBUG_VERIFY_OPERANDS(_Lhs, _Rhs) \
-  _GLIBCXX_DEBUG_VERIFY(!_Lhs._M_singular() && !_Rhs._M_singular(),	\
+  _GLIBCXX_DEBUG_VERIFY(!_Lhs._M_singular() && !_Rhs._M_singular()	\
+			|| (_Lhs.base() == _Iterator{}			\
+			    && _Rhs.base() == _Iterator{}),		\
 			_M_message(__msg_iter_compare_bad)		\
 			._M_iterator(_Lhs, "lhs")			\
 			._M_iterator(_Rhs, "rhs"));			\
diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc
index 18da9da9c52..ab2f2d855b2 100644
--- a/libstdc++-v3/src/c++11/debug.cc
+++ b/libstdc++-v3/src/c++11/debug.cc
@@ -424,10 +424,7 @@ namespace __gnu_debug
   bool
   _Safe_iterator_base::
   _M_can_compare(const _Safe_iterator_base& __x) const throw ()
-  {
-    return (!_M_singular()
-	    && !__x._M_singular() && _M_sequence == __x._M_sequence);
-  }
+  { return _M_sequence == __x._M_sequence; }
 
   __gnu_cxx::__mutex&
   _Safe_iterator_base::
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 } }
+
+#include <debug/deque>
+#include <testsuite_hooks.h>
+
+// PR libstdc++/98466
+
+void test01()
+{
+  __gnu_debug::deque<int>::iterator it{};
+  VERIFY( it == it );
+
+  __gnu_debug::deque<int>::const_iterator cit{};
+  VERIFY( cit == cit );
+}
+
+int main()
+{
+  test01();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/23_containers/unordered_map/debug/98466.cc b/libstdc++-v3/testsuite/23_containers/unordered_map/debug/98466.cc
new file mode 100644
index 00000000000..cc22b9ff80a
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/unordered_map/debug/98466.cc
@@ -0,0 +1,44 @@
+// 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 } }
+
+#include <debug/unordered_map>
+#include <testsuite_hooks.h>
+
+// PR libstdc++/98466
+
+void test01()
+{
+  __gnu_debug::unordered_map<int, int>::iterator it{};
+  VERIFY( it == it );
+
+  __gnu_debug::unordered_map<int, int>::const_iterator cit{};
+  VERIFY( cit == cit );
+
+  __gnu_debug::unordered_map<int, int>::local_iterator lit{};
+  VERIFY( lit == lit );
+
+  __gnu_debug::unordered_map<int, int>::const_local_iterator clit{};
+  VERIFY( clit == clit );
+}
+
+int main()
+{
+  test01();
+  return 0;
+}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libstdc++/98466 Fix _GLIBCXX_DEBUG N3644 integration
  2021-01-01 17:51 [PATCH] libstdc++/98466 Fix _GLIBCXX_DEBUG N3644 integration François Dumont
@ 2021-01-14 17:10 ` Jonathan Wakely
  2021-01-14 17:15   ` Jonathan Wakely
  2021-01-14 18:33   ` François Dumont
  0 siblings, 2 replies; 6+ messages in thread
From: Jonathan Wakely @ 2021-01-14 17:10 UTC (permalink / raw)
  To: François Dumont; +Cc: libstdc++, gcc-patches

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?

OK to commit anyway, thanks.




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libstdc++/98466 Fix _GLIBCXX_DEBUG N3644 integration
  2021-01-14 17:10 ` Jonathan Wakely
@ 2021-01-14 17:15   ` Jonathan Wakely
  2021-01-14 18:34     ` François Dumont
  2021-01-14 18:33   ` François Dumont
  1 sibling, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2021-01-14 17:15 UTC (permalink / raw)
  To: François Dumont; +Cc: libstdc++, gcc-patches

On 14/01/21 17:10 +0000, 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.

I've just realised that this C++14 change used to be noted in the
C++14 status table:

     <row>
       <?dbhtml bgcolor="#B0B0B0" ?>
       <entry>
        <link xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href="http://www.open-std.org/JTC1/sc22/WG21/docs/papers/2013/n3644.pdf">
          N3644
        </link>
       </entry>
       <entry>Null Forward Iterators</entry>
       <entry>Partial</entry>
       <entry>Only affects Debug Mode</entry>
      </row>

But I removed that last year when replacing the list of proposals with
the Table of Contents taken from the standard, in commit
57ede05c6a0b443943e312bf205cb79233c9396f (oops!)

For the branches we should either document that missing feature in a
note, or backport your fix in a few weeks.




^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libstdc++/98466 Fix _GLIBCXX_DEBUG N3644 integration
  2021-01-14 17:10 ` Jonathan Wakely
  2021-01-14 17:15   ` Jonathan Wakely
@ 2021-01-14 18:33   ` François Dumont
  2021-01-14 19:20     ` Jonathan Wakely
  1 sibling, 1 reply; 6+ messages in thread
From: François Dumont @ 2021-01-14 18:33 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches

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 ?


> OK to commit anyway, thanks.
>
>
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libstdc++/98466 Fix _GLIBCXX_DEBUG N3644 integration
  2021-01-14 17:15   ` Jonathan Wakely
@ 2021-01-14 18:34     ` François Dumont
  0 siblings, 0 replies; 6+ messages in thread
From: François Dumont @ 2021-01-14 18:34 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches

On 14/01/21 6:15 pm, Jonathan Wakely wrote:
> On 14/01/21 17:10 +0000, 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.
>
> I've just realised that this C++14 change used to be noted in the
> C++14 status table:
>
>     <row>
>       <?dbhtml bgcolor="#B0B0B0" ?>
>       <entry>
>        <link xmlns:xlink="http://www.w3.org/1999/xlink" 
> xlink:href="http://www.open-std.org/JTC1/sc22/WG21/docs/papers/2013/n3644.pdf">
>          N3644
>        </link>
>       </entry>
>       <entry>Null Forward Iterators</entry>
>       <entry>Partial</entry>
>       <entry>Only affects Debug Mode</entry>
>      </row>
>
> But I removed that last year when replacing the list of proposals with
> the Table of Contents taken from the standard, in commit
> 57ede05c6a0b443943e312bf205cb79233c9396f (oops!)
>
> For the branches we should either document that missing feature in a
> note, or backport your fix in a few weeks.
>
>
>
I'll keep the backport in my TODOs.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libstdc++/98466 Fix _GLIBCXX_DEBUG N3644 integration
  2021-01-14 18:33   ` François Dumont
@ 2021-01-14 19:20     ` Jonathan Wakely
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Wakely @ 2021-01-14 19:20 UTC (permalink / raw)
  To: François Dumont; +Cc: libstdc++, 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.



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-01-14 19:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-01 17:51 [PATCH] libstdc++/98466 Fix _GLIBCXX_DEBUG N3644 integration François Dumont
2021-01-14 17:10 ` Jonathan Wakely
2021-01-14 17:15   ` Jonathan Wakely
2021-01-14 18:34     ` François Dumont
2021-01-14 18:33   ` François Dumont
2021-01-14 19:20     ` 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).