[PATCH] D159064: [Modules] Make clang modules for the C standard library headers

Bigcheese added a comment. This change looks good with the stdint.h comments resolved. Comment at: clang/test/Modules/Inputs/System/usr/include/stdint.h:2 typedef int my_awesome_nonstandard_integer_type; + +/* C99 7.18.1.1 Exact-width integer types. iana wrote

[PATCH] D159483: [Modules] Add a flag to control builtin headers being in system modules

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. The implementation of this change looks good. I share the concern that changing the default may break other users, but that's easily remedied and this is the correct default. It would be

[PATCH] D156234: [clang][deps] add support for dependency scanning with cc1 command line

Bigcheese added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:482 + bool Success = false; + if (FinalCommandLine[1] == "-cc1") { +Success = createAndRunToolInvocation(FinalCommandLine, Action, *FileMgr, cpsu

[PATCH] D151523: [ASTStructuralEquivalence] Fix crash when ObjCCategoryDecl doesn't have corresponding ObjCInterfaceDecl.

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. Looks good as long as people are happy with the condition. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151523/new/ https://reviews.llvm.

[PATCH] D151277: [clang][modules] Mark fewer identifiers as out-of-date

Bigcheese accepted this revision. Bigcheese added a comment. Looks good, although I'm not an expert on this bit either. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151277/new/ https://reviews.llvm.org/D151277

[PATCH] D150689: [clang][DependencyScanner] Remove all warning flags when suppressing warnings

Bigcheese added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:50 + // optimize. + if (!IsSystemModule) +return; ributzka wrote: > This removes also all warnings when building your own module, because of the > `[sy

[PATCH] D150689: [clang][DependencyScanner] Remove all warning flags when suppressing warnings

Bigcheese added inline comments. Comment at: clang/test/ClangScanDeps/optimize-system-warnings.m:19 +// CHECK:], +// CHECK-NEXT: "name": "A" +// CHECK-NEXT: }, jansvoboda11 wrote: > I'd like to see a check line that would fail if the scanner

[PATCH] D150689: [clang][DependencyScanner] Remove all warning flags when suppressing warnings

Bigcheese created this revision. Bigcheese added a reviewer: jansvoboda11. Bigcheese added a project: clang. Herald added a subscriber: ributzka. Herald added a project: All. Bigcheese requested review of this revision. Herald added a subscriber: cfe-commits. Since system modules don't emit most w

[PATCH] D149693: [clang][deps] Make clang-scan-deps write modules in raw format

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/D149693/new/ https://reviews.llvm.org/D149693 ___ cfe-commits mailing list cfe-co

[PATCH] D145197: [clang][deps] NFC: Refactor and comment ModuleDeps sorting

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/D145197/new/ https://reviews.llvm.org/D145197 _

[PATCH] D146328: [clang][deps] Only cache files with specific extension

Bigcheese accepted this revision. Bigcheese added a comment. lgtm, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146328/new/ https://reviews.llvm.org/D146328 ___ cfe-commits mailing list cfe-comm

[PATCH] D146328: [clang][deps] Only cache files with specific extension

Bigcheese added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h:272-291 +enum class Enable { Yes, No }; +enum class ScanFile { Yes, No }; +enum class CacheStatFailure { Yes, No }; + +struct PathPolicy { + unsigned Enabl

[PATCH] D146328: [clang][deps] Only cache files with specific extension

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. This doesn't appear to behave as expected for `PP_CacheFailure` or `PP_ScanFile` without `PP_CacheSuccess`. Looks like that will still cache. Should probably just assert that's not the c

[PATCH] D146255: [clang] Unconditionally add autolink hints for frameworks.

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

[PATCH] D145838: [clang][deps] Handle response files in dep scanner

Bigcheese accepted this revision. Bigcheese added a comment. LTGM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145838/new/ https://reviews.llvm.org/D145838 ___ cfe-commits mailing list cfe-comm

[PATCH] D145526: [clang][DependencyScanner] Cache modulemap stat failures

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG57c7750f703d: [clang][DependencyScanner] Cache modulemap stat failures (authored by Bigcheese). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D145526: [clang][DependencyScanner] Cache modulemap stat failures

Bigcheese updated this revision to Diff 503164. Bigcheese added a comment. Also handle `module.map`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145526/new/ https://reviews.llvm.org/D145526 Files: clang/lib/Tooling/DependencyScanning/Dependenc

[PATCH] D145526: [clang][DependencyScanner] Cache modulemap stat failures

Bigcheese updated this revision to Diff 503160. Bigcheese added a comment. Forgot that `Filename` is actually a path, so call `llvm::sys::path::filename`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145526/new/ https://reviews.llvm.org/D145526 F

[PATCH] D145526: [clang][DependencyScanner] Cache modulemap stat failures

Bigcheese created this revision. Bigcheese added reviewers: ributzka, benlangmuir, jansvoboda11. Bigcheese added a project: clang. Herald added a project: All. Bigcheese requested review of this revision. Herald added a subscriber: cfe-commits. Add `module.modulemap` as a file we cache stat failur

[PATCH] D143040: [Clang][DependencyScanner] Remove secondary actions from -cc1

This revision was automatically updated to reflect the committed changes. Bigcheese marked an inline comment as done. Closed by commit rGffa2a647c6a9: [Clang][DependencyScanner] Remove secondary actions from -cc1 (authored by Bigcheese). Changed prior to commit: https://reviews.llvm.org/D143040

[PATCH] D143040: [Clang][DependencyScanner] Remove secondary actions from -cc1

Bigcheese created this revision. Bigcheese added a reviewer: jansvoboda11. Bigcheese added a project: clang. Herald added a subscriber: ributzka. Herald added a project: All. Bigcheese requested review of this revision. The -arcmt-action= and -objcmt-migrate* actions were being passed to module bu

[PATCH] D143027: [clang][deps] Fix module context hash for constant strings

Bigcheese accepted this revision. Bigcheese added a comment. Thanks for fixing this. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143027/new/ https://reviews.llvm.org/D143027 ___ cfe-commits maili

[PATCH] D141450: [Clang][cc1] Make -fno-modules-local-submodule-visibility the default for ObjC++20

Bigcheese updated this revision to Diff 489970. Bigcheese retitled this revision from "[Clang][cc1] Add -fno-modules-local-submodule-visibility to override the default" to "[Clang][cc1] Make -fno-modules-local-submodule-visibility the default for ObjC++20". Bigcheese edited the summary of this r

[PATCH] D141450: [Clang][cc1] Add -fno-modules-local-submodule-visibility to override the default

Bigcheese added a comment. In D141450#4044680 , @dblaikie wrote: >> This is a problem because some existing ObjectiveC code is not compatible >> with LSV > > Hmm, how is that true? Does this code only build with Clang Header Modules > enabled, and can't

[PATCH] D141450: [Clang][cc1] Add -fno-modules-local-submodule-visibility to override the default

Bigcheese created this revision. Bigcheese added reviewers: jansvoboda11, dblaikie, iains, ChuanqiXu. Bigcheese added projects: clang-modules, clang. Herald added a subscriber: ributzka. Herald added a project: All. Bigcheese requested review of this revision. Currently there is no way to have Obj

[PATCH] D137206: [clang][modules][deps] Including module maps are affecting

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/D137206/new/ https://reviews.llvm.org/D137206 _

[PATCH] D137198: [clang][modules][deps] Parent module maps are affecting

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

[PATCH] D137197: [clang][modules][deps] Transitive module maps are not affecting

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. Yeah, I agree with this. lgtm. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137197/new/ https://reviews.llvm.org/D137197 ___

[PATCH] D137192: [clang][serialization] NFCI: Avoid re-reading input file info

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. lgtm, thanks for fixing this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137192/new/ https://reviews.llvm.org/D137192

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

Bigcheese added a comment. I'm not aware of anyone using this mode, but please wait for responses from Google and Meta people to verify that. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137609/new/ https://reviews.llvm.org/D137609 ___ cfe-

[PATCH] D136007: [clang][modules][deps] System module maps might not be affecting

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. lgtm once D136624 lands. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136007/new/ https://reviews.llvm

[PATCH] D136624: [clang][modules] Account for non-affecting inputs in `ASTWriter`

Bigcheese added a comment. This looks reasonable. Have you measured the performance impact of this change? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136624/new/ https://reviews.llvm.org/D136624 ___ c

[PATCH] D135128: [clang][cli] Simplify repetitive macro invocations

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. Looks good with the suggestion to split the change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135128/new/ https://reviews.llvm.org/D13

[PATCH] D135414: [clang][deps] Don't share in-memory VFS across scans

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/D135414/new/ https://reviews.llvm.org/D135414 _

[PATCH] D135720: [clang][deps] Remove more codegen options

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. lgtm. Although I wonder if long term we should add something to the option tablegen file for this case? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D135416: [clang][deps] Respect VFS overlays in canonical preprocessing mode

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. Ah, I hit the same thing recently, thanks. lgtm. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135416/new/ https://reviews.llvm.org/D1354

[PATCH] D135637: [clang][deps] Prevent emitting diagnostics outside of source file

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/D135637/new/ https://reviews.llvm.org/D135637 _

[PATCH] D135636: [clang][modules][deps] Serialize inputs into PCMs using the "as requested" name

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/D135636/new/ https://reviews.llvm.org/D135636 _

[PATCH] D135381: [clang][modules] Fix handling of `ModuleHeaderRole::ExcludedHeader`

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/D135381/new/ https://reviews.llvm.org/D135381 _

[PATCH] D134248: [clang][modules][deps] Preserve module map load order

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. lgtm Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:129 +// TODO: Track these as `FileEntryRef` to simplify the equality check. +auto A

[PATCH] D134224: [clang][modules][deps] Report modulemaps describing excluded headers

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. lgtm with formatting fixed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134224/new/ https://reviews.llvm.org/D134224 __

[PATCH] D134224: [clang][modules][deps] Report modulemaps describing excluded headers

Bigcheese added a comment. I think this is fine, but I need to think more about the ModuleDepCollector.cpp change. Comment at: clang/lib/Lex/ModuleMap.cpp:1275 // specified module? - (void) Headers[Header.Entry]; + (void) Headers[Header.Entry].push_back(KH); ---

[PATCH] D134222: [clang][deps] Report module map describing compiled module

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

[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] D127182: [Clang][Modules] Merge availability attributes on imported decls

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG169417531578: [Clang][Modules] Merge availability attributes on imported decls (authored by Bigcheese). Repository: rG LLVM Github Monorepo CHANG

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

Bigcheese added a comment. In D127182#3582540 , @jansvoboda11 wrote: > Should we add a FIXME saying we need to handle other kinds of attributes? There already is one where we call `mergeInheritableAttributes`. Repository: rG LLVM Github Monorepo CH

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

Bigcheese created this revision. Bigcheese added reviewers: vsapsai, bruno. Bigcheese added a project: clang-modules. Herald added a project: All. Bigcheese requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Currently we do not in general merg

[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] 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] 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] 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] 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] 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] D118311: [Clang][ModuleMap] Add conditional parsing via requires block declaration

Bigcheese added a comment. > Drive-by comment on the docs; otherwise this sounds awesome; as long as else > is easy to add later this SGTM (I'll let others do the code review). > (Although, if else {} and else requires {} would be easy to add now/soon, I > feel like it'd be worth it. Modelling a

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

Bigcheese updated this revision to Diff 405133. Bigcheese added a comment. Add testing of empty blocks and nested blocks and make the missing `{` error clearer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118311/new/ https://reviews.llvm.org/D11

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

Bigcheese added a comment. > Is it possible to reference external module map from requires block? I mean > that in one context the module has some extra requirements but in a different > context doesn't have them. Can you provide an example where this would cause issues? > It would be nice to

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

Bigcheese added inline comments. Comment at: clang/docs/Modules.rst:651 + +requires cplusplus { + header "vector" iana wrote: > Is there any kind of `else` syntax here? Or do you just use `!whatever` for > the else? Is something like this valid? > > ``

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

Bigcheese created this revision. Bigcheese added reviewers: bruno, dexonsmith, vsapsai. Bigcheese added a project: clang-modules. Bigcheese requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Add a new form of requires called a requires block d

[PATCH] D118150: [clang] Fix serialized diagnostics edge-cases

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/D118150/new/ https://reviews.llvm.org/D118150 _

[PATCH] D114206: [Clang][ScanDeps] Use the virtual path for module maps

Bigcheese added a comment. In D114206#3221893 , @lenary wrote: > Update: I'm going to revert this on main. This patch was accepted > conditionally on you fixing the windows crash, which you have not done, so > this was landed without being accepted. > >

[PATCH] D114206: [Clang][ScanDeps] Use the virtual path for module maps

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG04192422c4e3: [Clang][ScanDeps] Use the virtual path for module maps (authored by Bigcheese). Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D113676: WIP: [clang][lex] Fix search path usage remark with modules

Bigcheese added a comment. Can we not consider a modulemap used when we load an AST that depends on that modulemap? It's possible this breaks if the module only includes textual headers though. It really feels like we should have a single place where we actually know if a module is used or not.

[PATCH] D114206: [Clang][ScanDeps] Use the virtual path for module maps

Bigcheese updated this revision to Diff 391166. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114206/new/ https://reviews.llvm.org/D114206 Files: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp clang/test/ClangScanDeps/modulemap-via-

[PATCH] D114206: [Clang][ScanDeps] Use the virtual path for module maps

Bigcheese added a comment. There were two issues for Windows. The first is an actual bug in the Clang VFS that should be fixed separately. You cannot pass just `main.m` as the input to Clang. Clang asks for its parent directory textually, which is `.`. It then tries to stat `.`, which the VFS

[PATCH] D114206: [Clang][ScanDeps] Use the virtual path for module maps

Bigcheese created this revision. Bigcheese added reviewers: jansvoboda11, dexonsmith. Bigcheese added a project: clang. Bigcheese requested review of this revision. Herald added a subscriber: cfe-commits. This patch makes clang-scan-deps use the virtual path for module maps instead of the on disk

[PATCH] D113880: [clang][modules] Infer framework modules in explicit builds

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. I think this makes sense to do given that you need to explicitly opt into inferred modules anyway. This isn't implicitly finding module maps, as we already have a module map with `framew

[PATCH] D112923: [clang][deps] Reset some benign codegen options

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/D112923/new/ https://reviews.llvm.org/D112923 _

[PATCH] D113391: [Modules] Don't support clang module and c++20 module at the same time

Bigcheese added inline comments. Comment at: clang/test/ClangScanDeps/Inputs/header-search-pruning/cdb.json:3 + "directory" : "DIR", + "command" : "clang -E DIR/header-search-pruning.cpp -Ibegin -I1 -Ia -I3 -I4 -I5 -I6 -Ib -I8 -Iend DEFINES -fmodules -fmodules-cache-path=DIR/m

[PATCH] D111860: [modules] Update visibility for merged ObjCProtocolDecl definitions.

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/D111860/new/ https://reviews.llvm.org/D111860 _

[PATCH] D112289: Support: Use sys::path::system_style() in a few places

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/D112289/new/ https://reviews.llvm.org/D112289 _

[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] 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. 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] D103524: [clang][deps] Handle precompiled headers' AST files

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. lgtm. Comment at: clang/test/ClangScanDeps/modules-pch.c:7 // RUN: %clang -x c-header %t/pch.h -fmodules -gmodules -fimplicit-module-maps \ -// RUN: -fmodules-cache

[PATCH] D103519: [clang][deps] Support object files

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. lgtm Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:161-162 + std::make_unique()); + PCHContainerOps->registerWriter( + std::

[PATCH] D103516: [clang][deps] Customize PCM path via -build-dir argument

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. lgtm with a few minor changes. Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:168-169 +"build-dir", +llvm::cl::desc("With '-generate-modules-path-arg

[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] D100942: [clang][deps] Include "-cc1" in the arguments

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

[PATCH] D100473: [clang] Implement CompilerInvocation copy assignment

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. I think this is fine given that we already have a copy constructor with deep copy semantics. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D100455: [clang] Rename CompilerInvocationBase to RefBase, split out ValueBase

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/D100455/new/ https://reviews.llvm.org/D100455 _

[PATCH] D100653: [clang][cli] NFC: Move conditional LangOptions parsing/generation

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/D100653/new/ https://reviews.llvm.org/D100653 _

[PATCH] D100533: [clang][deps] Remove the -full-command-line flag

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/D100533/new/ https://reviews.llvm.org/D100533 _

[PATCH] D98950: [clang][deps] NFC: Document collector, rename members

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. Thanks for the cleanup. Code makes more sense now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98950/new/ https://reviews.llvm.org/D9895

[PATCH] D97552: [clang][cli] Fix generation of '-fvisibility' with regards to '-mignore-xcoff-visibility'

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

[PATCH] D97702: [clang][modules] Use extensible RTTI for ModuleFileExtension

Bigcheese added a comment. Looks fine to me, but it would be good for a Swift person to look at this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97702/new/ https://reviews.llvm.org/D97702 ___ cf

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

[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] 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] 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] 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] D94682: [clang][cli] Parse Lang and CodeGen options separately

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/D94682/new/ https://reviews.llvm.org/D94682 __

[PATCH] D94681: [clang][cli] NFC: Promote ParseLangArgs and ParseCodeGenArgs to members

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/D94681/new/ https://reviews.llvm.org/D94681 __

[PATCH] D94680: [clang][cli] NFC: Parse some LangOpts after the defaults are set

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/D94680/new/ https://reviews.llvm.org/D94680 __

[PATCH] D94679: [clang][cli] NFC: Add PIE parsing for precompiled input and IR

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/D94679/new/ https://reviews.llvm.org/D94679 __

[PATCH] D94678: [clang][cli] Parse & generate options necessary for LangOptions defaults manually

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:5210-5213 def finclude_default_header : Flag<["-"], "finclude-default-header">, - HelpText<"I

[PATCH] D94676: [clang][cli] Specify KeyPath prefixes via TableGen classes

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/D94676/new/ https://reviews.llvm.org/D94676 __

[PATCH] D94675: [clang][cli] NFC: Decrease the scope of ParseCodeGenArgs parameters

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/D94675/new/ https://reviews.llvm.org/D94675 __

[PATCH] D94674: [clang][cli] NFC: Decrease the scope of ParseLangArgs parameters

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/D94674/new/ https://reviews.llvm.org/D94674 __

[PATCH] D84674: [clang][cli] Allow users to specify a conditional to prevent parsing options with MarshallingInfo

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

[PATCH] D93636: [clang][cli] Implement ContainsN Google Test matcher

Bigcheese accepted this revision. Bigcheese added a comment. This revision is now accepted and ready to land. lgtm. We don't really have anywhere specific to put extra matchers, and we don't want to add stuff to the actual gtest folder so it's easy to merge updates. So this is fine. Repositor

  1   2   3   >