[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

[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

[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=413046#toc

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

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

Quuxplusone added a comment. In D120398#3351998 , @MyDeveloperDay wrote: > Before this lands can we have a discussion about what clarity this gives us?, > because I think it makes the code more unreadable, but surely we are saving > just 7x(3 bytes)

[PATCH] D120629: [clang] Remove unused variable AllElementsInt. NFC.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd3db74eadbfc: [clang] Remove unused variable AllElementsInt. (authored by arthur.j.odwyer). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D120629: [clang] Remove unused variable AllElementsInt. NFC.

Quuxplusone created this revision. Quuxplusone added reviewers: rnk, rsmith, sammccall. Quuxplusone added a project: clang. Quuxplusone requested review of this revision. Herald added a subscriber: cfe-commits. This has been unused ever since it was committed in b8a501ccf1. Repository: rG

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

Quuxplusone added a comment. > Any runtime penalty? (Usually compilers choose a faster underlying type for > enums, not smaller) > The decision is made in the header (so I think it will always be `int` until > this doesn't fit). To decide what is faster the compiler would need to know > all

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

Quuxplusone added a comment. In D113393#3345275 , @aaron.ballman wrote: >> Also implemented decltype(auto)(x) (https://wg21.link/p0849r2) as a Clang >> extension. > > I'd like to better understand the use cases for this. WG21 considered this as > part

[PATCH] D120454: clang/www: Add links to tracking issues for C++20 features

Quuxplusone added inline comments. Comment at: clang/www/cxx_status.html:106 +automatically fetch status information from the issue in the tracker. For Example: +a class='autoupdate' href='https://github.com/llvm/llvm-project/issues/54006'No/a + HTML nit:

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

Quuxplusone added inline comments. Comment at: clang/test/CodeGenCXX/nrvo.cpp:1537 + } + return x; // FIXME: NRVO could happen if B == false, but doesn't +} Quuxplusone wrote: > Nit: `s/if/when/` Serendipitously, I just ran into almost this exact scenario in

[PATCH] D120086: Explicitly document that `__is_trivially_relocatable(T)` implies that `T` is implicit-lifetime.

Quuxplusone added a comment. FWIW, I think this is a good idea. I even think that the parentheses and "Note:" are too self-deprecating, and this deserves to be a bigger deal. Perhaps `__is_implicit_lifetime(T)` should be an intrinsic in its own right! Then you could say very concretely that

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

Quuxplusone updated this revision to Diff 409740. Quuxplusone added a comment. Rebase and update — this is becoming more and more of a trivial patch, which I guess is good! Add a test case for https://github.com/llvm/llvm-project/issues/53911 (which I finally thought to test, and was surprised

[PATCH] D119772: [clang] [NFC] More exhaustive tests for deducing void return types

This revision was automatically updated to reflect the committed changes. Closed by commit rG7adb85884b35: [clang] [NFC] More exhaustive tests for deducing void return types (authored by arthur.j.odwyer). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

Quuxplusone added inline comments. Comment at: clang/test/SemaCXX/deduced-return-type-cxx14.cpp:116 -auto _ret_2() {} // expected-error {{cannot deduce return type 'auto &' for function with no return statements}} +auto _ret_2() {} // expected-error {{cannot form a reference

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

Quuxplusone added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:3766 +Expr *, const AutoType *AT, +bool HasReturnStmt) { + // If this is the conversion function for a lambda, we

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

Quuxplusone accepted this revision. Quuxplusone added a comment. This revision is now accepted and ready to land. LGTM FWIW. You might want to wait for someone more authoritative to take a look; but it's also only adding test coverage, so it seems pretty uncontroversial, I would think.

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

Quuxplusone updated this revision to Diff 409359. Quuxplusone added a comment. Apply @rsmith's suggested approach. > I don't see any obvious way that this patch could be responsible for that > failure, unless it's something like a pre-existing use of uninitialized > memory or a use-after-free

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

Quuxplusone added a comment. Excellent! Comment at: clang/test/CodeGenCXX/nrvo.cpp:622 + if (b) +return x; + else Similar to what you did in `test5`, I think it'd be helpful to comment //on the return line itself// whether NRVO is

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

Quuxplusone added a comment. I'll update with the `CodeSynthesisContext` approach in a little bit. But meanwhile there's a problem with both the current patch //and// (I think even more) with the new approach. @rsmith any help?: When I try my latest patch with the libc++ tests, with

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

Quuxplusone added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:3805 +if (DAR != DAR_Succeeded) { + if (OrigResultType.getBeginLoc().isValid()) +Diag(OrigResultType.getBeginLoc(), diag::note_deducing_return_type_for) > I am curious

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

Quuxplusone updated this revision to Diff 409095. Quuxplusone added a comment. Rebase; adjust more expected output in tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119778/new/ https://reviews.llvm.org/D119778 Files:

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

Quuxplusone updated this revision to Diff 409090. Quuxplusone added a comment. Rebase; fix the clang-format nit; reinsert a missing `FD->setInvalidDecl()` that seemed to be causing crashes in Modules code under libcxx/test/ but otherwise doesn't seem to be tested in clang/test/ :( Repository:

[PATCH] D119893: [clang-format] Fixed handling of requires clauses followed by attributes

Quuxplusone added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:3018-3019 do { +bool LambdaThisTimeAllowed = LambdaNextTimeAllowed; +LambdaNextTimeAllowed = false; + Nit: For this pattern, consider `bool LambdaThisTimeAllowed =

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

Quuxplusone added a reviewer: mizvekov. Quuxplusone added a comment. The code is above my pay grade, but FWIW, I super support the intent of this patch! Let's get p2025 support into Clang! :) > The collection of common cases contains 20 examples: §4.1. Examples. Here is > the current status of

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

Quuxplusone created this revision. Quuxplusone added reviewers: sammccall, rsmith, dblaikie, majnemer, mizvekov, ChuanqiXu. Quuxplusone added a project: clang. Quuxplusone requested review of this revision. Herald added a subscriber: cfe-commits. Emit the note when we fail to deduce a return

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

Quuxplusone updated this revision to Diff 408594. Quuxplusone marked an inline comment as done and an inline comment as not done. Quuxplusone added a comment. Address review comments; add exhaustive tests as a parent revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119184/new/

[PATCH] D119772: [clang] [NFC] More exhaustive tests for deducing void return types

Quuxplusone created this revision. Quuxplusone added reviewers: sammccall, rsmith, dblaikie, mizvekov. Quuxplusone added a project: clang. Quuxplusone requested review of this revision. Herald added a subscriber: cfe-commits. This is a preliminary ahead of D119184

[PATCH] D119094: [clang] Don't emit redundant warnings for 'return;'

This revision was automatically updated to reflect the committed changes. Closed by commit rG3c8d2aa87c17: [clang] Dont emit redundant warnings for return; (authored by arthur.j.odwyer). Changed prior to commit: https://reviews.llvm.org/D119094?vs=406838=408434#toc Repository: rG LLVM

[PATCH] D119670: [clang] Warn on unqualified calls to std::move and std::forward

Quuxplusone added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:6437 + NamedDecl *D = dyn_cast_or_null(Call->getCalleeDecl()); + if (!D || !D->isInStdNamespace()) +return; cor3ntin wrote: > erichkeane wrote: > > Quuxplusone wrote: > > >

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

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

[PATCH] D119670: [clang] Warn on unqualified calls to std::move and std::forward

Quuxplusone added a comment. Tangential data point: I hacked up this check to find //all// unqualified `std` functions and ran it on libc++'s tests and also on some of my own codebase (including re2, asio, protobuf). I'm fixing the issues it turned up in libc++'s tests. In the other codebases,

[PATCH] D119670: [clang] Warn on unqualified calls to std::move and std::forward

Quuxplusone added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4812 +def warn_unqualified_call_to_std_function : Warning< + "unqualified call to %0">, InGroup>; + This should probably be named `-Wunqualified-std-move` resp.

[PATCH] D119138: [clang-format] Further improve support for requires expressions

Quuxplusone added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2798-2802 +// This one is really tricky, since most tokens doesn't have a type yet. +// The & or && can be binary operators, then we have a requires +// expression. But they also

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

Quuxplusone marked an inline comment as not done. Quuxplusone added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:3816 +Context.getTrivialTypeSourceInfo(Context.VoidTy, ReturnLoc), ReturnLoc); +Expr *Dummy = R.get(); +DeduceAutoResult DAR =

[PATCH] D119351: Update all LLVM documentation mentioning runtimes in LLVM_ENABLE_PROJECTS

Quuxplusone added inline comments. Comment at: bolt/docs/OptimizingClang.md:228-236 $ CPATH=${TOPLEV}/stage1/install/bin/ -$ cmake -G Ninja ${TOPLEV}/llvm-project/llvm -DLLVM_TARGETS_TO_BUILD=X86 \ +$ cmake -G Ninja -S ${TOPLEV}/llvm-project/llvm -B ${TOPLEV}/stage2-prof-gen \

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

Quuxplusone updated this revision to Diff 407284. Quuxplusone added a comment. Rebase; clang-format. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119184/new/ https://reviews.llvm.org/D119184 Files: clang/include/clang/Sema/Sema.h

[PATCH] D119351: Update all LLVM documentation mentioning runtimes in LLVM_ENABLE_PROJECTS

Quuxplusone added inline comments. Comment at: bolt/docs/OptimizingClang.md:228-236 $ CPATH=${TOPLEV}/stage1/install/bin/ -$ cmake -G Ninja ${TOPLEV}/llvm-project/llvm -DLLVM_TARGETS_TO_BUILD=X86 \ +$ cmake -G Ninja -S ${TOPLEV}/llvm-project/llvm -B ${TOPLEV}/stage2-prof-gen \

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

Quuxplusone updated this revision to Diff 406847. Quuxplusone added a comment. Update one additional test where the expected output has changed: auto& f() { } now produces "can't make a reference to void" instead of "can't deduce `auto&` from no return statements." I think this is a mild

[PATCH] D119094: [clang] Don't emit redundant warnings for 'return;'

Quuxplusone updated this revision to Diff 406838. Quuxplusone added a comment. Oops. I'd put `[clang] [test] Fix an apparent typo in SemaCXX/consteval-return-void.cpp` in a separate commit for hygiene purposes, and forgot to include that commit in this diff. Updated. Repository: rG LLVM

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

Quuxplusone created this revision. Quuxplusone added reviewers: sammccall, saar.raz, rsmith, mizvekov, majnemer, riccibruno. Quuxplusone added a project: clang. Quuxplusone requested review of this revision. Herald added a subscriber: cfe-commits. Fixes

[PATCH] D119094: [clang] Don't emit redundant warnings for 'return;'

Quuxplusone updated this revision to Diff 406585. Quuxplusone added a comment. This revision is now accepted and ready to land. Fix the two failing test cases (in one case by fixing what seems to have been a typo in the test). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119094/new/

[PATCH] D119094: [clang] Don't emit redundant warnings for 'return;'

Quuxplusone added a comment. In D119094#3301403 , @sammccall wrote: > In D119094#3301297 , @Quuxplusone > wrote: > >> Unfortunately some existing tests fail: >>

[PATCH] D119094: [clang] Don't emit redundant warnings for 'return;'

Quuxplusone planned changes to this revision. Quuxplusone added a comment. Unfortunately some existing tests fail: https://reviews.llvm.org/harbormaster/unit/view/2282838/ I haven't yet figured out why consteval functions are considered to have `FD->isInvalidDecl()`. There's also an Objective-C

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

Quuxplusone added inline comments. Comment at: clang/include/clang/Format/Format.h:3371 +/// If ``true``, put space between requires keyword in a requires clause and +/// opening parentheses, if there is one. +/// \code HazardyKnusperkeks wrote: >

[PATCH] D119094: [clang] Don't emit redundant warnings for 'return;'

Quuxplusone created this revision. Quuxplusone added reviewers: rsmith, sammccall, mizvekov, majnemer, riccibruno, clang. Quuxplusone added a project: clang. Quuxplusone requested review of this revision. Herald added a subscriber: cfe-commits. ...when the function declaration's return type is

[PATCH] D117603: [clang] Don't typo-fix an expression in a SFINAE context

Quuxplusone added a comment. In D117603#3292657 , @var-const wrote: > @Quuxplusone Now that it's landed, do we still need `__workaround_52970` in > libc++? (see >

[PATCH] D117603: [clang] Don't typo-fix an expression in a SFINAE context

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc0185ffaec3c: [clang] Dont typo-fix an expression in a SFINAE context. (authored by arthur.j.odwyer). Repository: rG LLVM Github Monorepo

[PATCH] D118552: [clang] [concepts] Correctly(?) handle placeholder types in ExprRequirements.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf6ce45670789: [clang] Correctly(?) handle placeholder types in ExprRequirements. (authored by arthur.j.odwyer). Repository: rG LLVM Github

[PATCH] D114903: [clang] Support array placement new in constant expressions

Quuxplusone added a comment. I have no special knowledge of this. Seems like "it can't be that easy," but I don't know. Comment at: clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp:138 } static_assert(call_std_construct_at()); I would think you ought to

[PATCH] D118552: [clang] [concepts] Correctly(?) handle placeholder types in ExprRequirements.

Quuxplusone updated this revision to Diff 404624. Quuxplusone added a comment. Update another single-line `if` too. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118552/new/ https://reviews.llvm.org/D118552 Files: clang/lib/AST/ASTContext.cpp clang/lib/Sema/SemaOverload.cpp

[PATCH] D118552: [clang] [concepts] Correctly(?) handle placeholder types in ExprRequirements.

Quuxplusone updated this revision to Diff 404621. Quuxplusone added a comment. Update assert style. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118552/new/ https://reviews.llvm.org/D118552 Files: clang/lib/AST/ASTContext.cpp clang/lib/Sema/SemaOverload.cpp

[PATCH] D118552: [clang] [concepts] Correctly(?) handle placeholder types in ExprRequirements.

Quuxplusone added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:3373 ASTContext::getLValueReferenceType(QualType T, bool SpelledAsLValue) const { - assert(getCanonicalType(T) != OverloadTy && - "Unresolved overloaded function type"); + if

[PATCH] D118552: [clang] [concepts] Correctly(?) handle placeholder types in ExprRequirements.

Quuxplusone added a comment. Gentle ping! (@ChuanqiXu @urnathan perhaps?) I'm hoping to land D118552 followed by a re-land of D117603 , this week and //ideally// tomorrow. :) CHANGES SINCE LAST ACTION

[PATCH] D117603: [clang] Don't typo-fix an expression in a SFINAE context

Quuxplusone updated this revision to Diff 404381. Quuxplusone added a comment. Poke CI (clang-format failed). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117603/new/ https://reviews.llvm.org/D117603 Files: clang/lib/Sema/Sema.cpp clang/lib/Sema/SemaExprMember.cpp

[PATCH] D118552: [clang] [concepts] Correctly(?) handle placeholder types in ExprRequirements.

Quuxplusone updated this revision to Diff 404380. Quuxplusone added a comment. Poke CI (clang-format failed). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118552/new/ https://reviews.llvm.org/D118552 Files: clang/lib/AST/ASTContext.cpp clang/lib/Sema/SemaOverload.cpp

[PATCH] D118552: [clang] [concepts] Correctly(?) handle placeholder types in ExprRequirements.

Quuxplusone added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:3375 +assert(T->isSpecificPlaceholderType(BuiltinType::UnknownAny) && "Unresolved placeholder type"); + } Quuxplusone wrote: > Btw, I strongly suspect that the presence of

[PATCH] D117603: [clang] Don't typo-fix an expression in a SFINAE context

Quuxplusone updated this revision to Diff 404343. Quuxplusone added a comment. Move the regression test to clang/test/SemaTemplate/, alongside the one for D118552 . Also, make it test the error message wording. CHANGES SINCE LAST ACTION

[PATCH] D117603: [clang] Don't typo-fix an expression in a SFINAE context

Quuxplusone updated this revision to Diff 404341. Quuxplusone added a comment. It seems like we need `tryToRecoverWithCall` to always identify a diagnostic explaining the problem, so just short-circuit-returning `ExprError()` with no diagnostic is not acceptable. Let's try this version instead.

[PATCH] D118552: [clang] [concepts] Correctly(?) handle placeholder types in ExprRequirements.

Quuxplusone added a subscriber: rjmccall. Quuxplusone added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:3375 +assert(T->isSpecificPlaceholderType(BuiltinType::UnknownAny) && "Unresolved placeholder type"); + } Btw, I strongly suspect that

[PATCH] D118552: [clang] [concepts] Correctly(?) handle placeholder types in ExprRequirements.

Quuxplusone created this revision. Quuxplusone added reviewers: ChuanqiXu, saar.raz, rsmith. Quuxplusone added a project: clang. Quuxplusone requested review of this revision. Herald added a subscriber: cfe-commits. PR52905 was originally papered over in a different way, but I believe this is

[PATCH] D118518: [clang][NFC] Change some ->getType()->isPlaceholderType() to just ->hasPlaceholderType()

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG424400da2db8: [clang][NFC] Change some -getType()-isPlaceholderType() to just… (authored by arthur.j.odwyer). Changed prior to commit:

[PATCH] D118518: [clang][NFC] Change some ->getType()->isPlaceholderType() to just ->hasPlaceholderType()

Quuxplusone created this revision. Quuxplusone added reviewers: ChuanqiXu, urnathan, efriedma, kazu. Quuxplusone added a project: clang. Quuxplusone requested review of this revision. Herald added a subscriber: cfe-commits. While trying to figure out

[PATCH] D117603: [clang] Don't typo-fix an expression in a SFINAE context

Quuxplusone reopened this revision. Quuxplusone added a comment. This revision is now accepted and ready to land. I'm not sure if this caused https://lab.llvm.org/buildbot/#/builders/60/builds/6350 https://lab.llvm.org/buildbot/#/builders/119/builds/7433 but I'm acting as if it did. Anyone see

[PATCH] D117603: [clang] Don't typo-fix an expression in a SFINAE context

This revision was automatically updated to reflect the committed changes. Closed by commit rG9be5f4d5afd9: [clang] Dont typo-fix an expression in a SFINAE context. (authored by arthur.j.odwyer). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

Quuxplusone accepted this revision. Quuxplusone added a comment. This revision is now accepted and ready to land. LGTM now, modulo my suggested edits (and the necessary corresponding edits in the test case). I don't think I'm really qualified to accept, but as nobody else is looking, and my

[PATCH] D117603: [clang] Don't typo-fix an expression in a SFINAE context

Quuxplusone added reviewers: ChuanqiXu, urnathan, hokein, dblaikie. Quuxplusone added a comment. Add some more reviewers who've touched SemaCXX recently. Ping! If no objections are forthcoming, I'd like to land this on Friday morning. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D92956: Fix range-loop-analysis checks for trivial copyability

Quuxplusone added a comment. What ever happened to this patch? @fanfuqiang, are you still around to rebase this? Do you need someone to commit it for you? (Not that I'm saying I think it's perfect as is — I haven't looked and don't know the code that well — but just wondering what kept this

[PATCH] D117603: [clang] Don't typo-fix an expression in a SFINAE context

Quuxplusone updated this revision to Diff 401652. Quuxplusone added a comment. Clang-formatted. @mizvekov or @rsmith: ping? I'd like to get this in before Feb 1, which is the release/14.x branch date AFAIK; because it affects workarounds in libc++ and thus makes a difference whether we will be

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

Quuxplusone added inline comments. Comment at: clang/test/SemaCXX/attr-trivial-abi.cpp:9-11 +// On Windows, trivial relocatability depends only on a trivial copy constructor existing. +// In this case, it is implicitly deleted. Similar concerns apply to later tests.

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

Quuxplusone added a comment. In D114732#3253142 , @devin.jeanpierre wrote: > OK, while I'm struggling to set up a new Windows machine so I can make sure > this works on Windows... @Quuxplusone, after this is merged, do you want to > rebase D67524

[PATCH] D117603: [clang] Don't typo-fix an expression in a SFINAE context

Quuxplusone created this revision. Quuxplusone added reviewers: ldionne, rsmith, leonardchan, mizvekov. Quuxplusone added a project: clang. Quuxplusone requested review of this revision. Herald added a subscriber: cfe-commits. If this is a SFINAE context, then continuing to look up names (in

[PATCH] D117535: [clang-tidy] Force LF newlines when writing files

Quuxplusone accepted this revision. Quuxplusone added a comment. This revision is now accepted and ready to land. LGTM FWIW. I might even wonder why this code is using `io.open` instead of just plain `open`. (I didn't know `io.open` was a thing; StackOverflow tells me that it was something in

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

[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

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

[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

[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] 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),

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

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

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

[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

[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

[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

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

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

[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 =(int) { return *this; } +}; fwolff wrote: > whisperity wrote: > > This is a

[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

[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

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

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

  1   2   3   4   5   6   7   >