[PATCH] D87853: [SemaTemplate] Stop passing insertion position around during VarTemplate instantiation

bruno updated this revision to Diff 293351. bruno retitled this revision from "[Sema] Update specialization iterator after template argument deduction." to "[SemaTemplate] Stop passing insertion position around during VarTemplate instantiation". bruno edited the summary of this revision. bruno ad

[PATCH] D87853: [SemaTemplate] Stop passing insertion position around during VarTemplate instantiation

bruno updated this revision to Diff 293356. bruno added a comment. Add context CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87853/new/ https://reviews.llvm.org/D87853 Files: clang/include/clang/Sema/Sema.h clang/include/clang/Sema/Template.h clang/lib/Sema/SemaTemplate.cpp cla

[PATCH] D86895: [Modules] Add stats to measure performance of building and loading modules.

bruno accepted this revision. bruno added a comment. This looks pretty useful, thanks for adding this Volodymyr. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86895/new/ https://reviews.llvm.org/D86895 ___

[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled

bruno added a comment. Hi Andrew, thanks for improving this. I think this makes sense: dependents can choose to not use modules without having to trigger the build system to rebuild all dependencies. Can you add a simple testcase to prove the point of the change? Comment at:

[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled

bruno added inline comments. Comment at: clang/lib/Frontend/FrontendAction.cpp:814 +CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile( +*File, /*IsSystem*/ false); + else andrewjcg wrote: > bruno wrote: > > Is this clang-format

[PATCH] D82118: [clang][module] Improve incomplete-umbrella warning

bruno added a comment. > Yes the wrapper is definitely problematic. I'm checking how the diagnostic > itself is handled correctly and start from there when I have time. For now > would it be better to separate this into multiple patches and get the > diagnostic improvement in first? Yep, tha

[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled

bruno added inline comments. Comment at: clang/test/Modules/explicit-module-maps.cpp:4 + +// RUN: %clang_cc1 -fmodule-map-file=%S/Inputs/explicit-module-maps/invalid.modulemap -verify %s +// expected-no-diagnostics Minor suggestion here given the content of the

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

bruno added reviewers: rsmith, v.g.vassilev. bruno added inline comments. Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:870 + // definition (which is required for `setPure`). + const bool pure = Record.readInt(); FD->setHasInheritedPrototype(Record.readInt()); --

[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] D91011: [NFC, Refactor] Rename the (scoped) enum DeclaratorContext's enumerators to avoid redundancy

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

[PATCH] D90990: [Coroutine][Sema] Cleanup temporaries as early as possible

bruno added a comment. Very nice explanation, thanks for improving this! Can you also add a AST dump test? The idea is to test for the presence of `ExprWithCleanups`s. Something along the lines of `clang/test/AST/coroutine-source-location-crash.cpp` ignoring the serialization part should be go

[PATCH] D90990: [Coroutine][Sema] Cleanup temporaries as early as possible

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/D90990/new/ https://reviews.llvm.org/D90990 ___ cfe

[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

[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] 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] D87853: [SemaTemplate] Stop passing insertion position around during VarTemplate instantiation

bruno added a comment. Ping! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87853/new/ https://reviews.llvm.org/D87853 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87853: [SemaTemplate] Stop passing insertion position around during VarTemplate instantiation

bruno added a comment. Ping! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87853/new/ https://reviews.llvm.org/D87853 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87853: [SemaTemplate] Stop passing insertion position around during VarTemplate instantiation

This revision was automatically updated to reflect the committed changes. Closed by commit rGcffb0dd54d41: [SemaTemplate] Stop passing insertion position around during VarTemplate… (authored by bruno). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87

[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] 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] 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] 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] 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] 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] 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] 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] 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] 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 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] 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 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] 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] 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] 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] 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 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] 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] 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] 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] 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] 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 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 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] 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] 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] 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] 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] 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] 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 cfe-commits@lists.llvm.or

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

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

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] D74795: Make diagnostic reporting more robust in presence of corrupt PCH data.

bruno added a comment. Thanks for working on this. > I'm not sure how (or if it's possible) to add a test that would demonstrate > the before/after here. Happy to take advice! Yea, those are some times hard. Did you try to write a unittest that hits the assertion and trigger this case? Perhaps

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

bruno added a comment. Ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71734/new/ https://reviews.llvm.org/D71734 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-

[PATCH] D75311: [modules] Allow frameworks to have only a private module without a public one.

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

[PATCH] D75395: [clang][Modules] Add -fsystem-module flag

bruno added a comment. Hi Michael, thanks for improving this. Comment at: clang/include/clang/Driver/Options.td:1448 Flags<[CC1Option]>, Alias; +def fsystem_module : Flag<["-"], "fsystem-module">, Flags<[CC1Option]>, + HelpText<"Build this module as a system module. Only us

[PATCH] D75395: [clang][Modules] Add -fsystem-module flag

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/D75395/new/ https://reviews.llvm.org/D75395 ___ c

[PATCH] D70063: clang/Modules: Error if ReadASTBlock does not find the main module

bruno added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSerializationKinds.td:78 +def err_module_file_missing_definition : Error< + "module file '%0' is missing the main module's definition">, DefaultFatal; aprantl wrote: > Should this be

[PATCH] D70063: clang/Modules: Error if ReadASTBlock does not find the main module

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM with some minor comment below! Comment at: clang/include/clang/Basic/DiagnosticSerializationKinds.td:78 +def err_module_file_missing_definition : Error< + "module file '%

[PATCH] D70556: clang/Modules: Refactor CompilerInstance::loadModule, NFC

bruno accepted this revision. bruno added a comment. Nice! LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70556/new/ https://reviews.llvm.org/D70556 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/

[PATCH] D70583: clang/Modules: Rename CompilerInstance::ModuleManager, NFC

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. > I have to assume there was a point in the past where they were just one > class, but it's been pretty confusing for a while. I think it's time to fix > it. Thanks! LGTM CHANGES SINCE LAST

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

bruno created this revision. bruno added reviewers: rsmith, arphaman, vsapsai, martong. Herald added subscribers: cfe-commits, ributzka, dexonsmith, jkorous, rnkovacs. Herald added a reviewer: jdoerfert. Herald added a project: clang. Take `struct Z {...}` defined differently and imported from bot

[PATCH] D75323: Support relative dest paths in headermap files

bruno requested changes to this revision. bruno added a comment. This revision now requires changes to proceed. I also want to understand why you want this change. I'm not convinced this is the behavior we want and it's not like we encourage people to use header maps in the first place. I'd be f

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

bruno added a comment. > - Why are you adding ODR hash support for `RecordDecl` and not `TagDecl`? We > already have support for `EnumDecl`, so `TagDecl` seems like a good candidate > to cover both. Honestly, I don't know if it is possible or a good idea but it > looks plausible enough to consi

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

bruno updated this revision to Diff 241855. bruno added a comment. - Update patch after Volodymyr's review. - Refactor bits done as part of rG90f58eaeff5f , update it to remove that part. Repository: rG LLVM Github Monorepo

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

bruno marked 3 inline comments as done. bruno added a comment. Thanks for taking a look Richard, comments inline. Comment at: clang/include/clang/AST/Decl.h:4009-4010 + + /// Store the ODR hash for this decl. + unsigned ODRHash; }; rsmith wrote: > We shouldn

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

bruno updated this revision to Diff 244435. bruno added a comment. Address Richard's review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71734/new/ https://reviews.llvm.org/D71734 Files: clang/include/clang/AST/Decl.h clang/include/clang/AST

[PATCH] D44498: Sink PrettyDeclStackTrace down to the AST library

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Hi Jordan, Thanks for improving this. LGTM Repository: rC Clang https://reviews.llvm.org/D44498 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D44652: [vfs] Don't bail out after a missing -ivfsoverlay file

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. One minor suggestion but otherwise LGTM Comment at: lib/Frontend/CompilerInvocation.cpp:3083 +} else { Diags.Report(diag::err_invalid_vfs_overlay) << File; } --

[PATCH] D44130: Driver: Add gcc search path for RHEL devtoolset-7

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D44130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/

[PATCH] D42608: Driver: Prefer vendor supplied gcc toolchain

bruno added a comment. The change seems fine. Perhaps add tests to make sure this won't cause problems in the future? Something along the lines of test/Driver/linux-ld.c? Repository: rC Clang https://reviews.llvm.org/D42608 ___ cfe-commits mail

[PATCH] D43871: [modules] No longer include stdlib.h from mm_malloc.h.

bruno added inline comments. Comment at: test/CXX/except/except.spec/libc-empty-except.cpp:3 +// RUN: not %clang_cc1 -std=c++11 -I %S/libc-empty-except-sys -fexceptions -fcxx-exceptions -fsyntax-only -verify %s + +// expected-no-diagnostics Since this is intende

[PATCH] D34848: Driver: Don't mix system tools with devtoolset tools on RHEL

bruno added a comment. Is there a way to test this? Comment at: lib/Driver/ToolChains/Linux.cpp:220 + "/../bin").str()); + } + You can remove the curly braces here. https://reviews.llvm.org/D34848 __

[PATCH] D44901: [Diag] Avoid emitting a redefinition note if no location is available.

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D44901 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D45012: [Modules] Skip adding unused module maps to the dependency file

bruno created this revision. bruno added a reviewer: rsmith. Current generation of dependency files when modules is on take into account all parsed module maps found while searching for a specific module. Add a new cc1 flag that allows changing that behavior: only add module maps that actually an

[PATCH] D33275: Keep into account if files were virtual.

bruno added a comment. Ping! https://reviews.llvm.org/D33275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

<    1   2   3   4   5   >