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

jfb added a comment. On C++20 mode rotting: it won't if someone sets up a bot. If it rots, then it's easier to un-rot with Barry's patch. Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:805-817 + return IsASCII ? "^" : (const char *)u8"\u2548"; case

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

jhenderson added a comment. In D78938#2259342 , @BRevzin wrote: > In D78938#2258557 , @jhenderson > wrote: > >> Not that I have anything particularly against this, but won't this likely >> rot fairly rapidly?

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

BRevzin added inline comments. Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:805-817 + return IsASCII ? "^" : (const char *)u8"\u2548"; case LineChar::RangeMid: - return IsASCII ? "|" : u8"\u2503"; + return IsASCII ? "|" : (const char *)u8"\u2503";

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

BRevzin added a comment. In D78938#2258557 , @jhenderson wrote: > Not that I have anything particularly against this, but won't this likely rot > fairly rapidly? It's not like LLVM is even on C++17 let alone C++20 yet, so > trying to make it work like

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

jhenderson added a comment. Not that I have anything particularly against this, but won't this likely rot fairly rapidly? It's not like LLVM is even on C++17 let alone C++20 yet, so trying to make it work like the latter when it's just going to break again seems a bit like wasted effort to me.

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

BRevzin updated this revision to Diff 290023. BRevzin added a comment. Herald added subscribers: rupprecht, MaskRay. Herald added a reviewer: jhenderson. Herald added a reviewer: MaskRay. Updating this review with some additional changes that need to be made since I last touched it, and some of

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

BRevzin added inline comments. Comment at: llvm/include/llvm/ADT/DirectedGraph.h:97 + } + friend bool operator!=(const NodeType , const NodeType ) { !(M == N); } jfb wrote: > davidstone wrote: > > Missing `return` >  > > Did this not trigger a diagnostic

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

BRevzin updated this revision to Diff 266071. BRevzin marked 2 inline comments as done. BRevzin added a comment. - Explaining the cryptic parentheses. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78938/new/ https://reviews.llvm.org/D78938 Files:

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

davidstone added a comment. I noticed the missing return because there is a warning (not as error) that caught it, I think the warning about falling off the end of a non-void-returning function. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

jfb accepted this revision. jfb added a comment. One suggestions, otherwise looks good. Thanks for doing this :) Comment at: llvm/include/llvm/ADT/DirectedGraph.h:97 + } + friend bool operator!=(const NodeType , const NodeType ) { !(M == N); } davidstone

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

BRevzin updated this revision to Diff 265900. BRevzin added a comment. - Backing out changes that aren't strictly comparison-related. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78938/new/ https://reviews.llvm.org/D78938 Files:

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

BRevzin updated this revision to Diff 265875. BRevzin added a comment. - Adding missing return. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78938/new/ https://reviews.llvm.org/D78938 Files: clang/include/clang/AST/StmtIterator.h

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

BRevzin updated this revision to Diff 265874. BRevzin added a comment. Herald added a subscriber: martong. - A few more changes from tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78938/new/ https://reviews.llvm.org/D78938 Files:

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

BRevzin marked 2 inline comments as done. BRevzin added a comment. I hadn't build the tests before, updated with a few more changes. Some of the tests require `u8` literals, whose type changes in C++20. I had no idea what to do with that, so I just `#ifdef`-ed out those tests with the

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

davidstone added inline comments. Herald added a subscriber: sstefan1. Comment at: llvm/include/llvm/ADT/DirectedGraph.h:97 + } + friend bool operator!=(const NodeType , const NodeType ) { !(M == N); } Missing `return` Repository: rG LLVM Github Monorepo

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

jfb accepted this revision. jfb added a comment. This revision is now accepted and ready to land. This seems fine, assuming you've run usual tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78938/new/ https://reviews.llvm.org/D78938

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

dblaikie added a comment. In D78938#2007571 , @BRevzin wrote: > In D78938#2007037 , @dblaikie wrote: > > > (peanut gallery: I'd consider, while you're touching these all anyway, > > changing them all to non-member

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

BRevzin added a comment. In 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

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

dblaikie added a comment. (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

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

BRevzin added a comment. In D78938#2006758 , @hubert.reinterpretcast wrote: > In D78938#2006715 , @BRevzin wrote: > > > Wtf, where'd my other changes go? > > > I've hit this before, use `arc diff --update D78938

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

BRevzin updated this revision to Diff 260521. BRevzin added a comment. Trying this again. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78938/new/ https://reviews.llvm.org/D78938 Files: clang/include/clang/AST/StmtIterator.h

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

hubert.reinterpretcast added a comment. In D78938#2006715 , @BRevzin wrote: > Wtf, where'd my other changes go? I've hit this before, use `arc diff --update D78938 `. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

BRevzin added a comment. Wtf, where'd my other changes go? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78938/new/ https://reviews.llvm.org/D78938 ___ cfe-commits mailing list

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

BRevzin marked 2 inline comments as done. BRevzin added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:69-70 + } bool operator==(OpenMPDirectiveKind V) const { return Value == unsigned(V); } bool operator!=(OpenMPDirectiveKind V) const { return Value !=

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

BRevzin updated this revision to Diff 260477. BRevzin added a comment. More idiomatic comparison implementation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78938/new/ https://reviews.llvm.org/D78938 Files: clang/lib/Parse/ParseOpenMP.cpp

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

rsmith added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:64-67 +return Value == unsigned(V); + } + bool operator!=(OpenMPDirectiveKindExWrapper V) const { +return Value != unsigned(V); Comparing against `V.Value` here would seem more

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

Quuxplusone added inline comments. Comment at: llvm/include/llvm/Support/BinaryStreamRef.h:124 - bool operator==(const RefType ) const { -if (BorrowedImpl != Other.BorrowedImpl) + friend bool operator==(const RefType , const RefType ) { +if (Self.BorrowedImpl !=

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

BRevzin created this revision. Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, hiraditya, MatzeB. Herald added a reviewer: jdoerfert. Herald added projects: clang, LLVM. Part of the <=> changes in C++20 make certain patterns of writing equality operators ambiguous with