[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

BRevzin added inline comments. Comment at: clang/test/AST/conditionally-trivial-smfs.cpp:39 + +template struct DefaultConstructorCheck<2>; +// CHECK: "kind": "ClassTemplateSpecializationDecl", royjacobson wrote: > BRevzin wrote: > > It's possible that

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

BRevzin added inline comments. Comment at: clang/test/AST/conditionally-trivial-smfs.cpp:39 + +template struct DefaultConstructorCheck<2>; +// CHECK: "kind": "ClassTemplateSpecializationDecl", It's possible that I just don't understand what these test

[PATCH] D78938: Make LLVM build in C++20 mode

BRevzin added a comment. In D78938#2450915 , @nlopes wrote: > Thanks @lebedev.ri for the pointer! > I started working on exactly the same thing as I was trying to link a C++20 > project with LLVM. > @BRevzin is there anything missing in this patch? Do you

[PATCH] D78938: Make LLVM build in C++20 mode

BRevzin added inline comments. Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h:167-171 - -inline bool operator!=(const DWARFExpression::iterator &LHS, - const DWARFExpression::iterator &RHS) { - return !(LHS == RHS); -} lebe

[PATCH] D78938: Make LLVM build in C++20 mode

BRevzin added inline comments. Comment at: llvm/include/llvm/IR/BasicBlock.h:324-325 +template ::value>> phi_iterator_impl(const phi_iterator_impl &Arg) dblaikie wrote: > BRevzin wrote: > > dblaikie wrote: > > > What tripped over/required this SFINAE? >

[PATCH] D78938: Make LLVM build in C++20 mode

BRevzin added inline comments. Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h:167-171 - -inline bool operator!=(const DWARFExpression::iterator &LHS, - const DWARFExpression::iterator &RHS) { - return !(LHS == RHS); -} dbla

[PATCH] D78938: Make LLVM build in C++20 mode

BRevzin added a comment. > @BRevzin, you should a) mention the u8/const char* issue in the description > too, and also what compiler you used to build this with. I fully expect at > this stage that there are some C++20 compilers that might have slightly > different interpretations of things whi

[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 th

[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 t

[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 &M, const NodeType &N) { !(M == N); } jfb wrote: > davidstone wrote: > > Missing `return` > 😱 > > Did this not trigger a diagnosti

[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.

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: clang/include/c

[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 clang/lib/Pa

[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 appropriat

[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: clang/inc

[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 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 clang/lib/Parse/Pa

[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 cfe-commits@lists.llvm.o

[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 != unsig

[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 In

[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 themselve

[PATCH] D41740: [clang-tidy] Adding a new bugprone check for streaming objects of type int8_t or uint8_t

BRevzin marked an inline comment as done. BRevzin added a comment. In https://reviews.llvm.org/D41740#968134, @JonasToth wrote: > Could you please add a test case with a template that reduces the type to > int8 or uint8? I don't actually know how to do that. I tried a few things, but getting t

[PATCH] D41740: [clang-tidy] Adding a new bugprone check for streaming objects of type int8_t or uint8_t

BRevzin updated this revision to Diff 128768. BRevzin added a comment. Updates based on review comments - and rebased off of latest so as to get the ReleaseNotes right. Added options so that the user can provide the list of stream types and int typedef types, as desired, defaulting to just `b

[PATCH] D41740: [clang-tidy] Adding a new bugprone check for streaming objects of type int8_t or uint8_t

BRevzin added a comment. So admittedly, I don't have any experience in clang-tidy. This check is intended to catch what is to us a really annoying source of error (and I'm probably not the only one?). I suspect there are far better ways of writing this check than the one that I pretty much gues