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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
