[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

bruno added a comment. Thanks Duncan. > Given that header maps are somewhat Apple-specific Some non-obvious background: It began Apple specific, but Meta uses them at scale as well, pretty important for us to get this right. > and unit test coverage is a bit lacking for these sorts of interact

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

bruno added reviewers: Bigcheese, ChuanqiXu, jansvoboda11. bruno added a comment. Adding code owners and more relevant folks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103930/new/ https://reviews.llvm.org/D103930 ___

[PATCH] D149850: [Clang][Modules] Support `requires cplusplus20` in a modulemap

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149850/new/ https://reviews.llvm.org/D149850 ___ c

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

bruno added a comment. > Looks like this patch causes a number of issues for us, I will work with > @jansvoboda11 to see if there's some way to resolve them. If you can share a reproducer I'm pretty sure @ivanmurashko can help make it work for y'all too. Thanks! Repository: rG LLVM Github

[PATCH] D146758: Fix codegen for coroutine with function-try-block

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM! @ChuanqiXu wdyt? Comment at: clang/test/CodeGenCoroutines/coro-function-try-block.cpp:1 +// RUN: %clang_cc1 -std=c++20 -triple=x86_64-- -emit-llvm -fcxx-exceptions \ +//

[PATCH] D136515: [builtins] Add __builtin_assume_separate_storage.

This revision was automatically updated to reflect the committed changes. Closed by commit rG07ef7b1ff21e: [Builtins] Add __builtin_assume_separate_storage (authored by bruno). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136515/new/ https://review

[PATCH] D145641: [Coroutines] Implement fix for cwg2563 issue and enable RVO under certain conditions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfa0d4e1f12a3: [Coroutines] Implement fix for cwg2563 issue and enable RVO under certain… (authored by bruno). Repository: rG LLVM Github Monorepo

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

This revision was automatically updated to reflect the committed changes. Closed by commit rG43f5085fa80f: [Coroutines] Fix premature conversion of return object (authored by bruno). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145639/new/ https://

[PATCH] D145641: [Coroutines] Implement fix for cwg2563 issue and enable RVO under certain conditions

bruno updated this revision to Diff 507224. bruno added a comment. Apply last round of reviews before landing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145641/new/ https://reviews.llvm.org/D145641 Files: clang/docs/ReleaseNotes.rst clang/i

[PATCH] D145641: [Coroutines] Implement fix for cwg2563 issue and enable RVO under certain conditions

bruno updated this revision to Diff 507087. bruno added a comment. Add release notes entry. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145641/new/ https://reviews.llvm.org/D145641 Files: clang/docs/ReleaseNotes.rst clang/include/clang/AST/S

[PATCH] D145641: [Coroutines] Implement fix for cwg2563 issue and enable RVO under certain conditions

bruno updated this revision to Diff 507085. bruno added a comment. Address last round of review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145641/new/ https://reviews.llvm.org/D145641 Files: clang/include/clang/AST/StmtCXX.h clang/lib/CodeG

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

bruno updated this revision to Diff 507084. bruno added a comment. Put dependency in place for D145641 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145639/new/ https://reviews.llvm.org/D145639 Files: clang/inc

[PATCH] D136515: [builtins] Add __builtin_assume_separate_storage.

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/docs/LanguageExtensions.rst:2393 +particular object (so for example, it's never correct to call this function +passing the addresses of fields in the sa

[PATCH] D146187: [docs] Update the status for coroutines

bruno added a comment. > ... I think in general we want feature testing macros to be treated as our > signal to the user that we think something is complete, not that we think > something is in progress but pretty usable. +1 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146187/new/ h

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

bruno added a comment. > @bruno, @ChuanqiXu please let us know if you have any objections, otherwise > we will land the revert in ~2 hours. No sweat, I didn't see this in time. Thanks for the reduced testcase. > What's the resolution here? Can we revert this and continue the discussions > inde

[PATCH] D145641: [Coroutines] Implement fix for cwg2563 issue and enable RVO under certain conditions

bruno updated this revision to Diff 505588. bruno marked 2 inline comments as done. bruno added a comment. Update after reviewer comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145641/new/ https://reviews.llvm.org/D145641 Files: clang/inc

[PATCH] D145641: [Coroutines] Implement fix for cwg2563 issue and enable RVO under certain conditions

bruno marked 3 inline comments as done. bruno added inline comments. Comment at: clang/lib/CodeGen/CGCoroutine.cpp:483-499 +// The call to get_­return_­object is sequenced before the call to +// initial_­suspend and is invoked at most once, but there are caveats +// r

[PATCH] D136515: [builtins] Add __builtin_assume_separate_storage.

bruno added inline comments. Comment at: clang/docs/LanguageExtensions.rst:2393 +particular object (so for example, it's never correct to call this function +passing the addresses of fields in the same struct, elements of the same array, +etc.). Not necessarily a

[PATCH] D146156: [clang][Lexer] Fix crash/assert clang::HeaderSearch::search_dir_nth

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Good catch, thanks! LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146156/new/ https://reviews.llvm.org/D146156 _

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG54225c457a33: [Coroutines] Fix premature conversion of return object (authored by bruno). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

bruno added a comment. Test failures are unrelated, thanks for the review @ChuanqiXu Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145639/new/ https://reviews.llvm.org/D145639 ___ cfe-commits mailing lis

[PATCH] D145641: [Coroutines] Implement fix for cwg2563 issue and enable RVO under certain conditions

bruno created this revision. bruno added reviewers: ChuanqiXu, clang-language-wg. Herald added subscribers: hoy, modimo, wenlei. Herald added a project: All. bruno requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. - The cwg2563 issue is fixed

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

bruno created this revision. bruno added a reviewer: ChuanqiXu. Herald added subscribers: hoy, modimo, wenlei. Herald added a project: All. bruno requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fix https://github.com/llvm/llvm-project/issue

[PATCH] D144680: [Coroutines] Avoid creating conditional cleanup markers in suspend block

bruno added a comment. Thanks for working on this Wei! Can you please add a testcase? Comment at: clang/lib/CodeGen/CodeGenFunction.h:336 std::unique_ptr Data; +bool InSuspendBlock = false; CGCoroInfo(); Should this live inside CGCoroData instead

[PATCH] D142077: [Clang][SemaCXX][Coroutines] Fix misleading diagnostics with -Wunsequenced

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0b8daee028a8: [Clang][SemaCXX][Coroutines] Fix misleading diagnostics with -Wunsequenced (authored by bruno). Repository: rG LLVM Github Monorepo

[PATCH] D142077: [Clang][SemaCXX][Coroutines] Fix misleading diagnostics with -Wunsequenced

bruno added a comment. Only test failing is `ClangScanDeps/modules-full.cpp` on Windoes, which is unrelated to this change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142077/new/ https://reviews.llvm.org/D142077 ___ cfe-commits mailing li

[PATCH] D142077: [Clang][SemaCXX][Coroutines] Fix misleading diagnostics with -Wunsequenced

bruno updated this revision to Diff 494740. bruno added a comment. Update checks that rely on `coroutine_traits` in `std-coroutine.h` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142077/new/ https://reviews.llvm.org/D142077 Files: clang/lib/Sema/SemaChecking.cpp clang/test/SemaCXX

[PATCH] D142077: [Clang][SemaCXX][Coroutines] Fix misleading diagnostics with -Wunsequenced

bruno added a comment. Thanks for the review, will push once all tests pass! Comment at: clang/lib/Sema/SemaChecking.cpp:15186 + if (ChildExpr == CSE->getOperand()) +// Do not recurse over a CoroutineSuspendExpr's operand. +// The operand is also a subexpre

[PATCH] D142077: [Clang][SemaCXX][Coroutines] Fix misleading diagnostics with -Wunsequenced

bruno updated this revision to Diff 494707. bruno added a comment. Update patch to reuse `std-coroutine.h` and add a few more other bits there. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142077/new/ https://reviews.llvm.org/D142077 Files: clang/lib/Sema/SemaChecking.cpp clang/te

[PATCH] D142077: [Clang][SemaCXX][Coroutines] Fix misleading diagnostics with -Wunsequenced

bruno added a comment. Test failure is unrelated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142077/new/ https://reviews.llvm.org/D142077 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D142077: [Clang][SemaCXX][Coroutines] Fix misleading diagnostics with -Wunsequenced

bruno created this revision. bruno added reviewers: ChuanqiXu, nridge, sammccall. Herald added subscribers: hoy, modimo, wenlei. Herald added a project: All. bruno requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. D115187

[PATCH] D124286: [modules] Allow parsing a duplicate Obj-C interface if a previous one comes from a hidden [sub]module.

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Nice new testcase snippets, LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124286/new/ https://reviews.llvm.org/D124286 _

[PATCH] D136515: [builtins] Add __builtin_assume_separate_storage.

bruno added a comment. Additionally, this likely deserves an entry in `./clang/docs/ReleaseNotes.rst`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136515/new/ https://reviews.llvm.org/D136515 ___ cfe-c

[PATCH] D136515: [builtins] Add __builtin_assume_separate_storage.

bruno requested changes to this revision. bruno added a comment. This revision now requires changes to proceed. Nice, thanks for adding the builtin layer. Comment at: clang/lib/Sema/SemaChecking.cpp:7808 +/// Handle __builtin_assume_separate_storage. For now this is a no-op, bu

[PATCH] D137609: [C++20] [Modules] Remove unmaintained header modules

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Also not using this at Meta, LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137609/new/ https://reviews.llvm.org/D137609 ___ cfe-commit

[PATCH] D130326: [ODRHash] Hash `ObjCPropertyDecl` and diagnose discovered mismatches.

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Very nice how this has improved since the version I worked on in the past, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130326/new/ http

[PATCH] D130327: [ODRHash] Detect duplicate `ObjCProtocolDecl` ODR mismatches during parsing.

bruno accepted this revision. bruno added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130327/new/ https://reviews.llvm.org/D130327 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D137179: [clang][Lex] Header search case insensitivity

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM with minor nitpick: can you reflect in the title that this is related to header maps? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137179/ne

[PATCH] D136190: [clang][Sema] remove redundant isTypeValid

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Nice cleanup, before landing please capitalize `remove` in the title and also add `[NFC]` to it! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136

[PATCH] D136633: [clang][AST] avoid unnecessary FunctionProtoTypes.FindNodeOrInsertPos call

bruno added a comment. Nice catch, thanks for improving this. Any chance you've got any small enough example that trigger this that could be transformed into a testcase? It would be nice to have a unittest for this, see others in `clang/unittests/AST`. Nitpick: please capitalize `avoid` in the

[PATCH] D135801: [clang][Lexer] Speedup HeaderSearch when there are many HeaderMaps

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Thanks for the detailed explanations, LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135801/new/ https://reviews.llvm.org/D135801 __

[PATCH] D130324: [ODRHash] Hash `ObjCProtocolDecl` and diagnose discovered mismatches.

bruno accepted this revision. bruno added a comment. Great to see both this and D130325 landing, LGTM too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130324/new/ https://reviews.llvm.org/D130324 _

[PATCH] D135801: [clang][Lexer] Speedup HeaderSearch when there are many HeaderMaps

bruno added a comment. Thanks for working on this Troy, nice speed up. A bit more context for reviewers: we're probably unique on how we use hmaps, we have tons of them in a single compiler invocation. The other users of hmaps I've seen don't use more than a handful. This means that general use

[PATCH] D135370: Narrow inline namespace filtration for unqualified friend declarations

bruno added a comment. > LGTM! Should we also add a release note or do we think this isn't enough of a > compile-time performance improvement to warrant that? Good point @aaron.ballman, given we have only observed in a few corner case large TUs I'd say it's probably not that normally perceiva

[PATCH] D130325: [ODRHash] Hash `ObjCMethodDecl` and diagnose discovered mismatches.

bruno accepted this revision. bruno added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130325/new/ https://reviews.llvm.org/D130325 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D135370: Narrow inline namespace filtration for unqualified friend declarations

bruno added subscribers: aaron.ballman, bruno. bruno added a reviewer: aaron.ballman. bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Hi Troy, thanks for working on this, nice compile time perf savings. Can you update the diff to contain more

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

bruno added a comment. > I'm not sure how to deal with missing `env -u`. > > - We could do `env CLANG_MODULE_CACHE_PATH=` and change the compiler's > interpretation of empty string for this variable. I'm not sure if the current > behaviour (there will be no module cache in the cc1 at all) is int

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Awesome! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133622/new/ https://reviews.llvm.org/D133622 _

[PATCH] D133617: [Clang][ScanDeps] Change multiple-commands.c test to use -fmodules-cache-path on implicit builds

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf4a13c9c0a04: [Clang][ScanDeps] Change multiple-commands.c test to use -fmodules-cache-path… (authored by bruno). Changed prior to commit: https:/

[PATCH] D133617: [Clang][ScanDeps] Change multiple-commands.c test to use -fmodules-cache-path on implicit builds

bruno added a comment. Thanks for the fast review! Good suggestions, will apply and land. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133617/new/ https://reviews.llvm.org/D133617 ___ cfe-commits mailin

[PATCH] D133617: [Clang][ScanDeps] Change multiple-commands.c test to use -fmodules-cache-path on implicit builds

bruno created this revision. bruno added reviewers: benlangmuir, jansvoboda11, arphaman. Herald added subscribers: hoy, modimo, wenlei. Herald added a project: All. bruno requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The module cache esca

[PATCH] D127182: [Clang][Modules] Merge availability attributes on imported decls

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127182/new/ https://reviews.llvm.org/D127182 ___ c

[PATCH] D127338: [Clang][CoverageMapping] Fix compile time explosions by adjusting only appropriated skipped ranges

This revision was automatically updated to reflect the committed changes. Closed by commit rGe6a76a49356e: [Clang][CoverageMapping] Fix compile time explosions by adjusting only… (authored by bruno). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1273

[PATCH] D127338: [Clang][CoverageMapping] Fix compile time explosions by adjusting only appropriated skipped ranges

bruno updated this revision to Diff 435388. bruno added a comment. Empty lines handling should also make it skipped, fix the 3 failures. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127338/new/ https://reviews.llvm.org/D127338 Files: clang/lib/

[PATCH] D127338: [Clang][CoverageMapping] Fix compile time explosions by adjusting only appropriated skipped ranges

bruno added a comment. > Just noticed that 3 test cases failed. Please fix them before landing. Sure thing, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127338/new/ https://reviews.llvm.org/D127338 ___

[PATCH] D127338: [Clang][CoverageMapping] Fix compile time explosions by adjusting only appropriated skipped ranges

bruno created this revision. bruno added reviewers: vsk, zequanwu. Herald added subscribers: hoy, modimo, wenlei. Herald added a project: All. bruno requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. D83592 ad

[PATCH] D124751: [HLSL] Support -E option for HLSL.

bruno added a comment. Hi, nice to see this getting in. Comments inline! Comment at: clang/lib/Sema/SemaDecl.cpp:11720 +if (!FD->hasAttr()) { + Diag(FD->getLocation(), diag::err_hlsl_missing_numthreads) << "Compute"; + FD->setInvalidDecl(); You ca

[PATCH] D126345: [Clang][CoverageMapping] Fix switch case counter compile time explosion

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGce54b22657f0: [Clang][CoverageMapping] Fix switch counter codegen compile time explosion (authored by bruno). Repository: rG LLVM Github Monorepo

[PATCH] D126345: [Clang][CoverageMapping] Fix switch case counter compile time explosion

bruno created this revision. bruno added reviewers: alanphipps, vsk, zequanwu. Herald added subscribers: hoy, modimo, wenlei, hiraditya. Herald added a project: All. bruno requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Af

[PATCH] D118311: [Clang][ModuleMap] Add conditional parsing via requires block declaration

bruno requested changes to this revision. bruno added a comment. This revision now requires changes to proceed. Herald added a project: All. > It would be nice to have some mechanism to notify developers that includes > are still performed regardless of requires I'd like to see a module remark f

[PATCH] D118005: [hmaptool] Fix string decoding for Python 3

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Herald added a project: All. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118005/new/ https://reviews.llvm.org/D118005

[PATCH] D118004: [hmaptool] Fix dumping

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Herald added a project: All. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118004/new/ https://reviews.llvm.org/D118004

[PATCH] D107944: [hmaptool] Port to python3

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

[PATCH] D112481: [Sema] fix nondeterminism in ASTContext::getDeducedTemplateSpecializationType

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. This is good to go, sorry for the delay. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112481/new/ https://reviews.llvm.org/D112481 _

[PATCH] D113523: Add toggling for -fnew-infallible/-fno-new-infallible

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113523/new/ https://reviews.llvm.org/D113523 ___ c

[PATCH] D112941: [clang] Add support for the new pointer authentication builtins.

bruno added inline comments. Comment at: clang/include/clang/Driver/Options.td:2865-2872 +let Group = f_Group in { + let Flags = [CC1Option] in { +def fptrauth_intrinsics : Flag<["-"], "fptrauth-intrinsics">, + HelpText<"Enable pointer-authentication intrinsics">; + }

[PATCH] D113523: Add toggling for -fnew-infallible/-fno-new-infallible

bruno added a comment. Hi @modimo, can you also add a driver test to check (`clang/test/Driver/...`) that nothing is added to cc1 cmdline when `-fno-new-infallible` is used or take precendence? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113523/

[PATCH] D112941: [clang] Add support for the new pointer authentication builtins.

bruno added a comment. Thanks for working on upstreaming this @ab. Overall looks good to me, I see clang-format issues, are those legit? One more comment inline. Comment at: clang/include/clang/Driver/Options.td:2865-2872 +let Group = f_Group in { + let Flags = [CC1Option] in

[PATCH] D112481: [Sema] fix nondeterminism in ASTContext::getDeducedTemplateSpecializationType

bruno added a reviewer: bruno. bruno added a comment. Nice catch, thanks for working on this! Comment at: clang/lib/AST/ASTContext.cpp:5640 + DeducedTemplateSpecializationType::Profile( + ID, Template, DeducedType, IsDependent || Template.isDependent()); if (DeducedTem

[PATCH] D107944: [hmaptool] Port to python3

bruno added a comment. Overall looks great but there are some test failures, looks like you also need to update line 223? -- Traceback (most recent call last): File "/var/lib/buildkite-agent/builds/llvm-project/build/bin/hmaptool", line 296, in main() File "/var/lib/buildkite

[PATCH] D88327: [SemaTemplateInstantiate] Handle lack of TypeSourceInfo on special member functions in templated lambdas

This revision was automatically updated to reflect the committed changes. Closed by commit rGc9aaf34b8db8: [SemaCXX] Handle lack of TypeSourceInfo on special member functions in… (authored by bruno). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D71734: [Modules] Handle tag types and complain about bad merges in C/Objective-C mode

bruno added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:9585-9587 +assert(getContext().hasSameType(FirstField->getType(), +SecondField->getType())); + rsmith wrote: > bruno wrote: > > rsmith wrote: >

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

bruno added a comment. @dexonsmith and @jansvoboda11 thanks for the fast reply and the extra testing. Comment at: clang/lib/Lex/HeaderSearch.cpp:421 + auto FixupSearchPathAndFindUsableModule = + [&](auto file) -> Optional { if (SearchPath) { Can you

[PATCH] D102820: [Clang] Check for returns_nonnull when deciding to add allocation null checks

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. > Assuming I'm answering the correct question that the returns_nonnull is > preserved through a PCH, the answer is yes. Thanks! > As mentioned this isn't strictly needed and the BE can elide

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

bruno added subscribers: jansvoboda11, vsapsai, dexonsmith. bruno added a comment. Thanks for working on this, comments inline. @vsapsai @jansvoboda11 @dexonsmith any headermap related concerns on your side? Comment at: clang/lib/Lex/HeaderSearch.cpp:445 +return None;

[PATCH] D103722: [clang] NFC: test for undefined behaviour in RawComment::getFormattedText()

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Thanks for adding more tests! LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103722/new/ https://reviews.llvm.org/D103722 __

[PATCH] D103142: [clang][clangd] Use reverse header map lookup in suggestPathToFileForDiagnostics

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103142/new/ https://reviews.llvm.org/D103142 ___

[PATCH] D103142: [clang][clangd] Use reverse header map lookup in suggestPathToFileForDiagnostics

bruno added inline comments. Comment at: clang/lib/Lex/HeaderMap.cpp:265 + } + return ReverseMap.lookup(DestPath); +} How about something along the lines of: ``` ... StringRef Key; for (unsigned i = 0; i != NumBuckets; ++i) { HMapBucket B = getBucket(i);

[PATCH] D98995: [CGAtomic] Lift stronger requirements on cmpxch and add support for acquire failure mode

bruno added a comment. @yln sorry, I didn't get notifications for this somehow. This got fixed as part of the more general support in https://github.com/llvm/llvm-project/commit/819e0d105e84c6081cfcfa0e38fd257b6124553a Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D103142: [clang][clangd] Use reverse header map lookup in suggestPathToFileForDiagnostics

bruno requested changes to this revision. bruno added a comment. This revision now requires changes to proceed. Overall looks good, few remaining nitpicks. Comment at: clang/lib/Lex/HeaderMap.cpp:245 +StringRef HeaderMapImpl::reverseLookupFilename(StringRef DestPath) const { +

[PATCH] D103229: [clang] NFC: split HeaderMapTest to have re-usable header map implementation for testing

bruno accepted this revision. bruno added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103229/new/ https://reviews.llvm.org/D103229 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D103142: [clang][clangd] Use reverse header map lookup in suggestPathToFileForDiagnostics

bruno added subscribers: JDevlieghere, vsapsai. bruno added inline comments. Comment at: clang/lib/Lex/HeaderSearch.cpp:1851 - - return path::convert_to_slash(File.drop_front(BestPrefixLength)); + // Try resolving resulting filaname via reverse search in header maps, + // ke

[PATCH] D100276: [clang] p1099 3/5: using Enum::member

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM. An additional review here would be nice though. > good, it appears the patch bot fail is fixed -- it seems unhappy with > non-tree-like dependency graphs, and tries to apply each reachable

[PATCH] D102241: [clang] p1099 4/5: using enum EnumTag

bruno accepted this revision. bruno added a reviewer: rsmith. bruno added a comment. This revision is now accepted and ready to land. Thanks for adding the tests. LGTM after some remaining nitpick. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:11632 + // As with enum-decls, we ig

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

bruno added a comment. Overall looks good, sounds like it's failing tests though? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102782/new/ https://reviews.llvm.org/D102782 ___ cfe-commits mailing list c

[PATCH] D101777: [clang] p1099 1/5: [NFC] Break out BaseUsingDecl from UsingDecl

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

[PATCH] D102820: [Clang] Check for returns_nonnull when deciding to add allocation null checks

bruno added a comment. Sounds reasonable to me! Can you double check whether this attribute gets correctly serialized/deserialized in face of `CXXNewExpr`? An example of how to test that would be in `clang/test/PCH/cxx-method.cpp`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102820/n

[PATCH] D100276: [clang] p1099 3/5: using Enum::member

bruno added a comment. Overall looks good, one more comment below. Comment at: clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p3.cpp:20 class C { +public: int g(); bruno wrote: > > The change to clang/test/CXX/dcl.dcl/basic.namespace/namespace.udecl

[PATCH] D100914: [clang] Revert "Re-fix _lrotl/_lrotr to always take Long, no matter the platform."

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. This is the same as https://reviews.llvm.org/D80172, right? LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100914/new/ https://reviews.llvm.o

[PATCH] D100276: [clang] p1099 using enum part 1

bruno added a comment. Hi Nathan, thanks for implementing this. Besides formatting nitpicks, few other comments/questions: - Update needed in cxx_status.html - Should we support this as an extension for earlier C++ versions? This is a very handy feature. In clang terms, `warn_cxx17_compat_cons

[PATCH] D100129: Tiny format fix

bruno accepted this revision. bruno added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100129/new/ https://reviews.llvm.org/D100129 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D98995: [CGAtomic] Lift stronger requirements on cmpxch and add support for acquire failure mode

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG431e3138a1f3: [CGAtomic] Lift stronger requirements on cmpxch and support acquire failure mode (authored by bruno). Herald added a project: clang. R

[PATCH] D98995: [CGAtomic] Lift stronger requirements on cmpxch and add support for acquire failure mode

bruno updated this revision to Diff 332823. bruno edited the summary of this revision. bruno added a comment. @jfb sure! Also fix test failures. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98995/new/ https://reviews.llvm.org/D98995 Files: clang/lib/CodeGen/CGAtomic.cpp clang/tes

[PATCH] D98638: [RFC][Coroutine] Force stack allocation after await_suspend() call

bruno added a comment. Hi Xun, great to see more improvements in this area. Comment at: clang/lib/CodeGen/CGCoroutine.cpp:221 CGF.EmitBlock(RealSuspendBlock); + } else if (ForcestackStart) { +Builder.CreateCall( ChuanqiXu wrote: > lxfind wrote: > > Ch

[PATCH] D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM, it would be great if someone else stamps this too, in case I missed something. Comment at: clang/include/clang/Driver/Options.td:164 +def m_m68k_Features_Group: OptionGr

[PATCH] D88393: [cfe][M68k] (Patch 7/8) Basic Clang support

bruno added a reviewer: bruno. bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88393/new/ https://reviews.llvm.org/D88393 ___ cfe-commits

[PATCH] D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k

bruno added a reviewer: bruno. bruno added inline comments. Comment at: clang/include/clang/Driver/Options.td:164 +def m_m68k_Features_Group: OptionGroup<"">, + Group, DocName<"M68k">; def m_mips_Features_Group : OptionGroup<"">, Look

[PATCH] D88394: [Driver][M68k] (Patch 8/8) Add driver support for M68k

bruno added inline comments. Comment at: clang/include/clang/Driver/Options.td:3125 +foreach i = {0-4} in + def m680#i#0 : Flag<["-"], "m680"#i#"0">, Group; rengolin wrote: > Same question as @RKSimon had below: Shouldn't this cover all models the > back-end

[PATCH] D88393: [cfe][M68k] (Patch 7/8) Basic Clang support

bruno added a comment. Looking forward to see m68k support (and hopefully sega genesis toolchain support someday)! Comment at: clang/lib/Basic/Targets/M68k.cpp:73 +void M68kTargetInfo::getTargetDefines(const LangOptions &Opts, +MacroBuil

[PATCH] D86853: [modules] Fix crash in call to `FunctionDecl::setPure()`

bruno added a comment. I forgot to follow up, but LGTM too. > @rsmith, @v.g.vassilev hey I stamped this patch assuming it looks ok. But > definitely shout at me if more feedback needs to be addressed. Happy to > follow up. FWIW, this has been working on our code base for sometime now. Reposi

  1   2   3   4   5   >