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

[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

[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

[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

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

[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

[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

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

[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

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

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

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

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

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

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

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

[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

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

[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

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

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

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

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

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

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

[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

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

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

[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

[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

[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

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

[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

[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

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

[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

[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

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

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

<    1   2   3   4   5