Re: [PATCH] __debug::list use C++11 direct initialization
I've just reverted the controversial changes.
2018-10-18 François Dumont
Partial revert.
2018-10-08 François Dumont
* include/debug/list (list<>::cbegin()): Use C++11 direct
initialization.
(list<>::cend()): Likewise.
(list<>::erase(const_iterator, const_iterator)): Ensure consistent
iterator comparisons.
(list<>::splice(const_iterator, list&&, const_iterator,
const_iterator)): Likewise.
Partial revert.
2018-10-15 François Dumont
* include/debug/vector (vector<>::cbegin()): Use C++11 direct
initialization.
(vector<>::cend()): Likewise.
(vector<>::insert(const_iterator, const _Tp&)): Use consistent
iterator comparison.
(vector<>::erase(const_iterator)): Likewise.
(vector<>::erase(const_iterator, const_iterator)): Likewise.
François
On 10/17/2018 06:10 PM, Jonathan Wakely wrote:
On 17/10/18 17:03 +0100, Jonathan Wakely wrote:
On 09/10/18 07:11 +0200, François Dumont wrote:
Here is the communication for my yesterday's patch which I thought
svn had failed to commit (I had to interrupt it).
Similarly to what I've done for associative containers here is a
cleanup of the std::__debug::list implementation leveraging more on
C++11 direct initialization.
I also made sure we use consistent comparison between
iterator/const_iterator in erase and emplace methods.
2018-10-08 François Dumont
* include/debug/list (list<>::cbegin()): Use C++11 direct
initialization.
(list<>::cend()): Likewise.
(list<>::emplace<>(const_iterator, _Args&&...)): Likewise.
(list<>::insert(const_iterator, initializer_list<>)): Likewise.
(list<>::insert(const_iterator, size_type, const _Tp&)): Likewise.
(list<>::erase(const_iterator, const_iterator)): Ensure consistent
iterator comparisons.
(list<>::splice(const_iterator, list&&, const_iterator,
const_iterator)): Likewise.
Tested under Linux x86_64 Debug mode and committed.
François
diff --git a/libstdc++-v3/include/debug/list
b/libstdc++-v3/include/debug/list
index 8add1d596e0..879e1177497 100644
--- a/libstdc++-v3/include/debug/list
+++ b/libstdc++-v3/include/debug/list
@@ -521,15 +521,17 @@ namespace __debug
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// 151. can't currently clear() empty container
__glibcxx_check_erase_range(__first, __last);
- for (_Base_const_iterator __victim = __first.base();
+ for (__decltype(__first.base()) __victim = __first.base();
This change causes the regression.
I think the problem is that the decltype is a reference, so every time
the loop does ++__victim it changes the iterator stored in __first.
This would work:
typedef typename __decltype(__first)::iterator_type _Base_iter;
for (_Base_iter __victim = __first.base();
__victim != __last.base(); ++__victim)
Or simply:
#if __cplusplus >= 201103L
typedef _Base_const_iterator _Base_iter;
#else
typedef _Base_iterator _Base_iter;
#endif
for (_Base_iter __victim = __first.base();
__victim != __last.base(); ++__victim)
Or just restore the original code which worked fine!
.
diff --git a/libstdc++-v3/include/debug/list b/libstdc++-v3/include/debug/list
index 879e1177497..aab146d058b 100644
--- a/libstdc++-v3/include/debug/list
+++ b/libstdc++-v3/include/debug/list
@@ -244,11 +244,11 @@ namespace __debug
#if __cplusplus >= 201103L
const_iterator
cbegin() const noexcept
- { return { _Base::begin(), this }; }
+ { return const_iterator(_Base::begin(), this); }
const_iterator
cend() const noexcept
- { return { _Base::end(), this }; }
+ { return const_iterator(_Base::end(), this); }
const_reverse_iterator
crbegin() const noexcept
@@ -521,14 +521,13 @@ namespace __debug
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// 151. can't currently clear() empty container
__glibcxx_check_erase_range(__first, __last);
- for (__decltype(__first.base()) __victim = __first.base();
+ for (_Base_const_iterator __victim = __first.base();
__victim != __last.base(); ++__victim)
{
- _GLIBCXX_DEBUG_VERIFY(
- __victim != __first._M_get_sequence()->_M_base().end(),
- _M_message(__gnu_debug::__msg_valid_range)
- ._M_iterator(__first, "position")
- ._M_iterator(__last, "last"));
+ _GLIBCXX_DEBUG_VERIFY(__victim != _Base::end(),
+ _M_message(__gnu_debug::__msg_valid_range)
+ ._M_iterator(__first, "position")
+ ._M_iterator(__last, "last"));
this->_M_invalidate_if(_Equal(__victim));
}
@@ -622,14 +621,13 @@ namespace __debug
// We used to perform the splice_alloc check: not anymore, redundant
// after implementing the relevant bits of N1599.
- for (__decltype(__first.base()) __tmp = __first.base();
+ for (_Base_const_iterator __tmp = __first.base();
__tmp != __last.base(); ++__tmp)
{
- _GLIBCXX_DEBUG_VERIFY(
- __tmp != __first._M_get_sequence()->_M_base().end(),
- _M_message(__gnu_debug::__msg_valid_range)
-
Re: [PATCH] __debug::list use C++11 direct initialization
On 16/10/18 07:06 +0200, François Dumont wrote:
On 10/15/2018 12:07 PM, Jonathan Wakely wrote:
On 09/10/18 07:11 +0200, François Dumont wrote:
Here is the communication for my yesterday's patch which I thought
svn had failed to commit (I had to interrupt it).
Similarly to what I've done for associative containers here is a
cleanup of the std::__debug::list implementation leveraging more
on C++11 direct initialization.
I also made sure we use consistent comparison between
iterator/const_iterator in erase and emplace methods.
2018-10-08 François Dumont
* include/debug/list (list<>::cbegin()): Use C++11 direct
initialization.
(list<>::cend()): Likewise.
(list<>::emplace<>(const_iterator, _Args&&...)): Likewise.
(list<>::insert(const_iterator, initializer_list<>)): Likewise.
(list<>::insert(const_iterator, size_type, const _Tp&)): Likewise.
(list<>::erase(const_iterator, const_iterator)): Ensure consistent
iterator comparisons.
(list<>::splice(const_iterator, list&&, const_iterator,
const_iterator)): Likewise.
Tested under Linux x86_64 Debug mode and committed.
François
diff --git a/libstdc++-v3/include/debug/list
b/libstdc++-v3/include/debug/list
index 8add1d596e0..879e1177497 100644
--- a/libstdc++-v3/include/debug/list
+++ b/libstdc++-v3/include/debug/list
@@ -244,11 +244,11 @@ namespace __debug
#if __cplusplus >= 201103L
const_iterator
cbegin() const noexcept
- { return const_iterator(_Base::begin(), this); }
+ { return { _Base::begin(), this }; }
const_iterator
cend() const noexcept
- { return const_iterator(_Base::end(), this); }
+ { return { _Base::end(), this }; }
For functions like emplace (which are C++11-only) and for forward_list
(also C++11-only) using this syntax makes it clearer.
But for these functions it just makes cbegin() and cend() look
different to the C++98 begin() and end() functions, for no obvious
benefit.
Simply using { return end(); } would have been another option.
Personnaly I hesitated in writting:
{ return { _Base::cbegin(), this }; }
cause I prefer when you see clearly that Debug implem forward calls to
the Normal implem.
I thought that using C++11 direct init was more likely to have gcc
elide the copy constructor so I used it everywhere possible.
It should make no difference to elision at all.
Re: [PATCH] __debug::list use C++11 direct initialization
On 10/15/2018 12:07 PM, Jonathan Wakely wrote:
On 09/10/18 07:11 +0200, François Dumont wrote:
Here is the communication for my yesterday's patch which I thought
svn had failed to commit (I had to interrupt it).
Similarly to what I've done for associative containers here is a
cleanup of the std::__debug::list implementation leveraging more on
C++11 direct initialization.
I also made sure we use consistent comparison between
iterator/const_iterator in erase and emplace methods.
2018-10-08 François Dumont
* include/debug/list (list<>::cbegin()): Use C++11 direct
initialization.
(list<>::cend()): Likewise.
(list<>::emplace<>(const_iterator, _Args&&...)): Likewise.
(list<>::insert(const_iterator, initializer_list<>)): Likewise.
(list<>::insert(const_iterator, size_type, const _Tp&)): Likewise.
(list<>::erase(const_iterator, const_iterator)): Ensure consistent
iterator comparisons.
(list<>::splice(const_iterator, list&&, const_iterator,
const_iterator)): Likewise.
Tested under Linux x86_64 Debug mode and committed.
François
diff --git a/libstdc++-v3/include/debug/list
b/libstdc++-v3/include/debug/list
index 8add1d596e0..879e1177497 100644
--- a/libstdc++-v3/include/debug/list
+++ b/libstdc++-v3/include/debug/list
@@ -244,11 +244,11 @@ namespace __debug
#if __cplusplus >= 201103L
const_iterator
cbegin() const noexcept
- { return const_iterator(_Base::begin(), this); }
+ { return { _Base::begin(), this }; }
const_iterator
cend() const noexcept
- { return const_iterator(_Base::end(), this); }
+ { return { _Base::end(), this }; }
For functions like emplace (which are C++11-only) and for forward_list
(also C++11-only) using this syntax makes it clearer.
But for these functions it just makes cbegin() and cend() look
different to the C++98 begin() and end() functions, for no obvious
benefit.
Simply using { return end(); } would have been another option.
Personnaly I hesitated in writting:
{ return { _Base::cbegin(), this }; }
cause I prefer when you see clearly that Debug implem forward calls to
the Normal implem.
I thought that using C++11 direct init was more likely to have gcc elide
the copy constructor so I used it everywhere possible.
Re: [PATCH] __debug::list use C++11 direct initialization
On 09/10/18 07:11 +0200, François Dumont wrote:
Here is the communication for my yesterday's patch which I thought svn
had failed to commit (I had to interrupt it).
Similarly to what I've done for associative containers here is a
cleanup of the std::__debug::list implementation leveraging more on
C++11 direct initialization.
I also made sure we use consistent comparison between
iterator/const_iterator in erase and emplace methods.
2018-10-08 François Dumont
* include/debug/list (list<>::cbegin()): Use C++11 direct
initialization.
(list<>::cend()): Likewise.
(list<>::emplace<>(const_iterator, _Args&&...)): Likewise.
(list<>::insert(const_iterator, initializer_list<>)): Likewise.
(list<>::insert(const_iterator, size_type, const _Tp&)): Likewise.
(list<>::erase(const_iterator, const_iterator)): Ensure consistent
iterator comparisons.
(list<>::splice(const_iterator, list&&, const_iterator,
const_iterator)): Likewise.
Tested under Linux x86_64 Debug mode and committed.
François
diff --git a/libstdc++-v3/include/debug/list b/libstdc++-v3/include/debug/list
index 8add1d596e0..879e1177497 100644
--- a/libstdc++-v3/include/debug/list
+++ b/libstdc++-v3/include/debug/list
@@ -244,11 +244,11 @@ namespace __debug
#if __cplusplus >= 201103L
const_iterator
cbegin() const noexcept
- { return const_iterator(_Base::begin(), this); }
+ { return { _Base::begin(), this }; }
const_iterator
cend() const noexcept
- { return const_iterator(_Base::end(), this); }
+ { return { _Base::end(), this }; }
For functions like emplace (which are C++11-only) and for forward_list
(also C++11-only) using this syntax makes it clearer.
But for these functions it just makes cbegin() and cend() look
different to the C++98 begin() and end() functions, for no obvious
benefit.
Simply using { return end(); } would have been another option.
[PATCH] __debug::list use C++11 direct initialization
Here is the communication for my yesterday's patch which I thought svn
had failed to commit (I had to interrupt it).
Similarly to what I've done for associative containers here is a cleanup
of the std::__debug::list implementation leveraging more on C++11 direct
initialization.
I also made sure we use consistent comparison between
iterator/const_iterator in erase and emplace methods.
2018-10-08 François Dumont
* include/debug/list (list<>::cbegin()): Use C++11 direct
initialization.
(list<>::cend()): Likewise.
(list<>::emplace<>(const_iterator, _Args&&...)): Likewise.
(list<>::insert(const_iterator, initializer_list<>)): Likewise.
(list<>::insert(const_iterator, size_type, const _Tp&)): Likewise.
(list<>::erase(const_iterator, const_iterator)): Ensure consistent
iterator comparisons.
(list<>::splice(const_iterator, list&&, const_iterator,
const_iterator)): Likewise.
Tested under Linux x86_64 Debug mode and committed.
François
diff --git a/libstdc++-v3/include/debug/list b/libstdc++-v3/include/debug/list
index 8add1d596e0..879e1177497 100644
--- a/libstdc++-v3/include/debug/list
+++ b/libstdc++-v3/include/debug/list
@@ -244,11 +244,11 @@ namespace __debug
#if __cplusplus >= 201103L
const_iterator
cbegin() const noexcept
- { return const_iterator(_Base::begin(), this); }
+ { return { _Base::begin(), this }; }
const_iterator
cend() const noexcept
- { return const_iterator(_Base::end(), this); }
+ { return { _Base::end(), this }; }
const_reverse_iterator
crbegin() const noexcept
@@ -405,8 +405,8 @@ namespace __debug
emplace(const_iterator __position, _Args&&... __args)
{
__glibcxx_check_insert(__position);
- return iterator(_Base::emplace(__position.base(),
- std::forward<_Args>(__args)...), this);
+ return { _Base::emplace(__position.base(),
+ std::forward<_Args>(__args)...), this };
}
#endif
@@ -430,7 +430,7 @@ namespace __debug
insert(const_iterator __p, initializer_list __l)
{
__glibcxx_check_insert(__p);
- return iterator(_Base::insert(__p.base(), __l), this);
+ return { _Base::insert(__p.base(), __l), this };
}
#endif
@@ -439,7 +439,7 @@ namespace __debug
insert(const_iterator __position, size_type __n, const _Tp& __x)
{
__glibcxx_check_insert(__position);
- return iterator(_Base::insert(__position.base(), __n, __x), this);
+ return { _Base::insert(__position.base(), __n, __x), this };
}
#else
void
@@ -465,7 +465,7 @@ namespace __debug
_Base::insert(__position.base(),
__gnu_debug::__unsafe(__first),
__gnu_debug::__unsafe(__last)),
- this
+ this
};
else
return { _Base::insert(__position.base(), __first, __last), this };
@@ -521,15 +521,17 @@ namespace __debug
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// 151. can't currently clear() empty container
__glibcxx_check_erase_range(__first, __last);
- for (_Base_const_iterator __victim = __first.base();
+ for (__decltype(__first.base()) __victim = __first.base();
__victim != __last.base(); ++__victim)
{
- _GLIBCXX_DEBUG_VERIFY(__victim != _Base::end(),
- _M_message(__gnu_debug::__msg_valid_range)
- ._M_iterator(__first, "position")
- ._M_iterator(__last, "last"));
+ _GLIBCXX_DEBUG_VERIFY(
+ __victim != __first._M_get_sequence()->_M_base().end(),
+ _M_message(__gnu_debug::__msg_valid_range)
+ ._M_iterator(__first, "position")
+ ._M_iterator(__last, "last"));
this->_M_invalidate_if(_Equal(__victim));
}
+
return iterator(_Base::erase(__first.base(), __last.base()), this);
}
@@ -586,7 +588,7 @@ namespace __debug
._M_iterator(__i, "__i"));
_GLIBCXX_DEBUG_VERIFY(__i._M_attached_to(std::__addressof(__x)),
_M_message(__gnu_debug::__msg_splice_other)
- ._M_iterator(__i, "__i")._M_sequence(__x, "__x"));
+ ._M_iterator(__i, "__i")._M_sequence(__x, "__x"));
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// 250. splicing invalidates iterators
@@ -620,19 +622,21 @@ namespace __debug
// We used to perform the splice_alloc check: not anymore, redundant
// after implementing the relevant bits of N1599.
- for (_Base_const_iterator __tmp = __first.base();
+ for (__decltype(__first.base()) __tmp = __first.base();
__tmp != __last.base(); ++__tmp)
{
- _GLIBCXX_DEBUG_VERIFY(__tmp != _Base::end(),
- _M_message(__gnu_debug::__msg_valid_range)
- ._M_iterator(__first, "first")
- ._M_iterator(__last, "last"));
+ _GLIBCXX_DEBUG_VERIFY(
+ __tmp != __first._M_get_sequence()->_M_base().end(),
+ _M_message(__gnu_debug::__msg_valid_range)
+ ._M_iterator(__first, "first")
+ ._M_iterator(__last, "last"));
_GLIBCXX_DEBUG_VERIFY(std::__addressof(__x) != this
|| __tmp != __position.base(),
_M_message(__gnu_debug::__msg_splice_overlap)
._M_iterator(__tmp, "position")
._M_iterator(__first,
