[1/2] libstdc++: Fix return value of vector::insert_range

Message ID 20250131175852.1834036-1-ppalka@redhat.com
State New
Headers
Series [1/2] libstdc++: Fix return value of vector::insert_range |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 fail Patch failed to apply

Commit Message

Patrick Palka Jan. 31, 2025, 5:58 p.m. UTC
  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

Patrick Palka Jan. 31, 2025, 6:04 p.m. UTC | #1
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
> 
>
  
Jonathan Wakely Jan. 31, 2025, 7:23 p.m. UTC | #2
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
>
  

Patch

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) );
 }