Make safe_iterator inline friends

[Jonathan Wakely]

On 29/08/18 07:54 +0200, François Dumont wrote:
>On 28/08/2018 21:04, Jonathan Wakely wrote:
>>On 23/08/18 22:59 +0200, François Dumont wrote:
>>>On 22/08/2018 23:45, Jonathan Wakely wrote:
>>>>On 22/08/18 23:08 +0200, François Dumont wrote:
>>>>>Only operator== and != remains outside _Safe_iterator because 
>>>>>all my attempts to make them inline friends failed. I 
>>>>>understand that an inline friend within a base class is not a 
>>>>>very clean design.
>>>>>
>>>>>Compiler error was:
>>>>>
>>>>>/home/fdt/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/safe_iterator.h:459: 
>>>>>error: redefinition of 'bool __gnu_debug::operator==(const 
>>>>>_Self&, const _OtherSelf&)'
>>>>>/home/fdt/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/safe_iterator.h:452: 
>>>>>note: 'bool __gnu_debug::operator==(const _Self&, const 
>>>>>_Self&)' previously declared here
>>>>>/home/fdt/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/safe_iterator.h:473: 
>>>>>error: redefinition of 'bool __gnu_debug::operator!=(const 
>>>>>_Self&, const _OtherSelf&)'
>>>>>/home/fdt/dev/gcc/build/x86_64-pc-linux-gnu/libstdc++-v3/include/debug/safe_iterator.h:466: 
>>>>>note: 'bool __gnu_debug::operator!=(const _Self&, const 
>>>>>_Self&)' previously declared here
>>>>>
>>>>>I don't know if it is a compiler issue
>>>>
>>>>I don't think so. The error seems clear: when _Self and _OtherSelf are
>>>>the same type the friend declarations are the same function.
>>>>
>>>>
>>>_Self and _OtherSelf and like the types defined in 
>>>_Safe_iterator<_It, _Sq, random_access_interator_tag> in this 
>>>patch. Depending on __conditional_type so definitely different.
>>
>>What about containers like std::set where iterator and const_iterator
>>are the same type?
>
>Good idear but no, it is not that.
>
>It really looks like g++ consider the inline friend definition in the 
>context of the instantiation of _Safe_iterator<It, Seq, 
>std::forward_access_iterator_tag> but also in the context of 
>_Safe_iterator<t, Seq, std::bidirectionnal_iterator_tag> and same for 
>RAI.

Yes, that's expected.

>I don't know if this behavior is correct or not but for sure it is not 
>a clean design so I would prefer to avoid it keeping those operators 
>in __gnu_debug namespace. Attach is my attempt to inline those if you 
>want to have a closer look and maybe fill a compiler bug entry.

I'm sure there is no compiler bug here. Consider:

template<typename T>
struct Iter
{
  template<typename U>
    friend bool
    operator==(Iter<U>, Iter<U>) { return true; }
};

Iter<int> i;
Iter<long> j;


This fails:

dup_friend.cc: In instantiation of 'struct Iter<long int>':
dup_friend.cc:10:12:   required from here
dup_friend.cc:6:5: error: redefinition of 'template<class U> bool operator==(Iter<U>, Iter<U>)'
6 |     operator==(Iter<U>, Iter<U>) { return true; }
  |     ^~~~~~~~
dup_friend.cc:6:5: note: 'template<class U> bool operator==(Iter<U>, Iter<U>)' previously declared here


The problem is that the friend function does not depend on the
template parameters of the enclosing class. That means that every
different specialization of Iter defines an identical friend function.

The specialization Iter<int> defines:

  template<typename U>
    friend bool
    operator==(Iter<U>, Iter<U>) { return true; }

and the specialization Iter<long> defines:

  template<typename U>
    friend bool
    operator==(Iter<U>, Iter<U>) { return true; }

That means you have two function definitions with identical
signatures, which is invalid. You can only define a function once.

A friend function defined inline in a class template needs to depend
on the enclosing class, so that each specialization of the class
template defines a *different* friend function.

For example:

template<typename T>
struct Iter
{
  friend bool
  operator==(Iter, Iter) { return true; }
};

Or:

template<typename T>
struct Iter
{
  template<typename U>
    friend bool
    operator==(Iter, Iter<U>) { return true; }

  template<typename U> friend class Iter<U>;
};

(This latter case will only be a friend of the left operand, not the
right operand. So access to private members of the right operand would
have to be via some other function, maybe a member of the left
operand, because Iter<T> and Iter<U> are friends.)