[PATCH] D51568: [modules] Add `-fno-absolute-module-directory` flag for relocatable modules

bruno added a comment. Thanks for working on this @andrewjcg In D51568#1313717 , @rsmith wrote: > I'm also not convinced we need to put this behind a flag. It would seem > reasonable to me to simply always emit the `MODULE_DIRECTORY` as relative (if > w

[PATCH] D54880: Ignore gcc's stack-clash-protection flag

bruno added a comment. Looks like adding a test in `test/Driver/clang_f_opts.c` would be good here. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54880/new/ https://reviews.llvm.org/D54880 ___ cfe-commits mailing lis

[PATCH] D54923: [Modules] Remove non-determinism while serializing DECL_CONTEXT_LEXICAL and DECL_RECORD

bruno added a comment. @rsmith this is on the non-determinism issue we discussed offline. Do you have any concerns with this approach? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54923/new/ https://reviews.llvm.org/D54923 ___ cfe-commits

[PATCH] D55097: [constexpr][c++2a] Try-catch blocks in constexpr functions

bruno updated this revision to Diff 176559. bruno added a comment. Address @rsmith comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55097/new/ https://reviews.llvm.org/D55097 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/AST/ExprConstant.cpp lib/Sema/SemaDeclCXX.cpp

[PATCH] D55097: [constexpr][c++2a] Try-catch blocks in constexpr functions

bruno updated this revision to Diff 176733. bruno added a comment. Update patch after @erik.pilkington review! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55097/new/ https://reviews.llvm.org/D55097 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/AST/ExprConstant.cpp lib/S

[PATCH] D54923: [Modules] Remove non-determinism while serializing DECL_CONTEXT_LEXICAL and DECL_RECORD

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

[PATCH] D55097: [constexpr][c++2a] Try-catch blocks in constexpr functions

bruno updated this revision to Diff 177367. bruno marked an inline comment as done. bruno added a comment. Address @aaron.ballman and @erik.pilkington reviews. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55097/new/ https://reviews.llvm.org/D55097 Files: include/clang/Basic/Diagnost

[PATCH] D55097: [constexpr][c++2a] Try-catch blocks in constexpr functions

This revision was automatically updated to reflect the committed changes. Closed by commit rL348789: [constexpr][c++2a] Try-catch blocks in constexpr functions (authored by bruno, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D55097?v

[PATCH] D55500: [Builtins] Implement __builtin_is_constant_evaluated for use in C++2a

bruno added a comment. Hi Eric, Thanks for working on this! Comment at: include/clang/Basic/Builtins.def:759 +// Random C++ builtins. +LANGBUILTIN(__builtin_is_constant_evaluated, "b", "ncu", CXX_LANG) + Name bikeshedding : perhaps the builtin name could be de

[PATCH] D54923: [Modules] Remove non-determinism while serializing DECL_CONTEXT_LEXICAL and DECL_RECORD

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

[PATCH] D58891: Modules: Add -Rmodule-import

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Nice, note that a subset of this information can be achieved with `-Wauto-import`, but this is way more general and solid, since it will handles `@import`s and pragma imports too. LGTM with a mi

[PATCH] D58890: Modules: Rename MemoryBufferCache to InMemoryModuleCache

bruno added a comment. `InMemoryModuleCache` seems like a way more appropriate name here. Also thanks for improving some of the comments. > Because of the move to Serialization we can no longer abuse the Preprocessor > to forward it to the ASTReader. Besides the rename and file move, that means

[PATCH] D58893: Modules: Invalidate out-of-date PCMs as they're discovered

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Thanks for working on this, the approach & patch LGTM. Comment at: clang/include/clang/Serialization/InMemoryModuleCache.h:37 /// Track the timeline of when this was add

[PATCH] D58890: Modules: Rename MemoryBufferCache to InMemoryModuleCache

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. > `ninja check-clang` passes... is there anything else I should be testing? I'm not sure, just double checking :) The general approach LGTM though. CHANGES SINCE LAST ACTION https://review

[PATCH] D51910: [Modules] Add platform feature to requires clause

bruno added inline comments. Comment at: docs/Modules.rst:476-477 + +*platform-environment* + A platform-environment variant (e.g. ``linux-gnueabi``, ``windows-msvc``) is available. rsmith wrote: > What is the reason to allow these to be combined into a singl

[PATCH] D51910: [Modules] Add platform feature to requires clause

bruno updated this revision to Diff 165861. bruno added a comment. Update patch after review. https://reviews.llvm.org/D51910 Files: docs/Modules.rst lib/Basic/Module.cpp test/Modules/target-platform-features.m Index: test/Modules/target-platform-features.m ==

[PATCH] D51910: [Modules] Add platform feature to requires clause

This revision was automatically updated to reflect the committed changes. Closed by commit rC342499: [Modules] Add platform and environment features to requires clause (authored by bruno, committed by ). Changed prior to commit: https://reviews.llvm.org/D51910?vs=165861&id=165998#toc Repositor

[PATCH] D51910: [Modules] Add platform feature to requires clause

This revision was automatically updated to reflect the committed changes. Closed by commit rL342499: [Modules] Add platform and environment features to requires clause (authored by bruno, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/

[PATCH] D52253: Fix an assert in the implementation of -Wquoted-include-in-framework-header

bruno added a comment. Hi Erik, Thanks for catching this. Can you rename the test and the input file path to something more meaningful? Better yet if you remove the radar bits. I think you can actually squeeze this test in test/Modules/double-quotes.m. Repository: rC Clang https://reviews

[PATCH] D52253: Fix an assert in the implementation of -Wquoted-include-in-framework-header

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM with one more small change. Comment at: clang/test/Modules/double-quotes.m:35 +// rdar://43692300 +#import "NotAFramework/Headers/Headers/Thing1.h" Plea

[PATCH] D50539: [VFS] Add property 'fallthrough' that controls fallback to real file system.

bruno added a comment. Hi Volodymyr, Thanks for working on this, really nice work with the tests! Comments below. > - No support for 'fallthrough' in crash reproducer. That'd be nice to have at some point to make sure we never escape into the real fs. > - Current way of working with modules i

[PATCH] D58072: Make ModuleDependencyCollector's method virtual (NFC)

bruno added a comment. How much of the `ModuleDependencyCollector` will be reused as is by LLDB? I wonder about the tradeoff versus inheriting from `DependencyCollector` directly. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58072/new/ https://reviews.llvm.org/

[PATCH] D58072: Make ModuleDependencyCollector's method virtual (NFC)

bruno added a comment. Not really. Would making only the `attachTo*` methods virtual enough though? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58072/new/ https://reviews.llvm.org/D58072 ___ cfe-commits mailing lis

[PATCH] D58072: Make ModuleDependencyCollector's method virtual (NFC)

bruno added a comment. I missed that the `DependencyCollector` already marks them virtual, you are just making it obvious here. I think you can omit the ones that are already virtual and only add to the ones that are on the intend of this patch. Repository: rC Clang CHANGES SINCE LAST ACTI

[PATCH] D58072: Make ModuleDependencyCollector's method virtual (NFC)

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58072/new/ https://reviews.llvm.org/D58072 ___ cfe-commits mai

[PATCH] D32520: Support __fp16 vectors

bruno added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:1042 +} + +assert(SrcElementTy->isFloatingPointTy() && What happens if the SrcElementTy is float and DstElementTy isn't? Seems like it will hit the assertion below.

[PATCH] D32520: Support __fp16 vectors

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

[PATCH] D37299: [Modules] Add ability to specify module name to module file mapping in a file

bruno added a comment. Hi Boris, This is a handy option, very nice. Can you also add testcases? Comment at: lib/Frontend/CompilerInvocation.cpp:1561 + + auto Buf = FileMgr.getBufferForFile(File); + if (!Buf) { `auto Buf` -> `auto *Buf` https://reviews.llvm

[PATCH] D36589: Add support for remembering origins to ExternalASTMerger

bruno added a comment. Thanks for the additional docs! More comments below. Comment at: lib/AST/ExternalASTMerger.cpp:116 +if (auto *ToDC = dyn_cast(To)) { + logs() << "(ExternalASTMerger*)" << (void*)&Parent + << " imported (DeclContext*)" << (void*)ToDC -

[PATCH] D38208: Add support for remembering origins to ExternalASTMerger

bruno added inline comments. Comment at: lib/AST/ExternalASTMerger.cpp:79 + return cast(SearchResultDecl)->getPrimaryContext(); +else + return nullptr; // This type of lookup is unsupported No need for `else` here. Comment at: lib

[PATCH] D63518: WIP BitStream reader: propagate errors

bruno added a comment. Hi JF. Thanks for working on this, nice improvement to error handling! The overall approach is pretty solid and should prevent a lot of red herring while investigating hard to reproduce crashes in clang, specially when implicit clang modules is involved. Dropping the erro

[PATCH] D65545: Handle some fs::remove failures

bruno added a comment. Thanks for working on this JF! Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:107-111 TemporaryFiles::~TemporaryFiles() { llvm::MutexGuard Guard(Mutex); for (const auto &File : Files) -llvm::sys::fs::remove(File.getKey()); +if (std:

[PATCH] D65846: Improve error message from FrontendAction

bruno added a comment. > Internal compiler error: LFS error for > "/Users/jfb/s/llvmm/debug/tools/clang/test/Index/Output/pch-from-libclang.c.tmp.mcp/14EC4PG1UTVLY/modules.idx"failed > to create unique file > /Users/jfb/s/llvmm/debug/tools/clang/test/Index/Output/pch-from-libclang.c.tmp.mcp

[PATCH] D65986: Allow setting the VFS to 'real' mode instead of default 'physical'

bruno accepted this revision. bruno added a comment. LGTM Comment at: clang/test/Driver/vfsmode.py:47 +# exceed PATH_MAX. +assert os.path.exists(file_relative_path) +assert not os.path.exists(os.path.join(absolute, filename)) If you want to make this even more

[PATCH] D65907: Introduce FileEntryRef and use it when handling includes to report correct dependencies when the FileManager is reused across invocations

bruno added inline comments. Comment at: clang/include/clang/Basic/FileManager.h:110 +/// A reference to a \c FileEntry that includes the name of the file as it was +/// accessed by the FileManager's client. +class FileEntryRef { How does that work with the VFS?

[PATCH] D65545: Handle some fs::remove failures

bruno accepted this revision. bruno added a comment. LGTM with one minor change. Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:38 +#define DEBUG_TYPE "pch" + `pch-preamble` is more accurate here. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D63518: BitStream reader: propagate errors

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

[PATCH] D60974: Clang IFSO driver action.

bruno added a comment. > I am currently working on the next part of clang interface stubs that will > take the interface stubs per compilation unit and merge them into one text > stub (which will be used by something like llvm-elfabi to generate a stubbed > out ELF .so file). I was using llvm

[PATCH] D55676: [Modules] Fix decl order for DeclsInPrototype

bruno created this revision. bruno added reviewers: rsmith, aprantl, v.g.vassilev. Herald added subscribers: dexonsmith, mgrang, jkorous. When a declarator is constructed for a function prototype in `Parser::ParseFunctionDeclarator`, it calls getCurScope()->decls() in order to populate DeclsInProt

[PATCH] D54923: [Modules] Remove non-determinism while serializing DECL_CONTEXT_LEXICAL and DECL_RECORD

bruno abandoned this revision. bruno added a comment. Abandon this one in favor of https://reviews.llvm.org/D55676 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54923/new/ https://reviews.llvm.org/D54923 ___ cfe-commits mailing list cfe-com

[PATCH] D55676: [Modules] Fix decl order for DeclsInPrototype

bruno added a comment. This fixes the same problem previously addressed in https://reviews.llvm.org/D54923 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55676/new/ https://reviews.llvm.org/D55676 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D55500: [Builtins] Implement __builtin_is_constant_evaluated for use in C++2a

bruno added inline comments. Comment at: include/clang/Basic/Builtins.def:759 +// Random C++ builtins. +LANGBUILTIN(__builtin_is_constant_evaluated, "b", "ncu", CXX_LANG) + EricWF wrote: > EricWF wrote: > > bruno wrote: > > > Name bikeshedding : perhaps the built

[PATCH] D67249: [Modules][PCH] Hash input files content

bruno marked an inline comment as done. bruno added inline comments. Comment at: clang/lib/Serialization/ASTWriter.cpp:1767 bool IsTopLevelModuleMap; + uint32_t ContentHash[2]; }; ributzka wrote: > bruno wrote: > > aprantl wrote: > > > Why is this not a uint

[PATCH] D67249: [Modules][PCH] Hash input files content

bruno updated this revision to Diff 219607. bruno added a comment. Update the patch to use two ::Fixed, 32 in abbrev to encode hash. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67249/new/ https://reviews.llvm.org/D67249 Files: clang/include/cl

[PATCH] D66982: [Modules][Objective-C] Use complete decl from module when diagnosing missing import

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

[PATCH] D67249: [Modules][PCH] Hash input files content

bruno added a comment. > Did you try xxHash64? No, and I'm not planning to. I believe `hash_code` is enough here, already has low overhead on performance and size, and bunch of prior use elsewhere in LLVM/Clang. If there's a strong reason to do it I wanna know why that's the case first. Re

[PATCH] D66982: [Modules][Objective-C] Use complete decl from module when diagnosing missing import

This revision was automatically updated to reflect the committed changes. Closed by commit rL372039: [Modules][Objective-C] Use complete decl from module when diagnosing missing… (authored by bruno, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed pri

[PATCH] D67249: [Modules][PCH] Hash input files content

bruno added a reviewer: v.g.vassilev. bruno added a comment. Ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67249/new/ https://reviews.llvm.org/D67249 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D67010: [Modules] Move search paths from control block to unhashed control block

bruno added a reviewer: v.g.vassilev. bruno added a comment. Ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67010/new/ https://reviews.llvm.org/D67010 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D37475: Make MultiplexASTDeserializationListener part of the API [NFC]

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

[PATCH] D46165: [Modules] Handle ObjC/C ODR-like semantics for EnumConstantDecl

bruno created this revision. bruno added a reviewer: rsmith. Herald added a subscriber: jkorous. Support for ObjC/C ODR-like semantics with structural equivalence checking was added back in r306918. There enums are handled and also checked for structural equivalence. However, at use time of Enum

[PATCH] D46165: [Modules] Handle ObjC/C ODR-like semantics for EnumConstantDecl

bruno marked an inline comment as done. bruno added inline comments. Comment at: lib/Serialization/ASTReaderDecl.cpp:2568-2570 // ODR-based merging is only performed in C++. In C, identically-named things // in different translation units are not redeclarations (but may sti

[PATCH] D46165: [Modules] Handle ObjC/C ODR-like semantics for EnumConstantDecl

bruno closed this revision. bruno marked an inline comment as done. bruno added a comment. Landed in r331232 & r331233. Repository: rC Clang https://reviews.llvm.org/D46165 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm

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

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

[PATCH] D30881: Track skipped files in dependency scanning

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Thanks for the detailed answers. LGTM https://reviews.llvm.org/D30881 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org

[PATCH] D46485: Add python tool to dump and construct header maps

bruno created this revision. bruno added a reviewer: rsmith. Herald added a subscriber: mgorny. Header maps are binary files used by Xcode, which are used to map header names or paths to other locations. Clang has support for those since its inception, but there's not a lot of header map testing a

[PATCH] D48367: [modules] Fix 37878; Autoload subdirectory modulemaps with specific LangOpts

bruno added inline comments. Comment at: clang/lib/Lex/HeaderSearch.cpp:285 // directory. -loadSubdirectoryModuleMaps(SearchDirs[Idx]); +if (ModMap.getLangOpts().ObjC1 || ModMap.getLangOpts().ObjC2) + loadSubdirectoryModuleMaps(SearchDirs[Idx]);

[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders

bruno closed this revision. bruno added a comment. Committed r335542 https://reviews.llvm.org/D47301 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48685: [PCH+Modules] Load -fmodule-map-file content before including PCHs

bruno created this revision. bruno added reviewers: rsmith, benlangmuir. Consider: 1. Generate PCH with -fmodules and -fmodule-map-file 2. Use PCH with -fmodules and the same -fmodule-map-file If we don't load -fmodule-map-file content before including PCHs, the modules that are dependencies in

[PATCH] D47118: [modules] Print input files when -module-file-info file switch is passed.

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. > It looks like the diagnostic options and few others were moved out from the > control block in r297655 by Manman Ren. The header files are still part of > the control block. The best I can d

[PATCH] D48367: [modules] Fix 37878; Autoload subdirectory modulemaps with specific LangOpts

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Thanks for working on this, LGTM https://reviews.llvm.org/D48367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-

[PATCH] D48736: [frontend] Don't include the C++ stdlib for -x assembler-with-cpp

bruno added inline comments. Comment at: lib/Frontend/InitHeaderSearch.cpp:474 + if (Lang.CPlusPlus && !Lang.AsmPreprocessor && + HSOpts.UseStandardCXXIncludes && HSOpts.UseStandardSystemIncludes) { if (HSOpts.UseLibcxx) { Since you added `HSOpts.UseSt

[PATCH] D48736: [frontend] Don't include the C++ stdlib for -x assembler-with-cpp

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/Frontend/InitHeaderSearch.cpp:474 + if (Lang.CPlusPlus && !Lang.AsmPreprocessor && + HSOpts.UseStandardCXXIncludes && HSOpts.UseStandardSystemInclude

[PATCH] D47297: [Modules][ObjC] Add protocol redefinition to the current scope/context

bruno added a comment. Herald added a subscriber: dexonsmith. Ping! Repository: rC Clang https://reviews.llvm.org/D47297 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47297: [Modules][ObjC] Add protocol redefinition to the current scope/context

bruno added inline comments. Comment at: lib/Sema/SemaDeclObjC.cpp:1208 +// serialize something meaningful. +if (getLangOpts().Modules) + PushOnScopeChains(PDecl, TUScope); arphaman wrote: > Is this a problem only for modules or does this show up in

[PATCH] D47297: [Modules][ObjC] Add protocol redefinition to the current scope/context

This revision was automatically updated to reflect the committed changes. Closed by commit rC336031: Add protocol redefinition to the current scope/context (authored by bruno, committed by ). Repository: rL LLVM https://reviews.llvm.org/D47297 Files: lib/Sema/SemaDeclObjC.cpp test/Modules

[PATCH] D47297: [Modules][ObjC] Add protocol redefinition to the current scope/context

This revision was automatically updated to reflect the committed changes. Closed by commit rL336031: Add protocol redefinition to the current scope/context (authored by bruno, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D47297?vs=14

[PATCH] D48685: [PCH+Modules] Load -fmodule-map-file content before including PCHs

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

[PATCH] D68528: [Implicit Modules] Add -cc1 option -fmodules-strict-hash which includes search paths and diagnostics.

bruno added a comment. Hi Michael, thanks for working on this! Comment at: include/clang/Lex/HeaderSearchOptions.h:206 + /// Weather we should include all things that could impact the module in the + /// hash. *whether Comment at: include

[PATCH] D67249: [Modules][PCH] Hash input files content

This revision was automatically updated to reflect the committed changes. Closed by commit rG2a1386c81de5: [Modules][PCH] Hash input files content (authored by bruno). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67249/new/ https://reviews.llvm.org

[PATCH] D68528: [Implicit Modules] Add -cc1 option -fmodules-strict-hash which includes search paths and diagnostics.

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM with one minor change. Can you add an entry in the modules docs for this flag and mention that using it can lead to more PCMs in an implicit build? CHANGES SINCE LAST ACTION https://revi

[PATCH] D68528: [Implicit Modules] Add -cc1 option -fmodules-strict-hash which includes search paths and diagnostics.

bruno added a comment. *using implicit modules in a build where compiler flags in different invocations aren't homogeneous, or something along those lines. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68528/new/ https://reviews.llvm.org/D68528 __

[PATCH] D68528: [Implicit Modules] Add -cc1 option -fmodules-strict-hash which includes search paths and diagnostics.

bruno added a comment. > While adding the documentation I realized that a better name for this option > would be `-fmodules-strict-context-hash` to make it clear which hash it's > referring to. `-fmodules-strict-context-hash` SGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68528

[PATCH] D84458: [Modules] Improve error message when cannot find parent module for submodule definition.

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM as is, minor suggestion below. Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:700 + "no module named '%0' %select{found|in '%2'}1, " + "expected parent modu

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

bruno added a comment. Hi Zixu, thanks for working on improving this. I agree with @vsapsai on the the `GenModuleActionWrapper` approach. Also, it seems to me that even though it would somehow improve the accuracy, it would be solving a more general problem, not really specific to this patch. W

[PATCH] D80263: [HeaderSearch] Fix processing #import-ed headers multiple times with modules enabled.

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

[PATCH] D85810: [clang] Pass-through remarks options to linker

bruno added a comment. Hi Wei, this looks handy! Minor stylish comments below. Comment at: clang/include/clang/Driver/Driver.h:638 +/// This checks for clang specific R-value ('-Rpass-*') group. +bool hasRpassOptions(const llvm::opt::ArgList &Args); + Nitpickin

[PATCH] D40112: [CodeGen][X86] Fix handling of __fp16 vectors

bruno added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:954 if (DstTy->isFloatingPointTy()) { - if (!CGF.getContext().getLangOpts().HalfArgsAndReturns) + if (CGF.getContext().getTargetInfo().useFP16ConversionIntrinsics()) return Builder.Creat

[PATCH] D40112: [CodeGen][X86] Fix handling of __fp16 vectors

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM with the minor comment below. Comment at: include/clang/Basic/TargetInfo.h:563 + /// Check whether llvm intrinsics such as llvm.convert.to.fp16 should be used + /// to c

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

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] 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] 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] D66907: [Modules] Fix rebuilding an updated module for each of its consumers.

bruno accepted this revision. bruno added a comment. Thanks for fixing this, LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66907/new/ https://reviews.llvm.org/D66907 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://list

[PATCH] D66710: ASTReader: Bypass overridden files when reading PCHs

bruno added a comment. Nice! Is this something that can be tested for in `unittests/Basic/FileManagerTest.cpp`? Comment at: clang/include/clang/Basic/FileManager.h:320 + /// twice, you get two new file entries. + const FileEntry *getBypassFile(const FileEntry &VFE); + --

[PATCH] D66982: [Modules][Objective-C] Use complete decl from module when diagnosing missing import

bruno created this revision. bruno added reviewers: rsmith, arphaman. Herald added subscribers: cfe-commits, ributzka, dexonsmith, jkorous. Herald added a project: clang. Otherwise the definition (first found) for ObjCInterfaceDecl's might precede the module one, which will eventually lead to cras

[PATCH] D66710: ASTReader: Bypass overridden files when reading PCHs

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

[PATCH] D67010: [Modules] Move search paths from control block to unhashed control block

bruno created this revision. bruno added reviewers: dexonsmith, arphaman, rsmith. Herald added subscribers: cfe-commits, ributzka, jkorous. Herald added a project: clang. Header search paths are currently ignored for the purpose of computing `getModuleHash()` during a module build. This means that

[PATCH] D67249: [Modules][PCH] Hash input files content

bruno created this revision. bruno added reviewers: dexonsmith, arphaman, rsmith, aprantl. Herald added subscribers: cfe-commits, jkorous. Herald added a project: clang. When files often get touched during builds, the mtime based validation leads to different problems in implicit modules builds, e

[PATCH] D67249: [Modules][PCH] Hash input files content

bruno marked 2 inline comments as done. bruno added a comment. > Nice! Are you planning to address the FIXME's in a later update of this patch? The FIXME's are just replaying what the code around does, both error dropping and `FileEntryRef` are recent changes that didn't make all the way throu

[PATCH] D67249: [Modules][PCH] Hash input files content

bruno updated this revision to Diff 219166. bruno added a comment. Update testcase to use a more portable version of `touch` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67249/new/ https://reviews.llvm.org/D67249 Files: clang/include/clang/Basi

[PATCH] D67249: [Modules][PCH] Hash input files content

bruno updated this revision to Diff 219168. bruno added a comment. Remove pasto from one of the testcases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67249/new/ https://reviews.llvm.org/D67249 Files: clang/include/clang/Basic/DiagnosticSeriali

[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.

bruno added a comment. Nice! One more comment: please also fix names of local variables, member variables and function parameters. See https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly. Yes, there is precedence for violations in other places in

[PATCH] D34878: [ARM] Option for reading thread pointer from coprocessor register

bruno added inline comments. Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:136 +if (ThreadPointer == ReadTPMode::Invalid && +!StringRef(A->getValue()).empty()) { + D.Diag(diag::err_drv_invalid_mtp) << A->getAsString(Args); What happens if you pa

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

bruno marked an inline comment as done. bruno added a comment. Comment at: clang/lib/Serialization/ASTReader.cpp:9286 + false /*UseCanonicalDecls*/); + (void)Ctx.IsEquivalent(D, Canon); +} martong wrote: > Would it be possible to check the st

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

bruno updated this revision to Diff 236964. bruno added a comment. Change the approach: handle type merging for tag types using the ODRHash mechanism Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71734/new/ https://reviews.llvm.org/D71734 Files:

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

bruno updated this revision to Diff 236965. bruno added a comment. Remove some FIXMEs that are now done. 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/inclu

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

bruno marked an inline comment as done. bruno added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:11007 + // Issue any pending ODR-failure diagnostics. + for (auto &Merge : RecordOdrMergeFailures) { rtrieu wrote: > Is this just a copy of

[PATCH] D87853: [Sema] Update specialization iterator after template argument deduction.

bruno commandeered this revision. bruno edited reviewers, added: hoy; removed: bruno. bruno added a comment. Herald added subscribers: modimo, lxfind, ributzka, jkorous. I'm taking this over from Hongtao (w/ his blessing :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

<    1   2   3   4   5   >