[PATCH] D47467: [IR] Begin removal of TerminatorInst by removing successor manipulation.

[Dean Michael Berris via Phabricator via llvm-commits]

dberris added inline comments.


================
Comment at: llvm/include/llvm/IR/CFG.h:193
+
+  inline bool operator==(const Self &x) const { return Idx == x.Idx; }
+
----------------
chandlerc wrote:
> dberris wrote:
> > Maybe this should be a non-member friend, and works with const and non-const versions of compatible type?
> > 
> > ```
> > friend inline bool operator==(const Self& L, const Self& R) {
> >   return L.Idx == R.Idx;
> > }
> > ```
> It has to be a member for the underlying iterator helper to work (due to how that underlying helper is implemented).
> 
> Is there really a problem to solve here though? Not sure what the benefit is of this in the absence of a type hierarchy... We don't do it very consistently in LLVM anywhere.
Ah, that's fair -- didn't know that the facade requires equality to be defined as a member.

The only problems the friend declaration solves are the overload resolution issue (if you want to have type deduction of the RHS to support slightly different types) and forces ADL when defined inline -- i.e. the only way to find the operator is through ADL.

Consider:

```
template <class T>
class C {
  template <class U> friend bool operator==(const C& L, const C<U>& R) {
    return ...;
  }
  ...
};
```

This allows C<int> and C<const int> types to be comparable, without having to make any special considerations. It also only allows for the equality comparison to be found with ADL, and no other way (AFAICT).

Whether that needs to be more widely adopted in LLVM/Clang is a separate issue, probably not worth spending a lot more time on.


Repository:
  rL LLVM

https://reviews.llvm.org/D47467