[PATCH] D55463: Introduce a source minimizer that reduces source to directives that might affect the dependency list for a compilation

Bigcheese requested changes to this revision. Bigcheese added a comment. This revision now requires changes to proceed. I have a bit more review to do, but this is what I've found so far. The naming comments are just suggestions, but the digit separators' are actually an issue. ==

[PATCH] D63101: [Frontend] SetUpDiagnosticLog should handle unowned diagnostic consumer in the compiler

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

[PATCH] D65493: Modernize atomic detection and usage

Bigcheese added a comment. This looks like a good simplification, but I think `call_once` could be simplified more. Comment at: llvm/cmake/modules/CheckAtomic.cmake:46 + if (NOT HAVE_ATOMICS_WITH_LIB) + message(FATAL_ERROR "Host compiler must support atomic!")

[PATCH] D65989: [clang-scan-deps] Add minimizer support for C++20 modules.

Bigcheese created this revision. Bigcheese added a reviewer: arphaman. Herald added subscribers: tschuett, dexonsmith. Herald added a project: clang. This only adds support to the minimizer, it doesn't actually capture the dependencies yet. Repository: rC Clang https://reviews.llvm.org/D6598

[PATCH] D65989: [clang-scan-deps] Add minimizer support for C++20 modules.

Bigcheese closed this revision. Bigcheese added a comment. Fixed and committed as r368381. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65989/new/ https://reviews.llvm.org/D65989 ___ cfe-commits mailing list cfe-com

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

Bigcheese added a comment. This fix works, but we could also use openat to get around max path length issues. Windows also has an API that can be used similarly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65986/new/ https://reviews.llvm.org/D6

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

Bigcheese added inline comments. Comment at: clang/include/clang/Basic/FileManager.h:130 + + const DirectoryEntry *getDir() const { return Entry.getDir(); } + Isn't this incorrect in the case of symlinks? Comment at: clang/include/clang/Basic/

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

Bigcheese requested changes to this revision. Bigcheese added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Basic/SourceManager.h:1024 + Optional getFileEntryRefForID(FileID FID) const { +bool MyInvalid = false; +const Sr

[PATCH] D63518: BitStream reader: propagate errors

Bigcheese added inline comments. Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:527-529 + if (llvm::Error Err = readSubBlock(BlockOrCode, I)) { +if (llvm::Error Skipped = Stream.SkipBlock()) + return Skipped; This is a pretty big

[PATCH] D64149: [clang-scan-deps] use `-Wno-error` when scanning for dependencies

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

[PATCH] D64164: [clang][ARM] Fix msvc arm{64} builtins to use int on LLP64 systems.

Bigcheese created this revision. Bigcheese added reviewers: bruno, rnk, bkelley. Herald added subscribers: kristina, jfb, dexonsmith, kristof.beyls, javed.absar. Herald added a project: clang. The `InterlockedX_{acq,nf,rel}` functions deal with 32 bits which is long on MSVC, but int on most other

[PATCH] D64164: [clang][ARM] Fix msvc arm{64} builtins to use int on LLP64 systems.

Bigcheese added a comment. In D64164#1569442 , @efriedma wrote: > Do the changes to BuiltinsARM.def have any practical effect? long should be > 32 bits on all 32-bit ARM targets. I don't think they do right now. I updated them there as that's what the

[PATCH] D64164: [clang][ARM] Fix msvc arm{64} builtins to use int on LP64 systems.

Bigcheese added a comment. In D64164#1569679 , @rnk wrote: > Please check the commit message: > > > [clang][ARM] Fix msvc arm{64} builtins to use int on LLP64 systems. > > I think you mean "use int on LP64 systems", since long is 32-bits on LLP64, > right

[PATCH] D64525: [clang-scan-deps] Dependency directives source minimizer: single quotes are not digit separators after a valid character literal prefix

Bigcheese added inline comments. Comment at: clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp:270 +return false; + if (Prev == '8' && (Cur - 1 != Start) && *(Cur - 2) == 'u') +return false; Are we sure at this point that it's always safe to jump bac

[PATCH] D64525: [clang-scan-deps] Dependency directives source minimizer: single quotes are not digit separators after a valid character literal prefix

Bigcheese accepted this revision. Bigcheese added inline comments. Comment at: clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp:270 +return false; + if (Prev == '8' && (Cur - 1 != Start) && *(Cur - 2) == 'u') +return false; arphaman wrote: > Bigchee

[PATCH] D55463: Introduce a source minimizer that reduces source to directives that might affect the dependency list for a compilation

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

[PATCH] D62271: [Driver] Fix -working-directory issues

Bigcheese created this revision. Bigcheese added reviewers: dexonsmith, arphaman, jkorous. Herald added a project: clang. Currently the `-working-directory` option does not actually impact the working directory for all of the clang driver, it only impacts how files are looked up to make sure they

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

Bigcheese created this revision. Bigcheese added reviewers: rsmith, bruno. Herald added a subscriber: dexonsmith. Herald added a project: clang. Repository: rC Clang https://reviews.llvm.org/D68528 Files: include/clang/Driver/CC1Options.td include/clang/Lex/HeaderSearchOptions.h lib/Fron

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

Bigcheese updated this revision to Diff 223345. Bigcheese retitled this revision from "[Implicit Modules] Add -cc1 option -fmodules-strict-hash which includes search paths." to "[Implicit Modules] Add -cc1 option -fmodules-strict-hash which includes search paths and diagnostics.". Bigcheese added

[PATCH] D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests.

Bigcheese added a comment. In D68193#1701426 , @kousikk wrote: > @arphaman I don't mind changing this if there are race conditions as you say, > but isn't the assumption of the tool that the filesystem remains unchanged > for a single run of the tool? If

[PATCH] D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests.

Bigcheese added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:83 + std::error_code getError() const { +assert(isValid() && "not initialized"); Is this actually called anywhere? Repository: r

[PATCH] D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests.

Bigcheese accepted this revision. Bigcheese added a comment. lgtm, but wait for Alex or Duncan to also take a look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68193/new/ https://reviews.llvm.org/D68193

[PATCH] D68835: [clang-scan-deps] Add basic support for Clang modules.

Bigcheese created this revision. Bigcheese added a reviewer: arphaman. Herald added subscribers: tschuett, dexonsmith. Herald added a project: clang. This fixes two issues that prevent simple uses of Clang modules from working. - We would previously minimize _every_ file opened by clang, even mod

[PATCH] D68835: [clang-scan-deps] Add basic support for Clang modules.

Bigcheese updated this revision to Diff 224507. Bigcheese marked 2 inline comments as done. Bigcheese added a comment. Addressed review comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68835/new/ https://reviews.llvm.org/D68835 Files: lib/Tooling/Depend

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

Bigcheese marked an inline comment as done. Bigcheese added inline comments. Comment at: include/clang/Lex/HeaderSearchOptions.h:209 + /// + /// This includes things like the full header search path. + unsigned ModulesStrictHash : 1; bruno wrote: > What else d

[PATCH] D69017: Include sanitize blacklist and other extra deps as part of scan-deps output

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. lgtm. Jan may want to take a look as I believe he was looking at a related issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69017/new/

[PATCH] D68835: [clang-scan-deps] Add basic support for Clang modules.

Bigcheese updated this revision to Diff 225311. Bigcheese added a comment. Added .i, .ii, .mi, and .mmi as files to minimize. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68835/new/ https://reviews.llvm.org/D68835 Files: lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cp

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

Bigcheese updated this revision to Diff 225326. Bigcheese marked an inline comment as done. Bigcheese added a comment. Fixed spelling and updated comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68528/new/ https://reviews.llvm.org/D68528 Files: include/clang/Driver/CC1Options.t

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

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG14a3f77ba103: [Implicit Modules] Add -cc1 option -fmodules-strict-context-hash which includes… (authored by Bigcheese). Changed prior to commit: https://reviews.llvm.org/D68528?vs=225326&id=225731#toc

[PATCH] D69186: Refactor DependencyScanningTool to its own file

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. LGTM with style nit. I like that this decouples `DependencyScanningTool` from printing the results. Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:105-107 +

[PATCH] D69122: Add support to find out resource dir and add it as compilation args

Bigcheese added a comment. The current assumption is that the clang-scan-deps binary is the one that comes next to the clang binary you are using. There are lots of other differences between clang versions than just the resource-dir. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D69122: Add support to find out resource dir and add it as compilation args

Bigcheese added a reviewer: klimek. Bigcheese added a comment. I've added Manuel as a reviewer as this patch is also changing the tooling APIs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69122/new/ https://reviews.llvm.org/D69122

[PATCH] D68835: [clang-scan-deps] Add basic support for Clang modules.

This revision was automatically updated to reflect the committed changes. Closed by commit rG9ab6d8236b17: [clang-scan-deps] Add basic support for modules. (authored by Bigcheese). Changed prior to commit: https://reviews.llvm.org/D68835?vs=225311&id=226352#toc Repository: rG LLVM Github Mon

[PATCH] D69420: [clang][clang-scan-deps] Add support for extracting full module dependencies.

Bigcheese created this revision. Bigcheese added reviewers: arphaman, kousikk. Bigcheese added a project: clang. Herald added subscribers: tschuett, dexonsmith, mgrang, mgorny. This adds experimental support for extracting a Clang module dependency graph from a compilation database. The output for

[PATCH] D69420: [clang][clang-scan-deps] Add support for extracting full module dependencies.

Bigcheese added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp:94 + +void printDependencies(std::string &S, StringRef MainFile) { + // Sort the modules by name to get a deterministic order. kousikk wrote: > S

[PATCH] D69420: [clang][clang-scan-deps] Add support for extracting full module dependencies.

Bigcheese updated this revision to Diff 226508. Bigcheese marked 3 inline comments as done. Bigcheese added a comment. Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69420/new/ https://reviews.llvm.org/D69420 Files: clang/include/clang/Tooling/DependencyScanni

[PATCH] D79998: Add AST_SIGNATURE record to unhashed control block of pcm files (Patch series 2/3)

Bigcheese added a comment. In D79998#2038430 , @dang wrote: > Should I add the test here or in the clang-scan-deps patch? It's best to have a test in every non-nfc patch. You should be able to test this with llvm-bcanalyzer. Repository: rG LLVM Gith

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

Bigcheese added inline comments. Comment at: clang/include/clang/Frontend/CompilerInvocation.h:156 /// \param [out] Res - The resulting invocation. + /// \param [in] CommandLineArgs - Array of argument strings, this should not + /// contain "-cc1". Is this r

[PATCH] D84185: Better defaults for MarshallingInfoString

Bigcheese accepted this revision. Bigcheese 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/D84185/new/ https://reviews.llvm.org/D84185 _

[PATCH] D76572: Replace `T(x)` with `reinterpret_cast(x)` everywhere it means reinterpret_cast. No functional change

Bigcheese accepted this revision. Bigcheese added a comment. LGTM for llvm/{lib,include/llvm}/Object/* and llvm/tools/llvm-readobj/*. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76572/new/ https://reviews.llvm.org/D76572 __

[PATCH] D77697: libc++: adjust modulemap for non-modular C

Bigcheese added a comment. In D77697#1970586 , @dexonsmith wrote: > In D77697#1969998 , @compnerd wrote: > > > @dexonsmith - yeah, sadly I dont think that there is a good way to audit > > that - any change to the p

[PATCH] D77772: [Clang] Expose RequiresNullTerminator in FileManager.

Bigcheese created this revision. Bigcheese added reviewers: dexonsmith, vsapsai, rdhindsa. Bigcheese added a project: clang. Herald added a subscriber: hiraditya. This is needed to fix the reason 0a2be46cfdb698fe (Modules: Invalidate out-of-date PCMs as they're discovered) and 5b44a4b07fc1d ([modu

[PATCH] D77772: [Clang] Expose RequiresNullTerminator in FileManager.

Bigcheese added a comment. Not really. It's a static function in MemoryBuffer.cpp, and the `MemoryBuffer` class doesn't have a `Kind` member so we can't check for `MemoryBufferMMapFile`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D2/new/ htt

[PATCH] D77772: [Clang] Expose RequiresNullTerminator in FileManager.

Bigcheese updated this revision to Diff 256722. Bigcheese added a comment. Added a test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D2/new/ https://reviews.llvm.org/D2 Files: clang/include/clang/Basic/FileManager.h clang/lib/Basic/Fi

[PATCH] D77772: [Clang] Expose RequiresNullTerminator in FileManager.

This revision was automatically updated to reflect the committed changes. Closed by commit rG92e8af0ecbe7: [Clang] Expose RequiresNullTerminator in FileManager. (authored by Bigcheese). Changed prior to commit: https://reviews.llvm.org/D2?vs=256722&id=257852#toc Repository: rG LLVM Githu

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

Bigcheese added inline comments. Comment at: clang/include/clang/Frontend/CompilerInvocation.h:248 + /// \returns - True if parsing was successful, false otherwise + bool parseSimpleArgs(const llvm::opt::ArgList &Args, + DiagnosticsEngine &Diags);

[PATCH] D83315: Turn arcmt-* options into a single option

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. I have a slight preference for `-arcmt-action=`, but up to you if you want to change it. Otherwise LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D82574: Merge TableGen files used for clang options

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. LGTM. We can look at splitting it up again once we know what the dependencies are. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82574/new

[PATCH] D83690: Port Migator option flags to new option parsing system

Bigcheese accepted this revision. Bigcheese 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/D83690/new/ https://reviews.llvm.org/D83690 _

[PATCH] D83406: Remove NormalizerRetTy and use the decltype of the KeyPath instead

Bigcheese accepted this revision. Bigcheese 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/D83406/new/ https://reviews.llvm.org/D83406 _

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

Bigcheese added inline comments. Comment at: clang/include/clang/Frontend/CompilerInvocation.h:193 + /// \param [out] Args - The generated arguments. Note that the caller is + /// responsible for insersting the path to the clang executable and "-cc1" if + /// desired.

[PATCH] D79796: Sketch support for generating CC1 command line from CompilerInvocation

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. LGTM with `KeyPathPrefix` moved to the patch that actually uses it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79796/new/ https://revie

[PATCH] D82874: Add diagnostic option backing field for -fansi-escape-codes

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. lgtm with the fix I mentioned. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3629 #undef MERGE_ASSIGN_VALUE + llvm::sys::Process::UseANSIEscapeCodes(Diagnosti

[PATCH] D68997: Allow searching for prebuilt implicit modules.

Bigcheese added inline comments. Comment at: clang/docs/Modules.rst:231 + prebuilt modules paths (specified via ``-fprebuilt-module-path``), we will + look for a matching implicit modules in the prebuilt modules paths. + Comment at: clang/do

[PATCH] D83071: Add support for options with two flags for controlling the same field.

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

[PATCH] D70351: [clang][WIP][clang-scan-deps] Add an experimental C API.

Bigcheese added a comment. I mostly just need to rebase this patch now. I'll try to get to that soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70351/new/ https://reviews.llvm.org/D70351 ___ cfe-co

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

Bigcheese created this revision. Bigcheese added reviewers: bruno, rnk, erichkeane. Bigcheese added a project: clang. Herald added a subscriber: dexonsmith. This reverts commit 92146ce399cdb26c3a9aa4d68af8cacb7c1c0fef. This commit broke users targeting LP64 systems that expect these intrinsics to

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

Bigcheese added a comment. In D80172#2042946 , @rnk wrote: > I'd be fine going back to the behavior from before. > > > Without this behavior there is no intrinsic for 32 bit rotates on these > > platforms. > > Strictly speaking, there seems to be `__built

[PATCH] D132971: [clang][modules] Don't hard code [no_undeclared_includes] for the Darwin module

Bigcheese accepted this revision. Bigcheese added a comment. I'm fine with this change, but do we actually have a backwards compatibility policy anywhere in Clang? Would be good to know what range of SDKs a compiler release is expected to support. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

Bigcheese added a comment. For future reference this was very difficult to merge into external changes. It looks like you resorted this at the same time as renaming it, and that messes up git's rename logic. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

Bigcheese added a comment. Also, this python script just doesn't work. It's missing a sys import, a "w" flag, and a new line after each write. It also dumps the output into the source directory. Was this actually tested? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D105527: libclang.so: Make SONAME independent from LLVM version

Bigcheese added a comment. Here's a version that actually works (python 3, not sure if it's valid in 2), although I would much prefer we not write to the source directory during a build. import re import os import sys input_file = open(sys.argv[1]) with open(sys.argv[2], "w") as ou

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

Bigcheese added inline comments. Comment at: clang/lib/Lex/ModuleMap.cpp:2355 +parseModuleMembers(); + } +} bruno wrote: > Too many curly braces? This is correct. It closes the block on line 2353. Repository: rG LLVM Github Monorepo CHANGES

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

Bigcheese updated this revision to Diff 405482. Bigcheese added a comment. - Fixed documentation typo. - Made missing '{' diagnostic more clear. - Use `ModuleMapParser::skipUntil`. Emitting an actual fixup is kind of difficult from `ModuleMapParser` because the way it handles tokens makes it har

[PATCH] D118915: [clang][deps] Generate '-fmodule-file=' only for direct dependencies

Bigcheese accepted this revision. Bigcheese 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/D118915/new/ https://reviews.llvm.org/D118915 _

[PATCH] D118890: [clang][deps] Disable global module index

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. lgtm. I agree that testing this isn't really necessary given the difficulty of even constructing one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D118986: [clang][deps] Return the whole TU command line

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. This does require build systems to be able to totally replace a command line rather than just add to it, but it seems like that's a requirement unless we add a way to modify the command

[PATCH] D118855: [modules] Add a flag for TagDecl if it was a definition demoted to a declaration.

Bigcheese accepted this revision. Bigcheese 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/D118855/new/ https://reviews.llvm.org/D118855

[PATCH] D66989: FileManager: Remove ShouldCloseOpenFile argument from getBufferForFile, NFC

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. lgtm CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66989/new/ https://reviews.llvm.org/D66989 ___ cfe-commits mailing list cfe-com

[PATCH] D67127: [clang-scan-deps] add skip excluded conditional preprocessor block preprocessing optimization

Bigcheese added inline comments. Herald added a subscriber: wuzish. Comment at: clang/include/clang/Frontend/CompilerInstance.h:256 + /// preprocessor. + void setAdditionalPPCallbacks(std::unique_ptr PPC); + This kinda sounds like it can be called multiple time

[PATCH] D67127: [clang-scan-deps] add skip excluded conditional preprocessor block preprocessing optimization

Bigcheese added a comment. Looking more into this I'm not sure you need the PPCallbacks. The Preprocessor should just own the PreprocessorSkippedMappings data structure and the dependency scanner can update it live. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D68997: Allow searching for prebuilt implicit modules.

Bigcheese accepted this revision. Bigcheese 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/D68997/new/ https://reviews.llvm.org/D68997 ___

[PATCH] D82860: Port ObjCMTAction to new option parsing system

Bigcheese accepted this revision. Bigcheese 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/D82860/new/ https://reviews.llvm.org/D82860 ___

[PATCH] D83211: Factor out call to EXTRACTOR in generateCC1CommandLine

Bigcheese added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3938 + if ((FLAGS)&options::CC1Option) { \ +const auto &Extracted = EXTRACTOR(this->KEYPATH); \ +if (ALWAYS_EMIT |

[PATCH] D83298: Add ability to make fixups to CompilerInvocation after option parsing

Bigcheese accepted this revision. Bigcheese 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/D83298/new/ https://reviews.llvm.org/D83298 __

[PATCH] D86290: Move all fields of '-cc1' option related classes into def file databases

Bigcheese accepted this revision. Bigcheese 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/D86290/new/ https://reviews.llvm.org/D86290 __

[PATCH] D92775: [clang][cli] Add flexible TableGen multiclass for boolean options

Bigcheese added inline comments. Comment at: clang/unittests/Frontend/CompilerInvocationTest.cpp:226 ASSERT_THAT(GeneratedArgs, - Contains(StrEq("-fno-experimental-new-pass-manager"))); + Not(Contains(StrEq("-fno-experimental-new-pass-manager";

[PATCH] D92857: [clang][cli] Don't always emit -f[no-]experimental-new-pass-manager

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. LGTM. I don't see any reason to always emit this option. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92857/new/ https://reviews.llvm.org

[PATCH] D92191: [clang-scan-deps] Add support for clang-cl

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. lgtm. Thanks for the patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92191/new/ https://reviews.llvm.org/D92191 ___ cfe-commits

[PATCH] D93008: [clang][cli] Do not marshall only CC1Option flags in BoolOption

Bigcheese accepted this revision. Bigcheese 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/D93008/new/ https://reviews.llvm.org/D93008 __

[PATCH] D93094: [clang][cli] Prevent double denormalization

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. lgtm with the test fix. Comment at: clang/unittests/Frontend/CompilerInvocationTest.cpp:267 - ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fdebug-pass-manager"))); +

[PATCH] D84669: [clang][cli] Port CodeGenOpts simple string flags to new option parsing system

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. lgtm with the comment. Comment at: clang/include/clang/Driver/Options.td:2123-2125 def fpatchable_function_entry_EQ : Joined<["-"], "fpatchable-function-entry=">, Gro

[PATCH] D84668: [clang][cli] Port TargetOpts simple string based options to new option parsing system

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. lgtm with the fix above. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:264-265 +template ::value, bool> = true> +static void denormalizeSimpleEnum(SmallVector

[PATCH] D84670: [clang][cli] Port HeaderSearch simple string options to new option parsing system

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. lgtm with nit. Comment at: clang/include/clang/Driver/Options.td:1815 + HelpText<"Specify the interval (in seconds) after which a module file will be considered unuse

[PATCH] D84671: [clang][cli] Port LangOpts simple string based options to new option parsing system

Bigcheese accepted this revision. Bigcheese 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/D84671/new/ https://reviews.llvm.org/D84671 __

[PATCH] D84672: [clang][cli] Port PreprocessorOpts simple string based options to new option parsing system

Bigcheese accepted this revision. Bigcheese 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/D84672/new/ https://reviews.llvm.org/D84672 ___

[PATCH] D93395: [clang][cli] Remove -f[no-]trapping-math from -cc1 command line

Bigcheese added a comment. This looks fine to be as that code is definitely dead. I would wait for @SjoerdMeijer in case there's a bug in the existing code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93395/new/ https://reviews.llvm.org/D93395

[PATCH] D82756: Port some floating point options to new option marshalling infrastructure

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

[PATCH] D82860: Port ObjCMTAction to new option parsing system

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

[PATCH] D83211: Factor out call to EXTRACTOR in generateCC1CommandLine

Bigcheese accepted this revision. Bigcheese added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3938 + if ((FLAGS)&options::CC1Option) { \ +const auto &Extracted = EXTRACTOR(this->KEYPATH);

[PATCH] D83211: Factor out call to EXTRACTOR in generateCC1CommandLine

Bigcheese added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3938 + if ((FLAGS)&options::CC1Option) { \ +const auto &Extracted = EXTRACTOR(this->KEYPATH); \ +if (ALWAYS_EMIT |

[PATCH] D83698: [clang][cli] Port Target option flags to new option parsing system

Bigcheese accepted this revision. Bigcheese 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/D83698/new/ https://reviews.llvm.org/D83698 ___

[PATCH] D92330: [clang-scan-deps] Improve argument parsing to find target object file path.

Bigcheese accepted this revision. Bigcheese 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/D92330/new/ https://reviews.llvm.org/D92330 __

[PATCH] D102923: [clang][lex] Remark for used header search paths

Bigcheese added a subscriber: akyrtzi. Bigcheese added a comment. I think this is looking good, but would like either @dexonsmith, @akyrtzi, or someone else familiar with this area to take a look. Only other comment I have is that the new functions you add should use the current LLVM naming conv

[PATCH] D94472: [WIP][clang][cli] Command line round-trip for HeaderSearch options

Bigcheese added a comment. In D94472#2519838 , @jansvoboda11 wrote: > In D94472#2508018 , @dexonsmith > wrote: > >> `strict` mode additionally uses the `GeneratedArgs1` to fill >> CompilerInvocation, indirectly c

[PATCH] D95514: [clang][cli] Teach CompilerInvocation to allocate strings on its own

Bigcheese added a comment. In D95514#2528324 , @jansvoboda11 wrote: > In D95514#2526064 , @dexonsmith > wrote: > >> Well, the compiler developers are the users, since `-cc1` isn't for >> end-users. I acknowledge

[PATCH] D95790: [clang][cli] Documentation of CompilerInvocation parsing/generation

Bigcheese added inline comments. Comment at: clang/docs/InternalsManual.rst:711 +required for parsing or generating the command line argument. + +**Positive Flag** You should create a separate section here for listing the classes. Comment at: c

[PATCH] D95790: [clang][cli] Documentation of CompilerInvocation parsing/generation

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. Needs an example in the "Creating new Command Line Option" section, but with that it looks good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D97461: [clang][cli] Implement '-cuid=' marshalling

Bigcheese accepted this revision. Bigcheese 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/D97461/new/ https://reviews.llvm.org/D97461 ___

  1   2   3   >