[PATCH] D95691: Implement P2173 for attributes on lambdas

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/DiagnosticGroups.td:269 +def CXXPre2BCompatPedantic : + DiagGroup<"c++98-c++11-c++14-c++17-c++20-compat-pedantic", [CXXPre2BCompat]>; aaron.ballman wrote: > rjmccall wrote: > > Uh, I thin

[PATCH] D109800: [clang] don't mark as Elidable CXXConstruct expressions used in NRVO

Quuxplusone added a comment. FWIW, this is almost entirely above my pay grade. Getting rid of that extra bool parameter throughout //looks// awesome, though. :) I do think it might be a good idea to include one or two tests that actually run all the way through codegen, e.g. https://godbolt.org

[PATCH] D109975: [CMake] Consistently use the LibXml2::LibXml2 target instead of LIBXML2_LIBRARIES

Quuxplusone added a comment. Herald added a subscriber: JDevlieghere. Serendipitously, I just yesterday upgraded from OSX 10.14 to 10.15, and have been unable to build clang because, even with a totally-empty-and-brand-new build directory, CMake says: CMake Error in lib/WindowsManifest/CMakeL

[PATCH] D109975: [CMake] Consistently use the LibXml2::LibXml2 target instead of LIBXML2_LIBRARIES

Quuxplusone added a comment. I was inspired to dig down and find the `LLVM_ENABLE_LIBXML2` option (which is on by default) and turn it off, so now I can build clang with the following commands: cmake -G Ninja -DLLVM_ENABLE_LIBXML2=0 -DDEFAULT_SYSROOT="$(xcrun --show-sdk-path)" -DLLVM_ENABLE_

[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

Quuxplusone added a comment. LGTM % comments, FWIW. Comment at: clang/test/Sema/warn-bitwise-and-bool.c:26 + b = foo() & a; + b = (p != 0) & (*p == 42); + b = foo() & (*q == 42); // expected-warning {{use of bitwise '&' with boolean operands}} Perhaps add a

[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:69 def note_cast_to_void : Note<"cast expression to void to silence warning">; +def note_cast_operand_to_int : Note<"cast one or both operands to int to silence warning">;

[PATCH] D37538: [libc++] Remove problematic ADL in container implementations.

Quuxplusone added a comment. Everything in this PR is obsolete and/or fixed at this point, //except// for the change to , which as Eric said, fixes the lower-hanging fruit but sadly can't "ADL-proof" it completely. (Grepping for "ADL" in the git history of `libcxx/include/` will turn up a lot o

[PATCH] D88220: [C++20] P1825R0: More implicit moves

Quuxplusone accepted this revision. Quuxplusone added a comment. This revision is now accepted and ready to land. > either way I'm a dead end — I don't consider myself authorized to "accept" > Clang changes. > You're still going to have to attract the attention of someone with the > authority an

[PATCH] D88220: [C++20] P1825R0: More implicit moves

Quuxplusone added a comment. In D88220#2566525 , @aaronpuchert wrote: > I think you're much more of an expert on implicit moves than most of us Short answer, yes. ;) My concern was more that I'm much less of an expert on Clang than "most of us." I'd rat

[PATCH] D88220: [C++20] P1825R0: More implicit moves

This revision was automatically updated to reflect the committed changes. Closed by commit rGfbee4a0c79cc: [C++20] [P1825] More implicit moves (authored by nullptr.cpp, committed by arthur.j.odwyer). Changed prior to commit: https://reviews.llvm.org/D88220?vs=318420&id=324115#toc Repository:

[PATCH] D50106: [libc++] Fix tuple assignment from types derived from a tuple-like

Quuxplusone added inline comments. Comment at: libcxx/include/tuple:432 +void __memberwise_copy_assign(_Dest& __dest, _Source const& __source, __tuple_indices<_Np...>) { +__swallow(((_VSTD::get<_Np>(__dest) = _VSTD::get<_Np>(__source)), void(), int())...); +} --

[PATCH] D96832: [Clang][Attributes] Allow not_tail_called attribute to be applied to virtual function.

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4109 - [[clang::not_tail_called]] int foo2() override; -}; }]; (Moving into a thread) > This patch doesn't prevent the call to method in the code below from being >

[PATCH] D96832: [Clang][Attributes] Allow not_tail_called attribute to be applied to virtual function.

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4109 - [[clang::not_tail_called]] int foo2() override; -}; }]; aaron.ballman wrote: > ahatanak wrote: > > Quuxplusone wrote: > > > (Moving into a thread) > > > > > >

[PATCH] D96971: Allow but ignore `-Wreturn-std-move-in-c++11`

Quuxplusone accepted this revision. Quuxplusone added a comment. This revision is now accepted and ready to land. FWIW, no particular comment from me. I agree with everything @thakis said. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96971/new/ https://reviews.llvm.org/D96971

[PATCH] D50106: [libc++] Fix tuple assignment from types derived from a tuple-like

Quuxplusone accepted this revision. Quuxplusone added a comment. Suggested some ways to improve test coverage, and continued bikeshedding on the SFINAE ;) but there's no reason to hold this up AFAIC. Comment at: libcxx/include/tuple:58 tuple& operator=(const tuple&); -

[PATCH] D50106: [libc++] Fix tuple assignment from types derived from a tuple-like

Quuxplusone added inline comments. Comment at: libcxx/include/tuple:971 +_VSTD::get<0>(*this) = _VSTD::forward<_Up1>(__pair.first); +_VSTD::get<1>(*this) = _VSTD::forward<_Up2>(__pair.second); +return *this; Oh, late-breaking nit: I would

[PATCH] D96832: [Clang][Attributes] Allow not_tail_called attribute to be applied to virtual function.

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4087-4088 -Marking virtual functions as ``not_tail_called`` is an error: +Marking virtual functions as ``not_tail_called`` will not have effect on the +overriding functions of derived classes:

[PATCH] D96832: [Clang][Attributes] Allow not_tail_called attribute to be applied to virtual function.

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4087-4088 -Marking virtual functions as ``not_tail_called`` is an error: +Marking virtual functions as ``not_tail_called`` will not have effect on the +overriding functions of derived classes:

[PATCH] D102760: [llvm] Let SmallVector construct from any Iterable

Quuxplusone added a comment. @gchatelet: Thanks for the link to D102679 — that seems like very relevant background info I didn't have! Having looked through D102679 , though, I don't see how it would benefit from D102760

[PATCH] D102782: Add support for Warning Flag "-Wstack-usage="

Quuxplusone added inline comments. Comment at: clang/docs/ReleaseNotes.rst:79-80 +- ``-Wstack-usage=`` warn if stack usage of user functions might + exceed . + Does this mean: - Warn if the size of any single function's stack frame (including temporaries and

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

Quuxplusone accepted this revision. Quuxplusone added a comment. This revision is now accepted and ready to land. @mizvekov, my understanding is: - D99696 was temporarily blocked on D103720 , but now D103720

[PATCH] D102736: Fix tmp files being left on Windows builds.

Quuxplusone added inline comments. Comment at: clang/lib/Frontend/CompilerInstance.cpp:857-858 + OS.reset(new llvm::raw_fd_ostream(Temp->FD, /*shouldClose=*/false, +Binary ? llvm::sys::fs::OF_None +

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

Quuxplusone accepted this revision. Quuxplusone added a comment. This revision is now accepted and ready to land. IMHO we should just land this already. (It's been sitting without major update since mid-April.) It is intended to affect only `-std=c++2b` mode, and implements a paper (full disclo

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

Quuxplusone added a comment. @hans: FYI, that looks related to the immediately following D99005 , not D99696 specifically. I suspect that reverting D99005 (but leaving D99696

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

Quuxplusone added a comment. @sberg: Thanks for the example! @mizvekov's comments are correct, but I wanted to put some more comments here for posterity. (I'll also try to find a place for this real-world example in the next revision of p2266

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

Quuxplusone added a comment. Some tiny nits. I can't think of any interesting block-related cases (but my knowledge of blocks is apparently very rusty at this point). Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1100 +// This is the last chance we have of che

[PATCH] D104500: DRAFT: [clang] Apply P1825 as DR for all modes below C++20.

Quuxplusone added inline comments. Comment at: clang/test/CXX/class/class.init/class.copy.elision/p3.cpp:58-60 A1 test1(A1 &&a) { - return a; // cxx11_17-error {{call to deleted constructor of 'test_implicitly_movable_rvalue_ref::A1'}} + return a; } Personal

[PATCH] D104680: [clang] Eliminate relational function pointer comparisons in all C++ modes

Quuxplusone added a comment. > There isn't any wording on it or anything, yet, but we must jump the gun here > and just do away with it pronto. I think "must" is the wrong word here. "Might as well"? Anyway, I agree with this general idea, FWLIW. Comment at: clang/lib/Sema/S

[PATCH] D106252: Make simple requirements starting with requires ill-formed in in requirement body

Quuxplusone accepted this revision. Quuxplusone added a comment. This revision is now accepted and ready to land. @cor3ntin: Sure, LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106252/new/ https://reviews.llvm.org/D106252 ___

[PATCH] D107292: [clang] adds warning to alert user when they use alternative tokens as references

Quuxplusone added a comment. @dblaikie wrote: > Not a huge objection - but minor quandry: What's the motivation for this > patch? AIUI, this diagnostic is a PR stunt, similar to Clang's existing `-Wunicode-homoglyph`: test.cpp:3:13: warning: treating Unicode character as identifier charac

[PATCH] D106394: [clang][pp] adds '#pragma include_instead'

Quuxplusone added inline comments. Comment at: clang/lib/Lex/Pragma.cpp:1996 AddPragmaHandler("clang", new PragmaSystemHeaderHandler()); + AddPragmaHandler("clang", new PragmaIncludeInsteadHandler()); AddPragmaHandler("clang", new PragmaDebugHandler()); @r

[PATCH] D107294: [clang-tidy] adds warning to suggest users replace symbols with words

Quuxplusone added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/UseAlternativeTokensCheck.cpp:41-42 + Finder->addMatcher( + binaryOperation(hasAnyOperatorName("&&", "||", "!", "&", "|", "~", "^")) + .bind("operator"), + this);

[PATCH] D107402: Correct a lot of diagnostic wordings for the driver

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:88 +def err_drv_bad_target_id : Error< + "invalid target ID: %0 (a target ID is a processor name followed by an " + "optional list of predefined features post-fixed by a plus or mi

[PATCH] D107450: [clang-tidy] Fix wrong and missing warnings in performance-move-const-arg

Quuxplusone requested changes to this revision. Quuxplusone added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp:75 + return std::move(x4); + // CHECK-MESSAGES: :[[@LINE-1]]

[PATCH] D107450: [clang-tidy] Fix wrong and missing warnings in performance-move-const-arg

Quuxplusone added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp:261 + showInt(std::move(a)); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effec

[PATCH] D107760: [clang] Fix warning -Wnon-virtual-dtor.

Quuxplusone added inline comments. Comment at: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp:311 : OpenCLBuiltinFileEmitterBase(Records, OS) {} + virtual ~OpenCLBuiltinTestEmitter() = default; Tip: When you're putting a defaulted virtual destructor

[PATCH] D107450: [clang-tidy] Fix wrong and missing warnings in performance-move-const-arg

Quuxplusone added a comment. I'm not at all convinced that any of this complexity is worth it. Have you run this check on any large codebase to see how many false positives it has? Does it catch any true positives? Comment at: clang-tools-extra/test/clang-tidy/checkers/perfo

[PATCH] D107900: Add a new clang-tidy check for trivially copyable types passed by const reference

Quuxplusone added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst:21 + + If set to `true`, this check will limit itself to the `builtinType()` types. Default is `false`. D107873 is related. I'd like to se

[PATCH] D107450: [clang-tidy] Fix wrong and missing warnings in performance-move-const-arg

Quuxplusone added a comment. > I suggest that this patch be divided into two patches. In the current patch, > fix the wrong AutoFix. What the current check should look like is left in the > second patch for discussion. @Sockke do you mind simplifying this patch and > only achieving the first go

[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

Quuxplusone added subscribers: mclow.lists, Quuxplusone. Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7428-7430 +def warn_bitwise_and_bool : Warning< + "bitwise and of boolean expressions; did you mean logical and?">, + InGroup

[PATCH] D107450: [clang-tidy] Fix wrong FIxIt in performance-move-const-arg

Quuxplusone added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp:263 + forwardToShowInt(std::move(a)); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has

[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

Quuxplusone added a comment. FWIW, all three of @nathanchance's detected lines look like good true positives to me: even if they're not //bugs//, they all look like places the programmer //meant// to write `&&` and only wrote `&` by accident. The third one might even be a bug bug, since it's do

[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7428-7430 +def warn_bitwise_and_bool : Warning< + "bitwise and of boolean expressions; did you mean logical and?">, + InGroup; xbolva00 wrote: > Quuxplusone wrote: >

[PATCH] D108003: [Clang] Extend -Wbool-operation to warn about bitwise and of bools with side effects

Quuxplusone added a comment. > Can we either emit that [`a &= foo()`] as a suggestion if the code looks like > `a = a & foo()`? Or simply not emit a warning? The problem is that if `a` is boolean, then `a = a & something` is 99% for sure a typo — 99% of the time, the programmer meant `a = a &&

[PATCH] D107450: [clang-tidy] Fix wrong FIxIt in performance-move-const-arg

Quuxplusone added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp:263 + forwardToShowInt(std::move(a)); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has

[PATCH] D104975: Implement P1949

Quuxplusone added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:1462 + static const llvm::sys::UnicodeCharSet XIDStartChars(XIDStartRanges); + return C == '_' || XIDStartChars.contains(C); + } else if (LangOpts.C11) { This is overly inde

[PATCH] D104984: [clang] add C++ feature test macro for P2266 simpler implicit move

Quuxplusone accepted this revision. Quuxplusone added a comment. This revision is now accepted and ready to land. LGTM. I assume we pick the integer value `202011` for everything C++2b-related until the standard ships with a real value in it, and then we update our integer to match. Repository

[PATCH] D105024: Add papers adopted by the C++ committee in the June 2021 plenary.

Quuxplusone added a comment. Shouldn't the "yes/Yes" entries display Clang major version numbers instead? Throughout, "NO" should be "No" (compare to lines 1221 and 1226). Comment at: clang/www/cxx_status.html:1287 + + if consteval + https://wg21.link/P1938R3";>P1

[PATCH] D105024: Add papers adopted by the C++ committee in the June 2021 plenary.

Quuxplusone added a comment. @cor3ntin do you have any interest in making a similar patch for the LWG issues? :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105024/new/ https://reviews.llvm.org/D105024 __

[PATCH] D105380: [clang] fixes named return of variables with dependent alignment

Quuxplusone added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:13317 + std::any_of(range.begin(), range.end(), [](const AlignedAttr *I) { + return I->isAlignmentDependent(); + }); Tangent: It would be nice to rename `isAlignm

[PATCH] D105439: [clang] protects users from relying on libc++ detail headers

Quuxplusone requested changes to this revision. Quuxplusone added a comment. This revision now requires changes to proceed. Step 1 should be to find out if this is even a problem at all. For example, try using one of these tools to compile a C++ program against GNU libstdc++, or against a librar

[PATCH] D105439: [clang] protects users from relying on libc++ detail headers

Quuxplusone added a comment. I should add that if any particular library really wants to //enforce// "thou shalt not deep-link detail headers" (instead of just documenting it and/or relying on the user-programmer's common sense) they can do it pretty

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

Quuxplusone added a comment. > I'd like to give some time for other stakeholders to give their opinion if > this is not too urgent, specially @Quuxplusone and @rsmith. I have no strong/well-informed opinions here. I have an idea to offer, //if// there is precedent for it. My idea is, keep it as

[PATCH] D104500: [clang] Apply P1825 as Defect Report from C++11 up to C++20.

Quuxplusone added a comment. In D104500#2864909 , @mizvekov wrote: > In D104500#2863368 , @jyknight > wrote: > >> This commit seems to have broken libc++ in C++98 mode, as it appears to have >> depended upon the

[PATCH] D105756: [clang] C++98 implicit moves are back with a vengeance

Quuxplusone accepted this revision. Quuxplusone added a comment. This revision is now accepted and ready to land. It would be an excellent idea to try the libc++ test suite with this patch (make sure to pass `--param std=c++03`). Comment at: clang/lib/Sema/SemaStmt.cpp:3484 +

[PATCH] D105439: [clang] protects users from relying on libc++ detail headers

Quuxplusone added a comment. Given that the goal is to get `include-what-you-use` to stop suggesting that users `#include <__detail/fooimpl.h>` when what they want is spelled ``, I think what's desired here (if anything) would be a pragma targeted at `include-what-you-use` (and other tools). Ba

[PATCH] D108696: [Coroutines] [Frontend] Lookup in std namespace first

Quuxplusone added a comment. @lewissbaker wrote: > #include // which transitively includes > #include Good example! I had not previously been thinking about transitive includes. I believe we "obviously" don't need to cater to code that manually includes both `` and `` in the same source

[PATCH] D108696: [Coroutines] [Frontend] Lookup in std namespace first

Quuxplusone added a comment. In D108696#3095789 , @ChuanqiXu wrote: > Since there are other `experimental/*` headers moved in to normal include > paths, I guess there may be similar problems before. I think this problem is > not limited in coroutine. So

[PATCH] D112579: Allow non-variadic functions to be attributed with `__attribute__((format))`

Quuxplusone added a comment. In D112579#3097360 , @aaron.ballman wrote: > [...] Having the types in the signatures changes something I think might be > pretty fundamental to the way the format string checker works -- it tries to > figure out types *aft

[PATCH] D112579: Allow non-variadic functions to be attributed with `__attribute__((format))`

Quuxplusone added a comment. > Agreed. This should be checking the instantiations, so by that point, the > variadic template is really more like a fixed parameter list anyway. FWIW, in my own mental model, there's a big semantic difference between (varargs functions, variadic templates) on the

[PATCH] D112221: Mark ATOMIC_VAR_INIT and ATOMIC_FLAG_INIT as deprecated

Quuxplusone added a comment. If libc++ is using these macros, then I think it would be useful to include (the removal of) those uses in this PR. ../libcxx/include/atomic:#define ATOMIC_VAR_INIT(value) see below ../libcxx/include/atomic:#define ATOMIC_FLAG_INIT see below ../libcxx/include/a

[PATCH] D112221: Mark ATOMIC_VAR_INIT and ATOMIC_FLAG_INIT as deprecated

Quuxplusone added a comment. > I don't know enough about libc++ to feel comfortable making those changes > yet. For example, does libc++ need to work with other compilers than Clang > (compilers which might give diagnostics if you fail to use ATOMIC_VAR_INIT in > some language mode)? The deprec

[PATCH] D113517: Correct handling of the 'throw()' exception specifier in C++17.

Quuxplusone added a comment. Peanut gallery says: It seems like `Proto->canThrow()` is already returning the correct answer for C++17-and-later: a function declared `throw()` cannot throw. So from the POV of C++17-and-later, this could be a simple patch: - if (isNoexceptExceptionSpec(EST) &&

[PATCH] D96832: [Clang][Attributes] Allow not_tail_called attribute to be applied to virtual function.

Quuxplusone accepted this revision. Quuxplusone added a comment. Marking "accepted" from me, merely to indicate that my grammar stuff is not blocking and I don't expect I'll have any further substantive comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4101 + Fo

[PATCH] D98104: Update __is_unsigned builtin to match the Standard.

Quuxplusone added a comment. 🎉 Comment at: clang/lib/Sema/SemaExprCXX.cpp:4837 +// Enum types should always return false (same as UTT_IsSigned). +return !T->isEnumeralType() && T->isUnsignedIntegerType(); FWIW, I'd lose the parenthetical comment, and

[PATCH] D98104: Update __is_unsigned builtin to match the Standard.

Quuxplusone added a comment. LGTM now, mod the (important!) typo in the test. Comment at: clang/test/SemaCXX/type-traits.cpp:1444 int t27[F(__is_signed(UnionAr))]; + int t28[F(__is_unsigned(UnsignedEnum))]; } s/is_unsigned/is_signed/ Repository: rG LLV

[PATCH] D98160: [clang] Use decltype((E)) for compound requirement type constraint

Quuxplusone added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:8838 +/// that expression, without accounting for the rules. +static QualType getBaseDecltypeForExpr(Sema &S, Expr *E) { + // C++11 [dcl.type.simple]p4: I think a better name for this mig

[PATCH] D113393: [c++2b] Implement P0849R8 auto(x)

Quuxplusone added a comment. Comments on tests. Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1032 + // the typename-specifier in a function-style cast expression may + // be 'auto' since C++2b Diag(Tok.getLocation(), rsmith wrote: > Nice catch

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

Quuxplusone requested changes to this revision. Quuxplusone added inline comments. This revision now requires changes to proceed. Comment at: clang/unittests/Format/FormatTest.cpp:22574 + verifyFormat("foo(auto{{}})"); +} + Looks like a good improvement to me! H

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

Quuxplusone accepted this revision. Quuxplusone added a comment. This revision is now accepted and ready to land. Personally I'd remove the tests that are invalid expressions; but I don't know what's the usual practice. Ship it AFAIC! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113826

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

arthur.j.odwyer added subscribers: MyDeveloperDay, arthur.j.odwyer. arthur.j.odwyer added a comment. Sorry, I meant remove the first four cases that are invalid C++, e.g. auto{}. Personally I'd keep the "valid but arguably misformatted" cases (which I had suggested earlier but you just removed). H

[PATCH] D113826: [clang-format][c++2b] support removal of the space between auto and {} in P0849R8

Quuxplusone added a comment. In D113826#3129383 , @lichray wrote: > void f() { > T { &a } -> n = 1; > } > > and > > void h() { > auto{ &a } -> n = 0; > } > > But this may not worth a fix (for now). Kibitzing: I don't think those cases mat

[PATCH] D113977: [Coroutine] Warn deprecated 'std::experimental::coro' uses

Quuxplusone requested changes to this revision. Quuxplusone added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11015-11017 + "Found deprecated std::experimental::%0. Consider to update your libc++ "

[PATCH] D113977: [Coroutine] Warn deprecated 'std::experimental::coro' uses

Quuxplusone accepted this revision as: Quuxplusone. Quuxplusone added a comment. FWIW, the problem with SpacesInAngles: Leave ^ is that the debian builder has an outdated (or maybe just 13.x?) `clang-format`; this option is super new. I don't know what the appropriate c

[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

Quuxplusone requested changes to this revision. Quuxplusone added a comment. This revision now requires changes to proceed. Peanut gallery says: I'd recommend //not// taking this particular patch; it seems much less "obvious" than the whitelist/inclusionlist type of patch. Whereas "whitelist" is

[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

Quuxplusone added inline comments. Comment at: clang/include/clang/AST/Redeclarable.h:261 assert(Current && "Advancing while iterator has reached end"); - // Sanity check to avoid infinite loop on invalid redecl chain. + // Validation check to avoid infinite loop

[PATCH] D114151: [clang-format] [C++20] [Module] clang-format couldn't recognize partitions

Quuxplusone added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:22443 + Style); +} + I'd like to see `module :private;` I'd like to see `import ;` I'd like to see `import foo...bar;` I'd like to see `import ..;` Is `modu

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

Quuxplusone added a comment. In D113898#3140320 , @kuhnel wrote: > When trying to revert some of the changes, I just noticed there are a couple > of `if (const auto* ...` in `CodeComplete.cpp` and `AST.cpp` (and maybe other > files as well). So some fol

[PATCH] D114151: [clang-format] [C++20] [Module] clang-format couldn't recognize partitions

Quuxplusone added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:22443 + Style); +} + Quuxplusone wrote: > I'd like to see `module :private;` > I'd like to see `import ;` > I'd like to see `import foo...bar;` > I'd like to see `i

[PATCH] D114151: [clang-format] [C++20] [Module] clang-format couldn't recognize partitions

Quuxplusone added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3235 +if (Left.is(TT_ModulePartitionColon) && +Right.isOneOf(tok::identifier, tok::kw_public, tok::kw_private)) + return false; owenpan wrote: > Is `module :public

[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

Quuxplusone added a comment. (Re repeated thanks: You're welcome. :) For the record, I personally see nothing wrong with the phrase "sanity-check" either; but given that it's gonna happen, and we're re-wording comments on by definition the subtlest and most confusing parts of the code, I'm tryi

[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

Quuxplusone added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:9176 // Don't check the implicit member of the anonymous union type. - // This is technically non-conformant, but sanity demands it. + // This is technically non-conformant, but validat

[PATCH] D114562: [clang][docs] Inclusive language: remove use of sanity check in option description

Quuxplusone added inline comments. Comment at: clang/docs/ThreadSafetyAnalysis.rst:471 - + ``-Wthread-safety-attributes``: Sanity checks on attribute syntax. + + ``-Wthread-safety-attributes``: Validation checks on attribute syntax. + ``-Wthread-safety-analysis``: The core

[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

Quuxplusone added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp:342 + auto w = new int(x); +} What about ``` template T foo(int i) { return T(i); } int main() { foo>(); // valid, OK(!) foo(); // v

[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

Quuxplusone added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp:342 + auto w = new int(x); +} salman-javed-nz wrote: > carlosgalvezp wrote: > > carlosgalvezp wrote: > > > Quuxplusone wrote: > > > > What a

[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

Quuxplusone accepted this revision. Quuxplusone added a comment. Marking "accepted" for the record; but my checkmark means merely "I'm not intending to block this," not "I claim the authority to say you //should// land this." :) Comment at: clang-tools-extra/clang-tidy/google

[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

Quuxplusone added inline comments. Comment at: clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp:101-110 +static StringRef getDestTypeString(const SourceManager &SM, + const LangOptions &LangOpts, +

[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

Quuxplusone added inline comments. Comment at: clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp:63-79 +static clang::CharSourceRange getReplaceRange(const CStyleCastExpr *CastExpr) { + return CharSourceRange::getCharRange( + CastExpr->getLParenLoc(), CastExpr-

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

Quuxplusone added a comment. On the one hand, I like that someone else is pushing a patch that's basically the same as D50119 , because maybe if enough people reimplement the same thing, eventually Clang will accept one of them. ;) However, I'd like to point out:

[PATCH] D114427: [clang-tidy] Warn on functional C-style casts

Quuxplusone added a comment. LGTM! Comment at: clang-tools-extra/test/clang-tidy/checkers/google-readability-casting.cpp:318 // FIXME: This should be a static_cast. // C HECK-FIXES: auto s5 = static_cast(cr); auto s6 = (S)cr; salman-javed-nz wrote: >

[PATCH] D114859: [clang-format] Add better support for co-routinues

Quuxplusone accepted this revision. Quuxplusone added a comment. This revision is now accepted and ready to land. LGTM % comments. I agree with all the formatting decisions shown in the new tests. Comment at: clang/docs/ReleaseNotes.rst:270 +- Improved Coroutinues support. +

[PATCH] D114859: [clang-format] Add better support for co-routinues

Quuxplusone added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:22727 + +TEST_F(FormatTest, CoRoutineawait) { + verifyFormat("int x = co_await foo();"); MyDeveloperDay wrote: > Quuxplusone wrote: > > > naming of the tests is to allow easy r

[PATCH] D114902: [Attrs] Elaborate on the trivial_abi documentation

Quuxplusone added a comment. LgenerallyGTM! - I would say "special member functions" instead of "methods" - instead of "passed indirectly by address", consider saying "passed by hidden reference" — is this something people will get, or is it a me-ism? :) There are lots of Google hits for "by hi

[PATCH] D115061: [clang-format][NFC] Prefer pass by reference

Quuxplusone added a comment. FWLIW, I'm strongly in favor of "Pass out-parameters by pointer," for the reason Marek said (and the reason Google, Bloomberg, Facebook, Mongo, etc, do it) — it makes life easier for the reader of the calling code. Especially for e.g. `addNextStateToQueue(Penalty, N

[PATCH] D113319: [clang-format] Improve require and concept handling

Quuxplusone added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:3424 +templates or between template and function delcarations. In case of +after the function delcaration it tries to stick to this. + `s/delca/decla/g` `s/Preceeding/Pr

[PATCH] D113369: [clang-format] Extend SpaceBeforeParens for requires

Quuxplusone added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:3756 + * ``bool AfterRequiresClause`` If ``true``, put space between requires keyword in a requires clause and +opening parentheses, if is are one. + HazardyKnusperkeks wr

[PATCH] D113319: [clang-format] Improve require and concept handling

Quuxplusone added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:3464-3483 + * ``RCPS_TwoLines`` (in configuration: ``TwoLines``) +The clause always gets its own line, and the content of the clause go +into the next line with some indentation. + +

[PATCH] D113369: [clang-format] Extend SpaceBeforeParens for requires

Quuxplusone added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:3756 + * ``bool AfterRequiresClause`` If ``true``, put space between requires keyword in a requires clause and +opening parentheses, if is are one. + HazardyKnusperkeks wr

[PATCH] D114197: [clang-tidy] Fix false positives involving type aliases in `misc-unconventional-assign-operator` check

Quuxplusone added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp:151 + using Alias3 = TemplateTypeAlias; + Alias3 &operator=(int) { return *this; } +}; fwolff wrote: > whisperity wrote: > > This

[PATCH] D76572: Replace `T(x)` with `reinterpret_cast(x)` everywhere it means reinterpret_cast. No functional change

Quuxplusone created this revision. Quuxplusone added reviewers: rsmith, espindola, DiggerLin, ahatanak, reames. Herald added subscribers: cfe-commits, rupprecht, dexonsmith, MaskRay, hiraditya. Herald added a reviewer: jhenderson. Herald added a project: clang. I wrote a Clang warning [not pictur

<    1   2   3   4   5   6   7   >