[1/2] libstdc++: Fix return value of vector::insert_range
Checks
Commit Message
In some cases we're wrongly returning an iterator pointing to (one past)
the last element inserted instead of to the first element inserted.
libstdc++-v3/ChangeLog:
* include/bits/stl_bvector.h (vector<bool>::insert_range):
Consistently return an iterator pointing to the first element
inserted.
* include/bits/vector.tcc (vector::insert_range): Likewise.
* testsuite/23_containers/vector/bool/modifiers/insert/insert_range.cc:
Verify insert_range return values.
* testsuite/23_containers/vector/modifiers/insert/insert_range.cc:
Likewise.
---
libstdc++-v3/include/bits/stl_bvector.h | 8 ++++----
libstdc++-v3/include/bits/vector.tcc | 3 ++-
.../bool/modifiers/insert/insert_range.cc | 18 ++++++++++++------
.../vector/modifiers/insert/insert_range.cc | 18 ++++++++++++------
4 files changed, 30 insertions(+), 17 deletions(-)
Comments
On Fri, 31 Jan 2025, Patrick Palka wrote:
> In some cases we're wrongly returning an iterator pointing to (one past)
> the last element inserted instead of to the first element inserted.
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/stl_bvector.h (vector<bool>::insert_range):
> Consistently return an iterator pointing to the first element
> inserted.
> * include/bits/vector.tcc (vector::insert_range): Likewise.
> * testsuite/23_containers/vector/bool/modifiers/insert/insert_range.cc:
> Verify insert_range return values.
> * testsuite/23_containers/vector/modifiers/insert/insert_range.cc:
> Likewise.
> ---
> libstdc++-v3/include/bits/stl_bvector.h | 8 ++++----
> libstdc++-v3/include/bits/vector.tcc | 3 ++-
> .../bool/modifiers/insert/insert_range.cc | 18 ++++++++++++------
> .../vector/modifiers/insert/insert_range.cc | 18 ++++++++++++------
> 4 files changed, 30 insertions(+), 17 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/stl_bvector.h b/libstdc++-v3/include/bits/stl_bvector.h
> index 961e4a25299..e5e4b7db5a9 100644
> --- a/libstdc++-v3/include/bits/stl_bvector.h
> +++ b/libstdc++-v3/include/bits/stl_bvector.h
> @@ -1341,9 +1341,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> std::copy_backward(__pos._M_const_cast(), end(),
> this->_M_impl._M_finish
> + difference_type(__n));
> - auto __i = ranges::copy(__rg, __pos._M_const_cast()).out;
> + ranges::copy(__rg, __pos._M_const_cast()).out;
Oops, consider this stray '.out' removed.
> this->_M_impl._M_finish += difference_type(__n);
> - return __i;
> + return __pos._M_const_cast();
> }
> else
> {
> @@ -1355,9 +1355,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> iterator __i = _M_copy_aligned(__begin,
> __pos._M_const_cast(),
> __start);
> - __i = ranges::copy(__rg, __i).out;
> + iterator __j = ranges::copy(__rg, __i).out;
> iterator __finish = std::copy(__pos._M_const_cast(),
> - __end, __i);
> + __end, __j);
> this->_M_deallocate();
> this->_M_impl._M_end_of_storage = __q + _S_nword(__len);
> this->_M_impl._M_start = __start;
> diff --git a/libstdc++-v3/include/bits/vector.tcc b/libstdc++-v3/include/bits/vector.tcc
> index 4f4c366080b..acb2f5fca1e 100644
> --- a/libstdc++-v3/include/bits/vector.tcc
> +++ b/libstdc++-v3/include/bits/vector.tcc
> @@ -984,8 +984,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> {
> if (__pos == cend())
> {
> + const auto __ins_idx = size();
> append_range(std::forward<_Rg>(__rg));
> - return end();
> + return begin() + __ins_idx;
> }
>
> if constexpr (ranges::forward_range<_Rg>)
> diff --git a/libstdc++-v3/testsuite/23_containers/vector/bool/modifiers/insert/insert_range.cc b/libstdc++-v3/testsuite/23_containers/vector/bool/modifiers/insert/insert_range.cc
> index 4f4835746ea..5c65610667d 100644
> --- a/libstdc++-v3/testsuite/23_containers/vector/bool/modifiers/insert/insert_range.cc
> +++ b/libstdc++-v3/testsuite/23_containers/vector/bool/modifiers/insert/insert_range.cc
> @@ -35,16 +35,22 @@ do_test()
> VERIFY( eq(v, a) );
> v.clear();
> v.shrink_to_fit();
> - v.insert_range(v.begin(), Range(a, a+3));
> - v.insert_range(v.end(), Range(a+6, a+9));
> - v.insert_range(v.begin()+3, Range(a+3, a+6));
> + auto it = v.insert_range(v.begin(), Range(a, a+3));
> + VERIFY( it == v.begin() );
> + it = v.insert_range(v.end(), Range(a+6, a+9));
> + VERIFY( it == v.begin()+3 );
> + it = v.insert_range(v.begin()+3, Range(a+3, a+6));
> + VERIFY( it == v.begin()+3 );
> VERIFY( eq(v, a) );
> v.resize(3);
> - v.insert_range(v.begin()+1, Range(a+4, a+9));
> - v.insert_range(v.begin()+1, Range(a+1, a+4));
> + it = v.insert_range(v.begin()+1, Range(a+4, a+9));
> + VERIFY( it == v.begin()+1 );
> + it = v.insert_range(v.begin()+1, Range(a+1, a+4));
> + VERIFY( it == v.begin()+1 );
> v.resize(9);
> VERIFY( eq(v, a) );
> - v.insert_range(v.begin(), Range(a, a));
> + it = v.insert_range(v.begin(), Range(a, a));
> + VERIFY( it == v.begin() );
> VERIFY( eq(v, a) );
> }
>
> diff --git a/libstdc++-v3/testsuite/23_containers/vector/modifiers/insert/insert_range.cc b/libstdc++-v3/testsuite/23_containers/vector/modifiers/insert/insert_range.cc
> index 68218e94f28..59071435126 100644
> --- a/libstdc++-v3/testsuite/23_containers/vector/modifiers/insert/insert_range.cc
> +++ b/libstdc++-v3/testsuite/23_containers/vector/modifiers/insert/insert_range.cc
> @@ -39,16 +39,22 @@ do_test()
> VERIFY( eq(v, a) );
> v.clear();
> v.shrink_to_fit();
> - v.insert_range(v.begin(), Range(a, a+3));
> - v.insert_range(v.end(), Range(a+6, a+9));
> - v.insert_range(v.begin()+3, Range(a+3, a+6));
> + auto it = v.insert_range(v.begin(), Range(a, a+3));
> + VERIFY( it == v.begin() );
> + it = v.insert_range(v.end(), Range(a+6, a+9));
> + VERIFY( it == v.begin()+3 );
> + it = v.insert_range(v.begin()+3, Range(a+3, a+6));
> + VERIFY( it == v.begin()+3 );
> VERIFY( eq(v, a) );
> v.resize(3);
> - v.insert_range(v.begin()+1, Range(a+4, a+9));
> - v.insert_range(v.begin()+1, Range(a+1, a+4));
> + it = v.insert_range(v.begin()+1, Range(a+4, a+9));
> + VERIFY( it == v.begin()+1 );
> + it = v.insert_range(v.begin()+1, Range(a+1, a+4));
> + VERIFY( it == v.begin()+1 );
> v.resize(9);
> VERIFY( eq(v, a) );
> - v.insert_range(v.begin() + 6, Range(a, a));
> + it = v.insert_range(v.begin() + 6, Range(a, a));
> + VERIFY( it == v.begin() + 6 );
> VERIFY( eq(v, a) );
> }
>
> --
> 2.48.1.157.g3b0d05c4a7
>
>
On Fri, 31 Jan 2025 at 18:37, Patrick Palka wrote:
>
> In some cases we're wrongly returning an iterator pointing to (one past)
> the last element inserted instead of to the first element inserted.
Oops! Did I get it right in the linked lists?
OK for trunk (with the .out removed).
>
> libstdc++-v3/ChangeLog:
>
> * include/bits/stl_bvector.h (vector<bool>::insert_range):
> Consistently return an iterator pointing to the first element
> inserted.
> * include/bits/vector.tcc (vector::insert_range): Likewise.
> * testsuite/23_containers/vector/bool/modifiers/insert/insert_range.cc:
> Verify insert_range return values.
> * testsuite/23_containers/vector/modifiers/insert/insert_range.cc:
> Likewise.
> ---
> libstdc++-v3/include/bits/stl_bvector.h | 8 ++++----
> libstdc++-v3/include/bits/vector.tcc | 3 ++-
> .../bool/modifiers/insert/insert_range.cc | 18 ++++++++++++------
> .../vector/modifiers/insert/insert_range.cc | 18 ++++++++++++------
> 4 files changed, 30 insertions(+), 17 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/stl_bvector.h b/libstdc++-v3/include/bits/stl_bvector.h
> index 961e4a25299..e5e4b7db5a9 100644
> --- a/libstdc++-v3/include/bits/stl_bvector.h
> +++ b/libstdc++-v3/include/bits/stl_bvector.h
> @@ -1341,9 +1341,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> std::copy_backward(__pos._M_const_cast(), end(),
> this->_M_impl._M_finish
> + difference_type(__n));
> - auto __i = ranges::copy(__rg, __pos._M_const_cast()).out;
> + ranges::copy(__rg, __pos._M_const_cast()).out;
> this->_M_impl._M_finish += difference_type(__n);
> - return __i;
> + return __pos._M_const_cast();
> }
> else
> {
> @@ -1355,9 +1355,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> iterator __i = _M_copy_aligned(__begin,
> __pos._M_const_cast(),
> __start);
> - __i = ranges::copy(__rg, __i).out;
> + iterator __j = ranges::copy(__rg, __i).out;
> iterator __finish = std::copy(__pos._M_const_cast(),
> - __end, __i);
> + __end, __j);
> this->_M_deallocate();
> this->_M_impl._M_end_of_storage = __q + _S_nword(__len);
> this->_M_impl._M_start = __start;
> diff --git a/libstdc++-v3/include/bits/vector.tcc b/libstdc++-v3/include/bits/vector.tcc
> index 4f4c366080b..acb2f5fca1e 100644
> --- a/libstdc++-v3/include/bits/vector.tcc
> +++ b/libstdc++-v3/include/bits/vector.tcc
> @@ -984,8 +984,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
> {
> if (__pos == cend())
> {
> + const auto __ins_idx = size();
> append_range(std::forward<_Rg>(__rg));
> - return end();
> + return begin() + __ins_idx;
> }
>
> if constexpr (ranges::forward_range<_Rg>)
> diff --git a/libstdc++-v3/testsuite/23_containers/vector/bool/modifiers/insert/insert_range.cc b/libstdc++-v3/testsuite/23_containers/vector/bool/modifiers/insert/insert_range.cc
> index 4f4835746ea..5c65610667d 100644
> --- a/libstdc++-v3/testsuite/23_containers/vector/bool/modifiers/insert/insert_range.cc
> +++ b/libstdc++-v3/testsuite/23_containers/vector/bool/modifiers/insert/insert_range.cc
> @@ -35,16 +35,22 @@ do_test()
> VERIFY( eq(v, a) );
> v.clear();
> v.shrink_to_fit();
> - v.insert_range(v.begin(), Range(a, a+3));
> - v.insert_range(v.end(), Range(a+6, a+9));
> - v.insert_range(v.begin()+3, Range(a+3, a+6));
> + auto it = v.insert_range(v.begin(), Range(a, a+3));
> + VERIFY( it == v.begin() );
> + it = v.insert_range(v.end(), Range(a+6, a+9));
> + VERIFY( it == v.begin()+3 );
> + it = v.insert_range(v.begin()+3, Range(a+3, a+6));
> + VERIFY( it == v.begin()+3 );
> VERIFY( eq(v, a) );
> v.resize(3);
> - v.insert_range(v.begin()+1, Range(a+4, a+9));
> - v.insert_range(v.begin()+1, Range(a+1, a+4));
> + it = v.insert_range(v.begin()+1, Range(a+4, a+9));
> + VERIFY( it == v.begin()+1 );
> + it = v.insert_range(v.begin()+1, Range(a+1, a+4));
> + VERIFY( it == v.begin()+1 );
> v.resize(9);
> VERIFY( eq(v, a) );
> - v.insert_range(v.begin(), Range(a, a));
> + it = v.insert_range(v.begin(), Range(a, a));
> + VERIFY( it == v.begin() );
> VERIFY( eq(v, a) );
> }
>
> diff --git a/libstdc++-v3/testsuite/23_containers/vector/modifiers/insert/insert_range.cc b/libstdc++-v3/testsuite/23_containers/vector/modifiers/insert/insert_range.cc
> index 68218e94f28..59071435126 100644
> --- a/libstdc++-v3/testsuite/23_containers/vector/modifiers/insert/insert_range.cc
> +++ b/libstdc++-v3/testsuite/23_containers/vector/modifiers/insert/insert_range.cc
> @@ -39,16 +39,22 @@ do_test()
> VERIFY( eq(v, a) );
> v.clear();
> v.shrink_to_fit();
> - v.insert_range(v.begin(), Range(a, a+3));
> - v.insert_range(v.end(), Range(a+6, a+9));
> - v.insert_range(v.begin()+3, Range(a+3, a+6));
> + auto it = v.insert_range(v.begin(), Range(a, a+3));
> + VERIFY( it == v.begin() );
> + it = v.insert_range(v.end(), Range(a+6, a+9));
> + VERIFY( it == v.begin()+3 );
> + it = v.insert_range(v.begin()+3, Range(a+3, a+6));
> + VERIFY( it == v.begin()+3 );
> VERIFY( eq(v, a) );
> v.resize(3);
> - v.insert_range(v.begin()+1, Range(a+4, a+9));
> - v.insert_range(v.begin()+1, Range(a+1, a+4));
> + it = v.insert_range(v.begin()+1, Range(a+4, a+9));
> + VERIFY( it == v.begin()+1 );
> + it = v.insert_range(v.begin()+1, Range(a+1, a+4));
> + VERIFY( it == v.begin()+1 );
> v.resize(9);
> VERIFY( eq(v, a) );
> - v.insert_range(v.begin() + 6, Range(a, a));
> + it = v.insert_range(v.begin() + 6, Range(a, a));
> + VERIFY( it == v.begin() + 6 );
> VERIFY( eq(v, a) );
> }
>
> --
> 2.48.1.157.g3b0d05c4a7
>
@@ -1341,9 +1341,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
std::copy_backward(__pos._M_const_cast(), end(),
this->_M_impl._M_finish
+ difference_type(__n));
- auto __i = ranges::copy(__rg, __pos._M_const_cast()).out;
+ ranges::copy(__rg, __pos._M_const_cast()).out;
this->_M_impl._M_finish += difference_type(__n);
- return __i;
+ return __pos._M_const_cast();
}
else
{
@@ -1355,9 +1355,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
iterator __i = _M_copy_aligned(__begin,
__pos._M_const_cast(),
__start);
- __i = ranges::copy(__rg, __i).out;
+ iterator __j = ranges::copy(__rg, __i).out;
iterator __finish = std::copy(__pos._M_const_cast(),
- __end, __i);
+ __end, __j);
this->_M_deallocate();
this->_M_impl._M_end_of_storage = __q + _S_nword(__len);
this->_M_impl._M_start = __start;
@@ -984,8 +984,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{
if (__pos == cend())
{
+ const auto __ins_idx = size();
append_range(std::forward<_Rg>(__rg));
- return end();
+ return begin() + __ins_idx;
}
if constexpr (ranges::forward_range<_Rg>)
@@ -35,16 +35,22 @@ do_test()
VERIFY( eq(v, a) );
v.clear();
v.shrink_to_fit();
- v.insert_range(v.begin(), Range(a, a+3));
- v.insert_range(v.end(), Range(a+6, a+9));
- v.insert_range(v.begin()+3, Range(a+3, a+6));
+ auto it = v.insert_range(v.begin(), Range(a, a+3));
+ VERIFY( it == v.begin() );
+ it = v.insert_range(v.end(), Range(a+6, a+9));
+ VERIFY( it == v.begin()+3 );
+ it = v.insert_range(v.begin()+3, Range(a+3, a+6));
+ VERIFY( it == v.begin()+3 );
VERIFY( eq(v, a) );
v.resize(3);
- v.insert_range(v.begin()+1, Range(a+4, a+9));
- v.insert_range(v.begin()+1, Range(a+1, a+4));
+ it = v.insert_range(v.begin()+1, Range(a+4, a+9));
+ VERIFY( it == v.begin()+1 );
+ it = v.insert_range(v.begin()+1, Range(a+1, a+4));
+ VERIFY( it == v.begin()+1 );
v.resize(9);
VERIFY( eq(v, a) );
- v.insert_range(v.begin(), Range(a, a));
+ it = v.insert_range(v.begin(), Range(a, a));
+ VERIFY( it == v.begin() );
VERIFY( eq(v, a) );
}
@@ -39,16 +39,22 @@ do_test()
VERIFY( eq(v, a) );
v.clear();
v.shrink_to_fit();
- v.insert_range(v.begin(), Range(a, a+3));
- v.insert_range(v.end(), Range(a+6, a+9));
- v.insert_range(v.begin()+3, Range(a+3, a+6));
+ auto it = v.insert_range(v.begin(), Range(a, a+3));
+ VERIFY( it == v.begin() );
+ it = v.insert_range(v.end(), Range(a+6, a+9));
+ VERIFY( it == v.begin()+3 );
+ it = v.insert_range(v.begin()+3, Range(a+3, a+6));
+ VERIFY( it == v.begin()+3 );
VERIFY( eq(v, a) );
v.resize(3);
- v.insert_range(v.begin()+1, Range(a+4, a+9));
- v.insert_range(v.begin()+1, Range(a+1, a+4));
+ it = v.insert_range(v.begin()+1, Range(a+4, a+9));
+ VERIFY( it == v.begin()+1 );
+ it = v.insert_range(v.begin()+1, Range(a+1, a+4));
+ VERIFY( it == v.begin()+1 );
v.resize(9);
VERIFY( eq(v, a) );
- v.insert_range(v.begin() + 6, Range(a, a));
+ it = v.insert_range(v.begin() + 6, Range(a, a));
+ VERIFY( it == v.begin() + 6 );
VERIFY( eq(v, a) );
}