[PATCH] D78938: Fixing all comparisons for C++20 compilation.

Tue Apr 28 08:34:19 PDT 2020

dblaikie added a comment.

In D78938#2007571 <https://reviews.llvm.org/D78938#2007571>, @BRevzin wrote:

> In D78938#2007037 <https://reviews.llvm.org/D78938#2007037>, @dblaikie wrote:
>
> > (peanut gallery: I'd consider, while you're touching these all anyway, changing them all to non-member (friended where required) as I believe that's best practice - allows equal implicit conversions on either side, for instance (even if some types have no implicit conversions - it at least provides a nice consistency/examples that people are likely to copy from))
>
>
> Hidden friend is probably the best way to write comparisons in C++17 and earlier, but I'm not sure that will hold in C++20 (even if LLVM isn't on C++20 and won't be for I imagine quite some time). With reversed candidates, I think member functions might be the way to go there - you still get implicit conversions on either side (just not on both sides at the same time) and hidden friends... are kind of weird, to be honest.

Yeah, probably just experience with things the way they've been, but the symmetry is kind of nice without relying on deeper aspects of the newer features (& the benefit of the code being more suitable for C++17, where LLVM is currently).

> Also, I didn't touch all of them - only the ones that break in C++20 (a lot of which just missing a `const`). A lot of comparison operators are already fine. I'm not sure it's worth changing them just to look the same.

Yeah - just meant the ones you are touching, might be nice to move them in that direction.

Anyway, I'll leave it to you/other reviewers - no /super/ strong feelings here.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78938/new/

https://reviews.llvm.org/D78938