Improve std::rotate usages

Message ID 60f982e6-0fd3-79e6-6b63-f956e051538a@gmail.com
State New
Headers show
Series
  • Improve std::rotate usages
Related show

Commit Message

François Dumont May 2, 2018, 5:26 a.m.
Hi

     std::rotate already returns the expected iterator so there is no 
need for calls to std::advance/std::distance.

Tested under Linux x86_64, ok to commit ?

François

Comments

François Dumont May 14, 2018, 8:14 p.m. | #1
Any feedback regarding this patch ?


On 02/05/2018 07:26, François Dumont wrote:
> Hi

>

>     std::rotate already returns the expected iterator so there is no 

> need for calls to std::advance/std::distance.

>

> Tested under Linux x86_64, ok to commit ?

>

> François

>
François Dumont May 27, 2018, 5:25 p.m. | #2
Still no chance to review it ?

I'd like this one to go in before submitting other algo related patches.

     * include/bits/stl_algo.h
     (__rotate(_Ite, _Ite, _Ite, forward_iterator_tag))
     (__rotate(_Ite, _Ite, _Ite, bidirectional_iterator_tag))
     (__rotate(_Ite, _Ite, _Ite, random_access_iterator_tag)): Move code 
duplication...
     (rotate(_Ite, _Ite, _Ite)): ...here.
     (__stable_partition_adaptive(_FIt, _FIt, _Pred, _Dist, _Pointer, 
_Dist)):
     Simplify rotate call.
     (__rotate_adaptive(_BIt1, _BIt1, _BIt1, _Dist, _Dist, _Bit2, _Dist)):
     Likewise.
     (__merge_without_buffer(_BIt, _BIt, _BIt, _Dist, _Dist, _Comp)):
     Likewise.

François

On 14/05/2018 22:14, François Dumont wrote:
> Any feedback regarding this patch ?

>

>

> On 02/05/2018 07:26, François Dumont wrote:

>> Hi

>>

>>     std::rotate already returns the expected iterator so there is no 

>> need for calls to std::advance/std::distance.

>>

>> Tested under Linux x86_64, ok to commit ?

>>

>> François

>>

>
diff --git a/libstdc++-v3/include/bits/stl_algo.h b/libstdc++-v3/include/bits/stl_algo.h
index e10a692..9c1b2d4 100644
--- a/libstdc++-v3/include/bits/stl_algo.h
+++ b/libstdc++-v3/include/bits/stl_algo.h
@@ -1245,11 +1245,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	     _ForwardIterator __last,
 	     forward_iterator_tag)
     {
-      if (__first == __middle)
-	return __last;
-      else if (__last  == __middle)
-	return __first;
-
       _ForwardIterator __first2 = __middle;
       do
 	{
@@ -1290,11 +1285,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       __glibcxx_function_requires(_Mutable_BidirectionalIteratorConcept<
 				  _BidirectionalIterator>)
 
-      if (__first == __middle)
-	return __last;
-      else if (__last  == __middle)
-	return __first;
-
       std::__reverse(__first,  __middle, bidirectional_iterator_tag());
       std::__reverse(__middle, __last,   bidirectional_iterator_tag());
 
@@ -1328,11 +1318,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       __glibcxx_function_requires(_Mutable_RandomAccessIteratorConcept<
 				  _RandomAccessIterator>)
 
-      if (__first == __middle)
-	return __last;
-      else if (__last  == __middle)
-	return __first;
-
       typedef typename iterator_traits<_RandomAccessIterator>::difference_type
 	_Distance;
       typedef typename iterator_traits<_RandomAccessIterator>::value_type
@@ -1434,6 +1419,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       __glibcxx_requires_valid_range(__first, __middle);
       __glibcxx_requires_valid_range(__middle, __last);
 
+      if (__first == __middle)
+	return __last;
+      else if (__last  == __middle)
+	return __first;
+
       return std::__rotate(__first, __middle, __last,
 			   std::__iterator_category(__first));
     }
@@ -1595,9 +1585,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 					   __right_len,
 					   __buffer, __buffer_size);
 
-      std::rotate(__left_split, __middle, __right_split);
-      std::advance(__left_split, std::distance(__middle, __right_split));
-      return __left_split;
+      return std::rotate(__left_split, __middle, __right_split);
     }
 
   template<typename _ForwardIterator, typename _Predicate>
@@ -2396,11 +2384,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    return __last;
 	}
       else
-	{
-	  std::rotate(__first, __middle, __last);
-	  std::advance(__first, std::distance(__middle, __last));
-	  return __first;
-	}
+	return std::rotate(__first, __middle, __last);
     }
 
   /// This is a helper function for the merge routines.
@@ -2507,9 +2491,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  __len11 = std::distance(__first, __first_cut);
 	}
 
-      std::rotate(__first_cut, __middle, __second_cut);
-      _BidirectionalIterator __new_middle = __first_cut;
-      std::advance(__new_middle, std::distance(__middle, __second_cut));
+      _BidirectionalIterator __new_middle
+	= std::rotate(__first_cut, __middle, __second_cut);
       std::__merge_without_buffer(__first, __first_cut, __new_middle,
 				  __len11, __len22, __comp);
       std::__merge_without_buffer(__new_middle, __second_cut, __last,
François Dumont June 8, 2018, 5:54 a.m. | #3
Gentle reminder.

On 27/05/2018 19:25, François Dumont wrote:
> Still no chance to review it ?

>

> I'd like this one to go in before submitting other algo related patches.

>

>     * include/bits/stl_algo.h

>     (__rotate(_Ite, _Ite, _Ite, forward_iterator_tag))

>     (__rotate(_Ite, _Ite, _Ite, bidirectional_iterator_tag))

>     (__rotate(_Ite, _Ite, _Ite, random_access_iterator_tag)): Move 

> code duplication...

>     (rotate(_Ite, _Ite, _Ite)): ...here.

>     (__stable_partition_adaptive(_FIt, _FIt, _Pred, _Dist, _Pointer, 

> _Dist)):

>     Simplify rotate call.

>     (__rotate_adaptive(_BIt1, _BIt1, _BIt1, _Dist, _Dist, _Bit2, _Dist)):

>     Likewise.

>     (__merge_without_buffer(_BIt, _BIt, _BIt, _Dist, _Dist, _Comp)):

>     Likewise.

>

> François

>

> On 14/05/2018 22:14, François Dumont wrote:

>> Any feedback regarding this patch ?

>>

>>

>> On 02/05/2018 07:26, François Dumont wrote:

>>> Hi

>>>

>>>     std::rotate already returns the expected iterator so there is no 

>>> need for calls to std::advance/std::distance.

>>>

>>> Tested under Linux x86_64, ok to commit ?

>>>

>>> François

>>>

>>

>
François Dumont June 12, 2018, 8:39 p.m. | #4
I'm struggling having a validation for this patch.

As it is a trivial one and to not bother you I plan to commit it 
tomorrow if not told otherwise.

François



On 08/06/2018 07:54, François Dumont wrote:
> Gentle reminder.

>

> On 27/05/2018 19:25, François Dumont wrote:

>> Still no chance to review it ?

>>

>> I'd like this one to go in before submitting other algo related patches.

>>

>>     * include/bits/stl_algo.h

>>     (__rotate(_Ite, _Ite, _Ite, forward_iterator_tag))

>>     (__rotate(_Ite, _Ite, _Ite, bidirectional_iterator_tag))

>>     (__rotate(_Ite, _Ite, _Ite, random_access_iterator_tag)): Move 

>> code duplication...

>>     (rotate(_Ite, _Ite, _Ite)): ...here.

>>     (__stable_partition_adaptive(_FIt, _FIt, _Pred, _Dist, _Pointer, 

>> _Dist)):

>>     Simplify rotate call.

>>     (__rotate_adaptive(_BIt1, _BIt1, _BIt1, _Dist, _Dist, _Bit2, 

>> _Dist)):

>>     Likewise.

>>     (__merge_without_buffer(_BIt, _BIt, _BIt, _Dist, _Dist, _Comp)):

>>     Likewise.

>>

>> François

>>

>> On 14/05/2018 22:14, François Dumont wrote:

>>> Any feedback regarding this patch ?

>>>

>>>

>>> On 02/05/2018 07:26, François Dumont wrote:

>>>> Hi

>>>>

>>>>     std::rotate already returns the expected iterator so there is 

>>>> no need for calls to std::advance/std::distance.

>>>>

>>>> Tested under Linux x86_64, ok to commit ?

>>>>

>>>> François

>>>>

>>>

>>

>
Jonathan Wakely June 12, 2018, 11:13 p.m. | #5
On 12/06/18 22:39 +0200, François Dumont wrote:
>I'm struggling having a validation for this patch.

>

>As it is a trivial one and to not bother you I plan to commit it 

>tomorrow if not told otherwise.


No, please wait for review.
François Dumont July 24, 2018, 10:23 a.m. | #6
Ping.

On 08/06/2018 07:54, François Dumont wrote:
> Gentle reminder.

>

> On 27/05/2018 19:25, François Dumont wrote:

>> Still no chance to review it ?

>>

>> I'd like this one to go in before submitting other algo related patches.

>>

>>     * include/bits/stl_algo.h

>>     (__rotate(_Ite, _Ite, _Ite, forward_iterator_tag))

>>     (__rotate(_Ite, _Ite, _Ite, bidirectional_iterator_tag))

>>     (__rotate(_Ite, _Ite, _Ite, random_access_iterator_tag)): Move 

>> code duplication...

>>     (rotate(_Ite, _Ite, _Ite)): ...here.

>>     (__stable_partition_adaptive(_FIt, _FIt, _Pred, _Dist, _Pointer, 

>> _Dist)):

>>     Simplify rotate call.

>>     (__rotate_adaptive(_BIt1, _BIt1, _BIt1, _Dist, _Dist, _Bit2, 

>> _Dist)):

>>     Likewise.

>>     (__merge_without_buffer(_BIt, _BIt, _BIt, _Dist, _Dist, _Comp)):

>>     Likewise.

>>

>> François

>>

>> On 14/05/2018 22:14, François Dumont wrote:

>>> Any feedback regarding this patch ?

>>>

>>>

>>> On 02/05/2018 07:26, François Dumont wrote:

>>>> Hi

>>>>

>>>>     std::rotate already returns the expected iterator so there is 

>>>> no need for calls to std::advance/std::distance.

>>>>

>>>> Tested under Linux x86_64, ok to commit ?

>>>>

>>>> François

>>>>

>>>

>>

>
Jonathan Wakely Aug. 7, 2018, 2:12 p.m. | #7
On 27/05/18 19:25 +0200, François Dumont wrote:
>Still no chance to review it ?

>

>I'd like this one to go in before submitting other algo related patches.

>

>    * include/bits/stl_algo.h

>    (__rotate(_Ite, _Ite, _Ite, forward_iterator_tag))

>    (__rotate(_Ite, _Ite, _Ite, bidirectional_iterator_tag))

>    (__rotate(_Ite, _Ite, _Ite, random_access_iterator_tag)): Move 

>code duplication...

>    (rotate(_Ite, _Ite, _Ite)): ...here.

>    (__stable_partition_adaptive(_FIt, _FIt, _Pred, _Dist, _Pointer, 

>_Dist)):

>    Simplify rotate call.

>    (__rotate_adaptive(_BIt1, _BIt1, _BIt1, _Dist, _Dist, _Bit2, _Dist)):

>    Likewise.

>    (__merge_without_buffer(_BIt, _BIt, _BIt, _Dist, _Dist, _Comp)):

>    Likewise.

>

>François

>

>On 14/05/2018 22:14, François Dumont wrote:

>>Any feedback regarding this patch ?

>>

>>

>>On 02/05/2018 07:26, François Dumont wrote:

>>>Hi

>>>

>>>    std::rotate already returns the expected iterator so there is 

>>>no need for calls to std::advance/std::distance.


Yes, looks like that code predated DR 488 which changed the return
type of std::rotate.

OK for trunk, thanks.

Patch

diff --git a/libstdc++-v3/include/bits/stl_algo.h b/libstdc++-v3/include/bits/stl_algo.h
index e10a692..9c1b2d4 100644
--- a/libstdc++-v3/include/bits/stl_algo.h
+++ b/libstdc++-v3/include/bits/stl_algo.h
@@ -1245,11 +1245,6 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	     _ForwardIterator __last,
 	     forward_iterator_tag)
     {
-      if (__first == __middle)
-	return __last;
-      else if (__last  == __middle)
-	return __first;
-
       _ForwardIterator __first2 = __middle;
       do
 	{
@@ -1290,11 +1285,6 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       __glibcxx_function_requires(_Mutable_BidirectionalIteratorConcept<
 				  _BidirectionalIterator>)
 
-      if (__first == __middle)
-	return __last;
-      else if (__last  == __middle)
-	return __first;
-
       std::__reverse(__first,  __middle, bidirectional_iterator_tag());
       std::__reverse(__middle, __last,   bidirectional_iterator_tag());
 
@@ -1328,11 +1318,6 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       __glibcxx_function_requires(_Mutable_RandomAccessIteratorConcept<
 				  _RandomAccessIterator>)
 
-      if (__first == __middle)
-	return __last;
-      else if (__last  == __middle)
-	return __first;
-
       typedef typename iterator_traits<_RandomAccessIterator>::difference_type
 	_Distance;
       typedef typename iterator_traits<_RandomAccessIterator>::value_type
@@ -1434,6 +1419,11 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
       __glibcxx_requires_valid_range(__first, __middle);
       __glibcxx_requires_valid_range(__middle, __last);
 
+      if (__first == __middle)
+	return __last;
+      else if (__last  == __middle)
+	return __first;
+
       return std::__rotate(__first, __middle, __last,
 			   std::__iterator_category(__first));
     }
@@ -1595,9 +1585,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 					   __right_len,
 					   __buffer, __buffer_size);
 
-      std::rotate(__left_split, __middle, __right_split);
-      std::advance(__left_split, std::distance(__middle, __right_split));
-      return __left_split;
+      return std::rotate(__left_split, __middle, __right_split);
     }
 
   template<typename _ForwardIterator, typename _Predicate>
@@ -2396,11 +2384,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    return __last;
 	}
       else
-	{
-	  std::rotate(__first, __middle, __last);
-	  std::advance(__first, std::distance(__middle, __last));
-	  return __first;
-	}
+	return std::rotate(__first, __middle, __last);
     }
 
   /// This is a helper function for the merge routines.
@@ -2507,9 +2491,8 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  __len11 = std::distance(__first, __first_cut);
 	}
 
-      std::rotate(__first_cut, __middle, __second_cut);
-      _BidirectionalIterator __new_middle = __first_cut;
-      std::advance(__new_middle, std::distance(__middle, __second_cut));
+      _BidirectionalIterator __new_middle
+	= std::rotate(__first_cut, __middle, __second_cut);
       std::__merge_without_buffer(__first, __first_cut, __new_middle,
 				  __len11, __len22, __comp);
       std::__merge_without_buffer(__new_middle, __second_cut, __last,