Make std::vector<bool> iterator operators friend inline

Message ID 4a88818f-c1ae-0565-9eb6-eac701c3c70c@gmail.com
State Superseded
Headers show
Series
  • Make std::vector<bool> iterator operators friend inline
Related show

Commit Message

François Dumont Oct. 12, 2018, 4:25 p.m.
Here is the patch for _Bit_iterator and _Bit_const_iterator operators.

I noticed that _Bit_reference == and < operators could be made inline 
friend too. Do you want me to include this change in the patch ?


     * include/bits/stl_bvector.h (_Bit_iterator_base::operator==): Replace
     member method with inline friend.
     (_Bit_iterator_base::operator<): Likewise.
     (_Bit_iterator_base::operator!=): Likewise.
     (_Bit_iterator_base::operator>): Likewise.
     (_Bit_iterator_base::operator<=): Likewise.
     (_Bit_iterator_base::operator>=): Likewise.
     (operator-(const _Bit_iterator_base&, const _Bit_iterator_base&)): Make
     inline friend.
     (_Bit_iterator::operator+(difference_type)): Replace member method with
     inline friend.
     (_Bit_iterator::operator-(difference_type)): Likewise.
     (operator+(ptrdiff_t, const _Bit_iterator&)): Make inline friend.
     (_Bit_const_iterator::operator+(difference_type)): Replace member 
method
     with inline friend.
     (_Bit_const_iterator::operator-(difference_type)): Likewise.
     (operator+(ptrdiff_t, const _Bit_const_iterator&)): Make inline
     friend.

Tested under Linux x86_64.

Ok to commit ?

François

Comments

Jonathan Wakely Oct. 15, 2018, 9:36 a.m. | #1
On 12/10/18 18:25 +0200, François Dumont wrote:
>Here is the patch for _Bit_iterator and _Bit_const_iterator operators.

>

>I noticed that _Bit_reference == and < operators could be made inline 

>friend too. Do you want me to include this change in the patch ?

>

>

>    * include/bits/stl_bvector.h (_Bit_iterator_base::operator==): Replace

>    member method with inline friend.

>    (_Bit_iterator_base::operator<): Likewise.

>    (_Bit_iterator_base::operator!=): Likewise.

>    (_Bit_iterator_base::operator>): Likewise.

>    (_Bit_iterator_base::operator<=): Likewise.

>    (_Bit_iterator_base::operator>=): Likewise.

>    (operator-(const _Bit_iterator_base&, const _Bit_iterator_base&)): Make

>    inline friend.

>    (_Bit_iterator::operator+(difference_type)): Replace member method with

>    inline friend.

>    (_Bit_iterator::operator-(difference_type)): Likewise.

>    (operator+(ptrdiff_t, const _Bit_iterator&)): Make inline friend.

>    (_Bit_const_iterator::operator+(difference_type)): Replace member 

>method

>    with inline friend.

>    (_Bit_const_iterator::operator-(difference_type)): Likewise.

>    (operator+(ptrdiff_t, const _Bit_const_iterator&)): Make inline

>    friend.

>

>Tested under Linux x86_64.

>

>Ok to commit ?

>

>François

>


>diff --git a/libstdc++-v3/include/bits/stl_bvector.h b/libstdc++-v3/include/bits/stl_bvector.h

>index 19c16839cfa..8fbef7a1a3a 100644

>--- a/libstdc++-v3/include/bits/stl_bvector.h

>+++ b/libstdc++-v3/include/bits/stl_bvector.h

>@@ -182,40 +182,40 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER

>       _M_offset = static_cast<unsigned int>(__n);

>     }

> 

>-    bool

>-    operator==(const _Bit_iterator_base& __i) const

>-    { return _M_p == __i._M_p && _M_offset == __i._M_offset; }

>+    friend bool

>+    operator==(const _Bit_iterator_base& __x, const _Bit_iterator_base& __y)

>+    { return __x._M_p == __y._M_p && __x._M_offset == __y._M_offset; }

> 

>-    bool

>-    operator<(const _Bit_iterator_base& __i) const

>+    friend bool

>+    operator<(const _Bit_iterator_base& __x, const _Bit_iterator_base& __y)

>     {

>-      return _M_p < __i._M_p

>-	    || (_M_p == __i._M_p && _M_offset < __i._M_offset);

>+      return __x._M_p < __y._M_p

>+	    || (__x._M_p == __y._M_p && __x._M_offset < __y._M_offset);

>     }

> 

>-    bool

>-    operator!=(const _Bit_iterator_base& __i) const

>-    { return !(*this == __i); }

>+    friend bool

>+    operator!=(const _Bit_iterator_base& __x, const _Bit_iterator_base& __y)

>+    { return !(__x == __y); }

> 

>-    bool

>-    operator>(const _Bit_iterator_base& __i) const

>-    { return __i < *this; }

>+    friend bool

>+    operator>(const _Bit_iterator_base& __x, const _Bit_iterator_base& __y)

>+    { return __y < __x; }

> 

>-    bool

>-    operator<=(const _Bit_iterator_base& __i) const

>-    { return !(__i < *this); }

>+    friend bool

>+    operator<=(const _Bit_iterator_base& __x, const _Bit_iterator_base& __y)

>+    { return !(__y < __x); }

> 

>-    bool

>-    operator>=(const _Bit_iterator_base& __i) const

>-    { return !(*this < __i); }

>-  };

>+    friend bool

>+    operator>=(const _Bit_iterator_base& __x, const _Bit_iterator_base& __y)

>+    { return !(__x < __y); }

> 

>-  inline ptrdiff_t

>-  operator-(const _Bit_iterator_base& __x, const _Bit_iterator_base& __y)

>-  {

>-    return (int(_S_word_bit) * (__x._M_p - __y._M_p)

>-	    + __x._M_offset - __y._M_offset);

>-  }


For the non-member operator- and operator+ this change makes sense,
but what is the benefit of changing all the others? As members they're
already not considered as candidates for unrelated types, or am I
missing something?
François Dumont Oct. 15, 2018, 8:39 p.m. | #2
On 10/15/2018 11:36 AM, Jonathan Wakely wrote:
> On 12/10/18 18:25 +0200, François Dumont wrote:

>> Here is the patch for _Bit_iterator and _Bit_const_iterator operators.

>>

>> I noticed that _Bit_reference == and < operators could be made inline 

>> friend too. Do you want me to include this change in the patch ?

>>

>>

>>     * include/bits/stl_bvector.h (_Bit_iterator_base::operator==): 

>> Replace

>>     member method with inline friend.

>>     (_Bit_iterator_base::operator<): Likewise.

>>     (_Bit_iterator_base::operator!=): Likewise.

>>     (_Bit_iterator_base::operator>): Likewise.

>>     (_Bit_iterator_base::operator<=): Likewise.

>>     (_Bit_iterator_base::operator>=): Likewise.

>>     (operator-(const _Bit_iterator_base&, const 

>> _Bit_iterator_base&)): Make

>>     inline friend.

>>     (_Bit_iterator::operator+(difference_type)): Replace member 

>> method with

>>     inline friend.

>>     (_Bit_iterator::operator-(difference_type)): Likewise.

>>     (operator+(ptrdiff_t, const _Bit_iterator&)): Make inline friend.

>>     (_Bit_const_iterator::operator+(difference_type)): Replace member 

>> method

>>     with inline friend.

>>     (_Bit_const_iterator::operator-(difference_type)): Likewise.

>>     (operator+(ptrdiff_t, const _Bit_const_iterator&)): Make inline

>>     friend.

>>

>> Tested under Linux x86_64.

>>

>> Ok to commit ?

>>

>> François

>>

>

>> diff --git a/libstdc++-v3/include/bits/stl_bvector.h 

>> b/libstdc++-v3/include/bits/stl_bvector.h

>> index 19c16839cfa..8fbef7a1a3a 100644

>> --- a/libstdc++-v3/include/bits/stl_bvector.h

>> +++ b/libstdc++-v3/include/bits/stl_bvector.h

>> @@ -182,40 +182,40 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER

>>       _M_offset = static_cast<unsigned int>(__n);

>>     }

>>

>> -    bool

>> -    operator==(const _Bit_iterator_base& __i) const

>> -    { return _M_p == __i._M_p && _M_offset == __i._M_offset; }

>> +    friend bool

>> +    operator==(const _Bit_iterator_base& __x, const 

>> _Bit_iterator_base& __y)

>> +    { return __x._M_p == __y._M_p && __x._M_offset == __y._M_offset; }

>>

>> -    bool

>> -    operator<(const _Bit_iterator_base& __i) const

>> +    friend bool

>> +    operator<(const _Bit_iterator_base& __x, const 

>> _Bit_iterator_base& __y)

>>     {

>> -      return _M_p < __i._M_p

>> -        || (_M_p == __i._M_p && _M_offset < __i._M_offset);

>> +      return __x._M_p < __y._M_p

>> +        || (__x._M_p == __y._M_p && __x._M_offset < __y._M_offset);

>>     }

>>

>> -    bool

>> -    operator!=(const _Bit_iterator_base& __i) const

>> -    { return !(*this == __i); }

>> +    friend bool

>> +    operator!=(const _Bit_iterator_base& __x, const 

>> _Bit_iterator_base& __y)

>> +    { return !(__x == __y); }

>>

>> -    bool

>> -    operator>(const _Bit_iterator_base& __i) const

>> -    { return __i < *this; }

>> +    friend bool

>> +    operator>(const _Bit_iterator_base& __x, const 

>> _Bit_iterator_base& __y)

>> +    { return __y < __x; }

>>

>> -    bool

>> -    operator<=(const _Bit_iterator_base& __i) const

>> -    { return !(__i < *this); }

>> +    friend bool

>> +    operator<=(const _Bit_iterator_base& __x, const 

>> _Bit_iterator_base& __y)

>> +    { return !(__y < __x); }

>>

>> -    bool

>> -    operator>=(const _Bit_iterator_base& __i) const

>> -    { return !(*this < __i); }

>> -  };

>> +    friend bool

>> +    operator>=(const _Bit_iterator_base& __x, const 

>> _Bit_iterator_base& __y)

>> +    { return !(__x < __y); }

>>

>> -  inline ptrdiff_t

>> -  operator-(const _Bit_iterator_base& __x, const _Bit_iterator_base& 

>> __y)

>> -  {

>> -    return (int(_S_word_bit) * (__x._M_p - __y._M_p)

>> -        + __x._M_offset - __y._M_offset);

>> -  }

>

> For the non-member operator- and operator+ this change makes sense,

> but what is the benefit of changing all the others? As members they're

> already not considered as candidates for unrelated types, or am I

> missing something?

>

>

Well, I remember a pretty old conversation about making all operators 
consistent that is to say not sometimes member and sometimes non-member.

So I am trying to make all operators non-member as much as possible. 
That's indeed the only reason.

Patch

diff --git a/libstdc++-v3/include/bits/stl_bvector.h b/libstdc++-v3/include/bits/stl_bvector.h
index 19c16839cfa..8fbef7a1a3a 100644
--- a/libstdc++-v3/include/bits/stl_bvector.h
+++ b/libstdc++-v3/include/bits/stl_bvector.h
@@ -182,40 +182,40 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       _M_offset = static_cast<unsigned int>(__n);
     }
 
-    bool
-    operator==(const _Bit_iterator_base& __i) const
-    { return _M_p == __i._M_p && _M_offset == __i._M_offset; }
+    friend bool
+    operator==(const _Bit_iterator_base& __x, const _Bit_iterator_base& __y)
+    { return __x._M_p == __y._M_p && __x._M_offset == __y._M_offset; }
 
-    bool
-    operator<(const _Bit_iterator_base& __i) const
+    friend bool
+    operator<(const _Bit_iterator_base& __x, const _Bit_iterator_base& __y)
     {
-      return _M_p < __i._M_p
-	    || (_M_p == __i._M_p && _M_offset < __i._M_offset);
+      return __x._M_p < __y._M_p
+	    || (__x._M_p == __y._M_p && __x._M_offset < __y._M_offset);
     }
 
-    bool
-    operator!=(const _Bit_iterator_base& __i) const
-    { return !(*this == __i); }
+    friend bool
+    operator!=(const _Bit_iterator_base& __x, const _Bit_iterator_base& __y)
+    { return !(__x == __y); }
 
-    bool
-    operator>(const _Bit_iterator_base& __i) const
-    { return __i < *this; }
+    friend bool
+    operator>(const _Bit_iterator_base& __x, const _Bit_iterator_base& __y)
+    { return __y < __x; }
 
-    bool
-    operator<=(const _Bit_iterator_base& __i) const
-    { return !(__i < *this); }
+    friend bool
+    operator<=(const _Bit_iterator_base& __x, const _Bit_iterator_base& __y)
+    { return !(__y < __x); }
 
-    bool
-    operator>=(const _Bit_iterator_base& __i) const
-    { return !(*this < __i); }
-  };
+    friend bool
+    operator>=(const _Bit_iterator_base& __x, const _Bit_iterator_base& __y)
+    { return !(__x < __y); }
 
-  inline ptrdiff_t
-  operator-(const _Bit_iterator_base& __x, const _Bit_iterator_base& __y)
-  {
-    return (int(_S_word_bit) * (__x._M_p - __y._M_p)
-	    + __x._M_offset - __y._M_offset);
-  }
+    friend ptrdiff_t
+    operator-(const _Bit_iterator_base& __x, const _Bit_iterator_base& __y)
+    {
+      return (int(_S_word_bit) * (__x._M_p - __y._M_p)
+	      + __x._M_offset - __y._M_offset);
+    }
+  };
 
   struct _Bit_iterator : public _Bit_iterator_base
   {
@@ -280,29 +280,29 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       return *this;
     }
 
-    iterator
-    operator+(difference_type __i) const
+    reference
+    operator[](difference_type __i) const
+    { return *(*this + __i); }
+
+    friend iterator
+    operator+(const iterator& __x, difference_type __n)
     {
-      iterator __tmp = *this;
-      return __tmp += __i;
+      iterator __tmp = __x;
+      return __tmp += __n;
     }
 
-    iterator
-    operator-(difference_type __i) const
+    friend iterator
+    operator+(difference_type __n, const iterator& __x)
+    { return __x + __n; }
+
+    friend iterator
+    operator-(const iterator& __x, difference_type __n)
     {
-      iterator __tmp = *this;
-      return __tmp -= __i;
+      iterator __tmp = __x;
+      return __tmp -= __n;
     }
-
-    reference
-    operator[](difference_type __i) const
-    { return *(*this + __i); }
   };
 
-  inline _Bit_iterator
-  operator+(ptrdiff_t __n, const _Bit_iterator& __x)
-  { return __x + __n; }
-
   struct _Bit_const_iterator : public _Bit_iterator_base
   {
     typedef bool                 reference;
@@ -370,29 +370,29 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
       return *this;
     }
 
-    const_iterator
-    operator+(difference_type __i) const
+    const_reference
+    operator[](difference_type __i) const
+    { return *(*this + __i); }
+
+    friend const_iterator
+    operator+(const const_iterator& __x, difference_type __n)
     {
-      const_iterator __tmp = *this;
-      return __tmp += __i;
+      const_iterator __tmp = __x;
+      return __tmp += __n;
     }
 
-    const_iterator
-    operator-(difference_type __i) const
+    friend const_iterator
+    operator-(const const_iterator& __x, difference_type __n)
     {
-      const_iterator __tmp = *this;
-      return __tmp -= __i;
+      const_iterator __tmp = __x;
+      return __tmp -= __n;
     }
 
-    const_reference
-    operator[](difference_type __i) const
-    { return *(*this + __i); }
+    friend const_iterator
+    operator+(difference_type __n, const const_iterator& __x)
+    { return __x + __n; }
   };
 
-  inline _Bit_const_iterator
-  operator+(ptrdiff_t __n, const _Bit_const_iterator& __x)
-  { return __x + __n; }
-
   inline void
   __fill_bvector(_Bit_type * __v,
 		 unsigned int __first, unsigned int __last, bool __x)