[PATCH] D119184: [clang] [concepts] Check constrained-auto return types for void-returning functions

Quuxplusone updated this revision to Diff 413006. Quuxplusone added a comment. Herald added a project: All. Rebased, poke CI one last time. If this is green, imma land it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119184/new/ https://reviews.ll

[PATCH] D119184: [clang] [concepts] Check constrained-auto return types for void-returning functions

This revision was automatically updated to reflect the committed changes. Closed by commit rGf0891cd61b2f: [clang] [concepts] Check constrained-auto return types for void-returning… (authored by arthur.j.odwyer). Changed prior to commit: https://reviews.llvm.org/D119184?vs=413006&id=413046#toc

[PATCH] D119778: [clang] Add a note "deducing return type for 'foo'"

Quuxplusone updated this revision to Diff 413089. Quuxplusone added a comment. Herald added a project: All. Rebased after landing D119184 . I'm no longer necessarily trying to land this note (it's not directly relevant to my interests anymore), but I'd like to s

[PATCH] D114382: [clang] Fix wrong -Wunused-local-typedef warning within a template function

Quuxplusone added inline comments. Comment at: clang/test/SemaCXX/warn-unused-local-typedef.cpp:246 + typedef int Int; // no-diag + typedef char Char; // expected-warning {{unused typedef 'Char'}} + Int m; I haven't tried to understand the main poi

[PATCH] D79714: [Diagnostics] Restore -Wdeprecated warning when user-declared copy assignment operator is defined as deleted (PR45634)

Quuxplusone accepted this revision. Quuxplusone added a comment. This revision is now accepted and ready to land. The new individual test files are awesome. :) The name of the `-W` options still aren't perfect IMHO, but maybe it will never be perfect, and meanwhile everything else looks great.

[PATCH] D101598: [clang][Sema] adds `[[clang::no_address]]` attribute

Quuxplusone added inline comments. Comment at: clang/test/SemaCXX/attr-no-address.cpp:47-48 +void address_test() { + void (*fp1)() = &std::one_overload; + // expected-error@-1{{cannot take address of 'std::one_overload' because functions in namespace 'std' are not addressable}

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

Quuxplusone added a comment. In D63423#2732152 , @hvdijk wrote: > It's bad enough that this warns for > > #define A 2 > int f() { return A ^ 1; } > > where as far as the users of A are concerned... I see how this warning is arguably overzealous in the //v

[PATCH] D79714: [Diagnostics] Restore -Wdeprecated warning when user-declared copy assignment operator is defined as deleted (PR45634)

Quuxplusone added a comment. In D79714#2735455 , @Abpostelnicu wrote: > I'm seeing here something very strange, if the user provided copy assignment > operator is provided but is = delete and the copy constructor is default this > warning will still tri

[PATCH] D101899: std::forward_list::swap to use propagate_on_container_swap for noexcept specification

Quuxplusone added subscribers: ldionne, Quuxplusone. Quuxplusone accepted this revision as: Quuxplusone. Quuxplusone added a comment. This does actually look like a pretty straightforward typo in `forward_list`; the other containers all seem to get it right. It's too bad that none of our tests no

[PATCH] D116351: Update Bug report URL to Github Issues

Quuxplusone added inline comments. Comment at: libcxx/docs/index.rst:220 * `libc++abi Homepage `_ -* `LLVM Bugzilla `_ +* `LLVM Issues `_ * `libcxx-commits Mailing List`_ -

[PATCH] D114425: [clang] Add __builtin_bswap128

Quuxplusone added a comment. In D114425#3216490 , @craig.topper wrote: > In D114425#3216231 , @philnik wrote: > >> In D114425#3209794 , @craig.topper >> wrote: >> >>> Wh

[PATCH] D112221: Mark ATOMIC_VAR_INIT and ATOMIC_FLAG_INIT as deprecated

Quuxplusone added a comment. TLDR on `_CLANG_DISABLE_CRT_DEPRECATION_WARNINGS`: I'm neutral. :) Comment at: clang/docs/ReleaseNotes.rst:159-164 + - The ``ATOMIC_VAR_INIT`` macro from is now diagnosed as + deprecated in both C (C17 and later) and C++ (C++20 and later), an

[PATCH] D114425: [clang] Add __builtin_bswap128

Quuxplusone added a comment. In D114425#3216802 , @majnemer wrote: > OOC, how hard would it be to generalize this builtin a little? It is nice > that we have builtins like `__builtin_add_overflow` which do the right thing > regardless of their input. >

[PATCH] D115867: [C++20] [Coroutines] Warning for always_inline coroutine

Quuxplusone requested changes to this revision. Quuxplusone added a comment. This revision now requires changes to proceed. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:0-3 +def warn_always_inline_coroutine : Warning< + "A coroutine marked always_inlin

[PATCH] D116775: [clang][#47272] Avoid suggesting deprecated version of a declaration over another in typo correction

Quuxplusone requested changes to this revision. Quuxplusone added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Sema/SemaLookup.cpp:4345 + return; } } It seems like this code is overdue for a "compare these two

[PATCH] D116775: [clang][#47272] Avoid suggesting deprecated version of a declaration over another in typo correction

Quuxplusone added a comment. LGTM % comments, but I'll take one more look. Comment at: clang/test/SemaCXX/typo-correction.cpp:772-782 +namespace B { +int pr47272(); // expected-note{{'B::pr47272' declared here}} +} + +namespace [[deprecated]] A { +using B::pr47272; +}

[PATCH] D112221: Mark ATOMIC_VAR_INIT and ATOMIC_FLAG_INIT as deprecated

Quuxplusone accepted this revision. Quuxplusone added a comment. LGTM FWIW. I'm not the right person to approve for libc, but I'm not sure any of the currently listed reviewers are any more appropriate either. So you should find and ping a new reviewer, or just ship it, at your discretion. :)

[PATCH] D116775: [clang][#47272] Avoid suggesting deprecated version of a declaration over another in typo correction

Quuxplusone accepted this revision. Quuxplusone added a comment. This revision is now accepted and ready to land. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116775/new/ https://reviews.llvm.org/D116775 ___ cfe-commits mailing list cf

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

Quuxplusone accepted this revision. Quuxplusone added a comment. This revision is now accepted and ready to land. @Sockke: Throughout, `trivially-copyable` should be `trivially copyable` (two words). Other than that, sure, I have no particular opinions at this point. Comment a

[PATCH] D116778: [clang-tidy][clang] Don't trigger unused-parameter warnings on naked functions

Quuxplusone added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp:290-292 +// Do not warn on naked functions. +[[gnu::naked]] int nakedFunction(int a, float b, const char *c) { ; } +__attribute__((naked)) void nakedFunction(int

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

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3211-3215 +The compiler will pass and return a trivially relocatable type using the C ABI +for the underlying type, even when the type would otherwise be considered +non-trivially-relocatable. If

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

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3211-3215 +The compiler will pass and return a trivially relocatable type using the C ABI +for the underlying type, even when the type would otherwise be considered +non-trivially-relocatable. If

[PATCH] D115374: [NFC][clang] Return std::strings built from raw_string_ostreams more efficiently

Quuxplusone added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h:36 +OS.flush(); +return Str; } FWIW, it appears to me that in most (all?) of these cases, what's really wanted is not "a string //and// a stream

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

Quuxplusone added a comment. Am I seeing correctly that there's no feature-test macro for `auto(x)`? I'm proposing to introduce `#define _LIBCPP_AUTO_CAST(expr)` into libc++, so that we can use it for Ranges things that specify in terms of `auto(x)`... but if I want to make it conditionally exp

[PATCH] D115867: [C++20] [Coroutines] Warning for always_inline coroutine

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:0-3 +def warn_always_inline_coroutine : Warning< + "A coroutine marked always_in

[PATCH] D115867: [C++20] [Coroutines] Warning for always_inline coroutine

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:0-3 +def warn_always_inline_coroutine : Warning< + "A coroutine marked always_inline might not be inlined properly." + >, + InGroup; ChuanqiXu wrote: > Quux

[PATCH] D116204: [C++20] [Coroutines] Allow promise_type to not define return_void or return_value

Quuxplusone added inline comments. Comment at: clang/test/SemaCXX/coroutines.cpp:987 +// +// So it isn't ill-formed if the promise doesn't define return_value and return_void. It is just a UB. +coro no_return_value_or_return_void() { It's not UB; it's //potentia

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

Quuxplusone added inline comments. Comment at: clang/unittests/Format/TemplateArgumentKeywordFixerTest.cpp:119 + Style); +} + My personal recommended style is that the programmer uses `template` consistently and therefore anytime you see the (more

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

Quuxplusone added inline comments. Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:55 +// For `auto` language version, be conservative and assume we are < C++17 +KeepTemplateTemplateKW = (Style.Standard == FormatStyle::LS_Auto) || +

[PATCH] D116351: Update Bug report URL to Github Issues

Quuxplusone accepted this revision. Quuxplusone added a comment. Tweaked English wording throughout. LGTM with all these modifications made. Orthogonally, it does occur to me that one might save a //lot// of this churn (and also the next time the bug tracker moves, e.g. if we go from GitHub to

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

Quuxplusone added a comment. Confirmed: yikes. @nullptr.cpp, I'm going to open a new pull request to revert this, to poke the buildbot, just in case reverting it causes new failures. But I certainly don't object if someone wants to be more bold than me in reverting quickly. Here's a reduced tes

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

Quuxplusone created this revision. Quuxplusone added reviewers: rsmith, nullptr.cpp, aaronpuchert, mizvekov, erik.pilkington, sberg, davidstone, david_stone. Quuxplusone added a project: clang. Quuxplusone requested review of this revision. Herald added a subscriber: cfe-commits. Review D88220

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

Quuxplusone added a comment. To whom it may concern, see D98971 . I'm (unwisely?) trying a surgical fix rather than trying to revert the month-old thing. (Let's please discuss anything D98971 -specific over there.) Repository:

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

Quuxplusone updated this revision to Diff 331973. Quuxplusone added a comment. Per @mizvekov, use `VDType` instead of `VD->getType()` wherever possible. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98971/new/ https://reviews.llvm.org/D98971 Files

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

Quuxplusone marked 2 inline comments as done. Quuxplusone added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:3095 - // ...non-volatile... - if (VD->getType().isVolatileQualified()) -return false; - - // C++20 [class.copy.elision]p3: - // ...rvalue reference

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

Quuxplusone marked 3 inline comments as done. Quuxplusone added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:3095 - // ...non-volatile... - if (VD->getType().isVolatileQualified()) -return false; - - // C++20 [class.copy.elision]p3: - // ...rvalue reference

[PATCH] D99005: [clang] WIP: Implement P2266

Quuxplusone added a comment. Conspicuously missing: tests for `decltype(auto)` return types, tests for `co_return`. Personally I'd rather see these new p2266-related tests go in a separate test file, not appended to the existing file, so as to keep each test file relatively simpler and shorter

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

Quuxplusone updated this revision to Diff 332663. Quuxplusone added a comment. Shrink the code by one line, by introducing another local named variable. Still hoping for an "accept" here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98971/new/ htt

[PATCH] D98971: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5f1de9cab1ce: [C++20] [P1825] Fix bugs with implicit-move from variables of reference type. (authored by arthur.j.odwyer). Repository: rG LLVM Git

[PATCH] D99225: [clang] cxx tests: cleanup, update and add some new ones

Quuxplusone added a comment. IMHO you should just go ahead and land the trivial trailing-whitespace changes. Comment at: clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p7-cxx14.cpp:3 +// RUN: %clang_cc1 -verify -std=c++20 -verify %s +// RUN: %clang_cc1 -verify -std=c++

[PATCH] D99005: [clang] WIP: Implement P2266

Quuxplusone added a comment. Data points: I've run this new Clang in `-std=c++2b` mode over my employer's (medium-sized, not too crazy) C++17 codebase, and also built Clang with Clang. Neither one required any source-code changes to deal with p2266. (The C++17 codebase did need about 10 identic

[PATCH] D99456: [C++2b] Support size_t literals

Quuxplusone added inline comments. Comment at: clang/lib/Frontend/InitPreprocessor.cpp:593-594 + // C++2b features. + if (LangOpts.CPlusPlus2b) +Builder.defineMacro("__cpp_size_t_suffix", "202011L"); if (LangOpts.Char8) aaron.ballman wrote: > AntonBikine

[PATCH] D99456: [C++2b] Support size_t literals

Quuxplusone added inline comments. Comment at: clang/lib/Frontend/InitPreprocessor.cpp:593-594 + // C++2b features. + if (LangOpts.CPlusPlus2b) +Builder.defineMacro("__cpp_size_t_suffix", "202011L"); if (LangOpts.Char8) AntonBikineev wrote: > AntonBikine

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

Quuxplusone added a comment. Wow, nice catches. For posterity, here's the missed-optimization being addressed IIUC: https://godbolt.org/z/MEoKGs7cE Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99696/new/ https://reviews.llvm.org/D99696 _

[PATCH] D99225: [clang] tests: cleanup, update and add some new ones

Quuxplusone added a comment. LGTM (just a bunch of style comments from me), but I think you'll have to get someone else's attention if you're expecting to get signoff from someone to land this. I also think it would make sense for you to land the absolutely trivial trailing-whitespace diffs ri

[PATCH] D99225: [clang] tests: cleanup, update and add some new ones

Quuxplusone added a comment. There's unlikely to be any further substantive comments from me, and this basically LGTM (or, in the places it looks bad, it's just the pre-existing awfulness of how the Clang test suite is organized). I think you need to either get someone's attention to approve thi

[PATCH] D99893: [WIP] Replace std::forward & std::move by cast expressions during Sema

Quuxplusone added a comment. No matter how it works internally, I think that (nearer the end of the process) someone should insist that you add some Clang tests to verify e.g. Widget&& a = static_cast(Widget()); // is lifetime-extended, but Widget&& b = std::move(Widget()); //

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

Quuxplusone added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:3140 +/// \param ReturnType This is the return type of the function. +void Sema::updNRVOResultWithRetType(NRVOResult &Res, QualType ReturnType) { + if (!Res.Candidate) mizvekov wrote: > a

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

Quuxplusone added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:3084-3085 + bool ForceCXX20) { + bool hasCXX11 = getLangOpts().CPlusPlus11 || ForceCXX20, + hasCXX20 = getLangOpts().CPlusPlus20 || ForceCXX20; + Named

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

Quuxplusone added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:3066 - if (isCopyElisionCandidate(ReturnType, VD, CESK)) -return VD; - return nullptr; +static void downgradeNRVOResult(Sema::NRVOResult &Res, bool CanMove) { + Res.S = std::min(Res.S, CanMove ? S

[PATCH] D100778: [clang-format] Prevent extraneous space insertion in bitshift operators

Quuxplusone added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:7652 + verifyFormat("test < a - 1 >> 1;"); verifyFormat("test >> a >> b;"); IMO you should use `"test < a | b >> c;"` as your test case here, to reassure the reader that i

[PATCH] D100778: [clang-format] Prevent extraneous space insertion in bitshift operators

Quuxplusone added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:7652 + verifyFormat("test < a - 1 >> 1;"); verifyFormat("test >> a >> b;"); Quuxplusone wrote: > IMO you should use `"test < a | b >> c;"` as your test case here, to reassu

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

Quuxplusone added a subscriber: oneraynyday. Quuxplusone added a comment. In D76572#2472221 , @lebedev.ri wrote: > Do you plan to follow-up with the diagnostic itself, in some form? No, I don't. My diagnostic code was just an ad-hoc noisy hack with no hop

[PATCH] D93962: [Sema] Fix the program state in reference initialization

Quuxplusone added a comment. This could use some tests. (What's observably bad about the old code's behavior? does this patch fix that behavior?) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93962/new/ https://reviews.llvm.org/D93962 ___

[PATCH] D93955: [Sema] Add support for reporting multiple errors during initialization

Quuxplusone added inline comments. Comment at: clang/test/SemaCXX/diagnostic-initialization.cpp:7 +template +class initializer_list { + const _E *__begin_; Looking at other places like `clang/test/CXX/dcl.decl/dcl.init/dcl.init.list/p3.cpp`, I think you don't

[PATCH] D92936: [Sema] Fix deleted function problem in implicitly movable test

Quuxplusone added a comment. In D92936#2476091 , @aeubanks wrote: > This broke [ https://godbolt.org/z/9EG7ev ] > is this intentional? Yes, intentional. This brings Clang's C++14 conformance into line with GCC/EDG/MSVC, who all already (correctly) reject

[PATCH] D92936: [Sema] Fix deleted function problem in implicitly movable test

Quuxplusone added a comment. In D92936#2476356 , @vitalybuka wrote: > Something is not initialized > http://lab.llvm.org:8011/#/builders/74/builds/1834/steps/9/logs/stdio Confirmed; @nullptr.cpp what do you want to do about this? I hypothesize that maybe

[PATCH] D92936: [Sema] Fix deleted function problem in implicitly movable test

Quuxplusone added a comment. In D92936#2477764 , @dmajor wrote: > Before the revert, our bots hit the following issue where we only error out > when `-Wall` is given, so there's definitely something strange going on. https://godbolt.org/z/P1dv9f Yeah, I

[PATCH] D92936: [Sema] Fix deleted function problem in implicitly movable test

Quuxplusone added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:4020 bool IsListInit = false, bool IsInitListCopy = false) { assert(((!IsListInit && !IsInitListCopy) || -

[PATCH] D93095: Introduce -Wreserved-identifier

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:377 + "%0 is a reserved identifier">, + InGroup, DefaultIgnore; + If you leave it like this, you //will// receive multiple bug reports per year about how in some co

[PATCH] D93095: Introduce -Wreserved-identifier

Quuxplusone added inline comments. Comment at: clang/test/Sema/reserved-identifier.cpp:77 + +long double operator"" _BarbeBleue(long double) // no-warning +{ This should get a warning, since it's using an identifier "reserved for any use." https://stackoverflow.

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

Quuxplusone added a comment. @nullptr.cpp: I may or may not come up with more review comments, but 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 and/or force-of-will to

[PATCH] D90188: Add support for attribute 'using_if_exists'

Quuxplusone added inline comments. Comment at: clang/test/SemaCXX/using-if-exists.cpp:11 + +using NotNS::x UIE; // expected-error{{use of undeclared identifier 'NotNS'}} +} // test_basic Do you also have a test for `using NS::NotNS::x UIE;`? (Both of these `NotNS

[PATCH] D110436: Add %n format specifier warning

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9230 +def warn_printf_n_specifier : Warning< + "usage of '%%n' can lead to unsafe writing to memory">, InGroup; def warn_printf_data_arg_not_used : Warning< FWIW, I

[PATCH] D110436: Add %n format specifier warning

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9230 +def warn_printf_n_specifier : Warning< + "usage of '%%n' can lead to unsafe writing to memory">, InGroup; def warn_printf_data_arg_not_used : Warning< Jaysony

[PATCH] D111215: clang release notes: document the -Wbool-operation improvement

Quuxplusone added inline comments. Comment at: clang/docs/ReleaseNotes.rst:54 -- ... +- -Wbitwise-instead-of-logical (part of -Wbool-operation) warns about use of bitwise with boolean operands xbolva00 wrote: > I think we should mention that with boolean ope

[PATCH] D111655: [analyzer] non-obvious analyzer warning: Use of zero-allocated memory

Quuxplusone added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2474-2475 auto R = std::make_unique( -*BT_UseZerroAllocated[*CheckKind], "Use of zero-allocated memory", N); +*BT_UseZerroAllocated[*CheckKind], +"Use o

[PATCH] D112356: [NFC] Tidy up spelling, grammar, and inconsistencies in clang-tidy documentation

Quuxplusone added a comment. Definitely an improvement! I looked at all the changed places and found some more improvements you can make. I don't need to see this again, though. The only disimprovement I found was "Jaro–Winkler" becoming "Jaro-Winkler". Comment at: clang-tool

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

Quuxplusone added a comment. In D108696#3082866 , @ChuanqiXu wrote: > @Quuxplusone gentle ping~ I think this PR is mostly above my pay grade. :) IIUC, there is a chicken-and-egg problem between D108696 and D109433

[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:262-263 + int a = 10; + forwardToShowInt(std::move(a)); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: std::move of the variable 'a' of the trivially-copy

[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:262-263 + int a = 10; + forwardToShowInt(std::move(a)); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: std::move of the variable 'a' of the trivially-copy

[PATCH] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

Quuxplusone added a comment. Peanut gallery says: Does this godbolt demonstrate the situation where "destroying an exception can throw an exception"? https://godbolt.org/z/Ghjz53rrj You're not proposing to change the behavior of this program, are you? (If you're proposing to change it: I don't th

[PATCH] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

Quuxplusone added a comment. > Well, I'm saying two things. First, it is not clear to me what the expected > behavior of that code is under the standard. The fact that it appears to work > in one particular implementation is not in any way conclusive; we have to > look at the specification. Ah

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

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1553 + "%select{references|destructors|block pointers|ref-qualified member functions}2">, + InGroup>; + @cjdb: I suggest splitting this diagnostic up, mainly for s

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

Quuxplusone added a comment. In D108003#2983609 , @xbolva00 wrote: > I think I will start with AND only as this is more error prone pattern. FWIW, I still see no reason //not// to warn on `|`-for-`||`. Again per https://quuxplusone.github.io/blog/2020/0

[PATCH] D109408: [libcxxabi] NFC: fix incorrect indentation of braces

Quuxplusone added inline comments. Comment at: libcxxabi/src/cxa_exception_storage.cpp:102 } } +} // namespace __cxxabiv1 LGTM, FWIW. It's surprising that clang-format adds `} // namespace foo` closing comments, but doesn't add `} // extern "C"` closing comme

[PATCH] D109411: [clang] Enable the special enable_if_t diagnostics for libc++'s _EnableIf as well.

Quuxplusone created this revision. Quuxplusone added reviewers: ldionne, doug.gregor, rsmith, dblaikie, courbet, saar.raz. Quuxplusone added a project: clang. Quuxplusone requested review of this revision. Herald added a subscriber: cfe-commits. This comes from lengthy discussion between @Quuxplu

[PATCH] D109411: [clang] Enable the special enable_if_t diagnostics for libc++'s __enable_if_t as well.

Quuxplusone updated this revision to Diff 371350. Quuxplusone retitled this revision from "[clang] Enable the special enable_if_t diagnostics for libc++'s _EnableIf as well." to "[clang] Enable the special enable_if_t diagnostics for libc++'s __enable_if_t as well.". Quuxplusone edited the summar

[PATCH] D109411: [clang] Enable the special enable_if_t diagnostics for libc++'s __enable_if_t as well.

Quuxplusone added a comment. In D109411#2990494 , @dblaikie wrote: > Looks like most of the testing for the enable_if custom dialog was added in > `clang/test/SemaTemplate/overload-candidates.cpp` in > rG6f8d2c6c9c3451effdf075a7034bbe77045bfeba >

[PATCH] D109408: [libcxxabi] NFC: fix incorrect indentation of braces

Quuxplusone added a comment. I was going to land this just now, but then realized that I've lost the thread of why we're doing most of this. Why are we touching clang/test/SemaCXX/? So I'm going to land just the fixing of the curly braces in libcxxabi/src/. Comment at: libcxx

[PATCH] D109408: [libcxxabi] NFC: fix incorrect indentation of braces

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6e91666e2864: [libcxxabi] NFC: fix incorrect indentation of braces (authored by zhouyizhou, committed by arthur.j.odwyer). Changed prior to commit:

[PATCH] D109411: [clang] Enable the special enable_if_t diagnostics for libc++'s __enable_if_t as well.

This revision was automatically updated to reflect the committed changes. Closed by commit rG2b4cad5e471c: [clang] Enable the special enable_if_t diagnostics for libc++'s __enable_if_t… (authored by arthur.j.odwyer). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

<    2   3   4   5   6   7