Improve std::rotate usages

Improve std::rotate usages

[François Dumont]

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

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


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

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,

Re: Improve std::rotate usages

[François Dumont]

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
>

Re: Improve std::rotate usages

[François Dumont]

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

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
>>
>


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

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,

Re: Improve std::rotate usages

[François Dumont]

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
>>>
>>
>

Re: Improve std::rotate usages

[François Dumont]

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
>>>>
>>>
>>
>

Re: Improve std::rotate usages

[Jonathan Wakely]

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.

Re: Improve std::rotate usages

[François Dumont]

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
>>>>
>>>
>>
>

Re: Improve std::rotate usages

[Jonathan Wakely]

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.