[PATCH] D131934: [clang][deps] Compute command-lines for dependencies immediately

benlangmuir updated this revision to Diff 453085. benlangmuir edited the summary of this revision. benlangmuir added a comment. Per review - Add typedef for callback type - Add comment about mapping paths to "-" - Update commit message for module cache path computation change CHANGES SINCE LAST

[PATCH] D131934: [clang][deps] Compute command-lines for dependencies immediately

benlangmuir marked 2 inline comments as done. benlangmuir added inline comments. Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:403 + +static std::string getModuleCachePath(ArrayRef Args) { + for (StringRef Arg : llvm::reverse(Args)) { jansvoboda11 wr

[PATCH] D131934: [clang][deps] Compute command-lines for dependencies immediately

This revision was automatically updated to reflect the committed changes. Closed by commit rG5482432bf6cc: [clang][deps] Compute command-lines for dependencies immediately (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D13193

[PATCH] D132066: [clang][deps] Allow switching between lazily/eagerly loaded PCMs

benlangmuir added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h:50 +bool OptimizeArgs = false, +bool EagerLoadModules = false); Since you're chang

[PATCH] D132066: [clang][deps] Allow switching between lazily/eagerly loaded PCMs

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. Thanks, LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132066/new/ https://reviews.llvm.org/D132066 _

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, Bigcheese. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Instead of trying to "fix" the original driver invocation by a

[PATCH] D131796: [clang][deps] Use a cc1 invocation in FullDependencies::getCommandLine()

benlangmuir abandoned this revision. benlangmuir added a comment. Closing in favour of https://reviews.llvm.org/D132405 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131796/new/ https://reviews.llvm.org/D131796

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir added inline comments. Herald added a subscriber: ormris. Comment at: clang/test/ClangScanDeps/diagnostics.c:31 // CHECK-NEXT: { -// CHECK-NEXT: "clang-context-hash": "[[HASH_TU:.*]], +// CHECK:"clang-context-hash": "[[HASH_TU:.*]], // CHECK-NE

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:34 +/// \see FullDependencies::Commands. +class Command { +public: jansvoboda11 wrote: > Have you considered using the `Job`/`Command` classes

[PATCH] D132419: [clang] Pull some utility functions into CompilerInvocation NFC

benlangmuir created this revision. benlangmuir added a reviewer: jansvoboda11. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Move copying compiler arguments to a vector and modifying common module-

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h:102 +std::vector +serializeCompilerInvocation(const CompilerInvocation &CI); + jansvoboda11 wrote: > Maybe we could add this API directly to

[PATCH] D132419: [clang] Pull some utility functions into CompilerInvocation NFC

This revision was automatically updated to reflect the committed changes. Closed by commit rG3708a148421f: [clang] Pull some utility functions into CompilerInvocation NFC (authored by benlangmuir). Changed prior to commit: https://reviews.llvm.org/D132419?vs=454639&id=454847#toc Repository:

[PATCH] D132430: [clang][modules] Track affecting modules

benlangmuir added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:311 +if (!MDC.isPrebuiltModule(M)) + DirectModularDeps.insert(M); + If using eager loading, this will cause us to load the module, right? Does that

[PATCH] D132430: [clang][modules] Track affecting modules

benlangmuir added inline comments. Comment at: clang/test/ClangScanDeps/modules-incomplete-umbrella.c:82 +// CHECK_TU-NEXT: "command-line": [ +// CHECK_TU:], +// CHECK_TU-NEXT: "file-deps": [ jansvoboda11 wrote: > benlangmuir wrote: > > Th

[PATCH] D132502: [clang][modules] Consider M affecting after mapping M.Private to M_Private

benlangmuir added inline comments. Comment at: clang/lib/Frontend/CompilerInstance.cpp:2017 // migrate off of Foo.Private syntax. -if (!Sub && PP->getLangOpts().ImplicitModules && Name == "Private" && -Module == Module->getTopLevelModule()) { +if (!Sub && Nam

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir updated this revision to Diff 455315. benlangmuir added a comment. - Remove CompilerInvocation from Command and ModuleDeps. Only the arg strings are exposed now. - Simplify Command, which is now just a simple struct. - Move the logic for mutating the CompilerInvocation for the TU into

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir marked 11 inline comments as done. benlangmuir added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:44 + + enum CommandKind { +CK_CC1, tschuett wrote: > Why is this not an enum class? Removed

[PATCH] D132502: [clang][modules] Consider M affecting after mapping M.Private to M_Private

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/Frontend/CompilerInstance.cpp:2017 // migrate off of Foo.Private syntax. -if (!Sub && PP->getLangOpts().ImplicitModules && Name

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir updated this revision to Diff 455338. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132405/new/ https://reviews.llvm.org/D132405 Files: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h clang/include/clang/Tooling/DependencyScanning/DependencyScannin

[PATCH] D132615: [clang][tooling] Allow -cc1 arguments in ToolInvocation

benlangmuir created this revision. benlangmuir added a reviewer: jansvoboda11. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. ToolInvocation is useful even if you already have a -cc1 invocation, sin

[PATCH] D132616: [clang][deps] Remove CompilerInvocation from ModuleDeps

benlangmuir created this revision. benlangmuir added a reviewer: jansvoboda11. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The invocation is only ever used to serialize cc1 arguments from, so ins

[PATCH] D132617: [clang][deps] Minor ModuleDepCollector refactorings NFC

benlangmuir created this revision. benlangmuir added a reviewer: jansvoboda11. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. - Factor module map and module file path functions out - Use a secondary

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir added a comment. In D132405#3747232 , @jansvoboda11 wrote: > I'd like to see this split into multiple patches. I can see some formatting > changes, removal of `CompilerInvocation` from `ModuleDeps`, isolated changes > to `Tooling`, etc. Tha

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h:224 + /// a preprocessor. Storage owned by \c ModularDeps. + llvm::StringMap ModuleDepsByID; + /// Directly imported prebuilt deps. jansvoboda11

[PATCH] D132615: [clang][tooling] Allow -cc1 arguments in ToolInvocation

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbdc20d61b8e4: [clang][tooling] Allow -cc1 arguments in ToolInvocation (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D132616: [clang][deps] Remove CompilerInvocation from ModuleDeps

This revision was automatically updated to reflect the committed changes. Closed by commit rGe8febb23a07b: [clang][deps] Remove CompilerInvocation from ModuleDeps (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132616/new/ h

[PATCH] D132617: [clang][deps] Minor ModuleDepCollector refactorings NFC

benlangmuir updated this revision to Diff 455468. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132617/new/ https://reviews.llvm.org/D132617 Files: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp Index:

[PATCH] D132617: [clang][deps] Minor ModuleDepCollector refactorings NFC

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc0a55121618b: [clang][deps] Minor ModuleDepCollector refactorings NFC (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir updated this revision to Diff 455666. benlangmuir added a comment. - Rebased, picking up the changes that were previously split out. - Broke up applying changes to CI from passing to consumer. The MDC is no longer responsible for passing invocations to the consumer, and it is done at

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir added inline comments. Comment at: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h:204 + /// invocation, for example when there are multiple chained invocations. + void handleInvocation(CompilerInvocation CI); + jansvoboda11 wro

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir updated this revision to Diff 455712. benlangmuir added a comment. - handleBuildCommand now takes a Command - Removed now-unused forward decl - Expanded comment about scan-once behaviour - Moved all calls to handleBuildCommand to a single place - Added comments to new tests CHANGES S

[PATCH] D132405: [clang][deps] Split translation units into individual -cc1 or other commands

benlangmuir marked 6 inline comments as done. benlangmuir added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:165 +if (Scanned) { + // If we have already scanned an upstream command, just forward to the

[PATCH] D133988: [clang][deps] Make sure ScanInstance outlives collector

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. Good catch, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133988/new/ https://reviews.llvm.org/D133988 __

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

benlangmuir added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:194 +const FileEntry *CurrentModMap = +PP.getHeaderSearchInfo().getModuleMap().getModuleMapFileForUniquing( +CurrentModule); I think

[PATCH] D134923: [clang][deps] Canonicalize module map path

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, Bigcheese. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When dep-scanning, canonicalize the module map path as much as

[PATCH] D134923: [clang][deps] Canonicalize module map path

benlangmuir added a comment. In D134923#3825673 , @jansvoboda11 wrote: > IIUC this not only canonicalizes path to the main input file when compiling a > PCM, but also `-fmodule-map-file=` arguments for dependencies, correct? Since > that behavior is de

[PATCH] D134923: [clang][deps] Canonicalize module map path

benlangmuir updated this revision to Diff 464104. benlangmuir added a comment. Test -fmodule-map-file paths as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134923/new/ https://reviews.llvm.org/D134923 Files: clang/include/clang/Lex/ModuleMap.h clang/lib/Lex/HeaderSearch.cpp

[PATCH] D134923: [clang][deps] Canonicalize module map path

benlangmuir updated this revision to Diff 464109. benlangmuir added a comment. Fix windows path separators using sed instead of regex "." CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134923/new/ https://reviews.llvm.org/D134923 Files: clang/include/clang/Lex/ModuleMap.h clang/lib/

[PATCH] D134923: [clang][deps] Canonicalize module map path

benlangmuir updated this revision to Diff 464117. benlangmuir added a comment. Simplify modmap checks in test to use PREFIX CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134923/new/ https://reviews.llvm.org/D134923 Files: clang/include/clang/Lex/ModuleMap.h clang/lib/Lex/HeaderSear

[PATCH] D134923: [clang][deps] Canonicalize module map path

benlangmuir updated this revision to Diff 464313. benlangmuir added a comment. Canonicalize path separator(s) in between the (already canonicalized directory) and the filename. This could cause issues with redundant path separators (`foo//bar`) or non-native separators (`foo/bar` vs `foo\bar` on

[PATCH] D134976: [clang][deps] Abolish FileManager sharing

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. Thanks for cleaning this up! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134976/new/ https://reviews.llvm.org/D134976 _

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

benlangmuir added inline comments. Comment at: clang/include/clang/AST/DeclObjC.h:1091 virtual void collectPropertiesToImplement(PropertyMap &PM, PropertyDeclOrder &PO) const {} Can we use the existing `PropertyDe

[PATCH] D135220: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, bnbarham. Herald added a subscriber: arphaman. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Update `SourceManager::Con

[PATCH] D135220: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef

benlangmuir updated this revision to Diff 465457. benlangmuir edited the summary of this revision. benlangmuir added a comment. - Used `OptionalFileEntryRefDegradesToFileEntryPtr` at two callsites - Attempted to fix Windows path / vs \ in test. CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D135220: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef

benlangmuir added inline comments. Comment at: clang/include/clang/Basic/SourceManager.h:146-155 /// References the file which the contents were actually loaded from. /// /// Can be different from 'Entry' if we overridden the contents of one file /// with the contents

[PATCH] D135220: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef

This revision was automatically updated to reflect the committed changes. Closed by commit rG5ea78c4113f8: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135220

[PATCH] D134923: [clang][deps] Canonicalize module map path

benlangmuir updated this revision to Diff 465544. benlangmuir added a comment. Rebased after landing https://reviews.llvm.org/D135220, which should hopefully fix the Windows test failure (it fixed the path for me locally on Windows, but I had some python-related issues that prevented me from run

[PATCH] D134923: [clang][deps] Canonicalize module map path

This revision was automatically updated to reflect the committed changes. Closed by commit rG074fcec1eabf: [clang][deps] Canonicalize module map path (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134923/new/ https://review

[PATCH] D135220: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef

benlangmuir added a comment. > That change might be problematic for content addressing storages. E.g. > clang/test/Driver/cl-pch-showincludes.cpp started to fail in my setup as > clang/test/Driver/header{0,1,3,4}.h are all identical and can be symlinked What is the failure you're seeing? I wou

[PATCH] D135373: [clang][test] Make headers unique to avoid linking issues

benlangmuir created this revision. benlangmuir added reviewers: goncharov, dexonsmith. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Make the empty headers used by cl-pch-showincludes.cpp unique so

[PATCH] D135220: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef

benlangmuir added a comment. Okay, I was able to reproduce by symlinking all the 0-byte header files to header0 (any choice probably works). The behaviour is deterministic before and after my change. This was only passing by luck in this setup, because it was relying on mutation of `FileEntry

[PATCH] D135220: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef

benlangmuir added a comment. > I was mostly trying to confirm that symlink setups will be fine I don't know if we promise this is supported anywhere, but I know we've made this kind of test change to force unique files several times in the past. Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D135373: [clang][test] Make headers unique to avoid linking issues

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8d9a3a6b9bd1: [clang][test] Make headers unique to avoid linking issues (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

benlangmuir accepted this revision. benlangmuir added a comment. Re-added a few of my minor comments that I think got lost on a specific version of the diff. Otherwise LGTM. Comment at: llvm/include/llvm/Support/StatCacheFileSystem.h:23 +/// A ProxyFileSystem using cached info

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

benlangmuir added a comment. > Sorry for missing these. I think I addressed all of them now. Thanks, LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136651/new/ https://reviews.llvm.org/D136651 ___

[PATCH] D137303: [clang] Move accessors for OrigEntry and Filename to FileInfo NFC

benlangmuir created this revision. Herald added subscribers: arphaman, martong. Herald added a reviewer: shafik. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Refactor ContentCache::OrigEntry and F

[PATCH] D137304: [clang] Store filename per include instead of mutating filename

benlangmuir created this revision. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When including a header, store the filename per include (per FileInfo) rather than storing it once in the ContentCac

[PATCH] D137304: [clang] Store filename per include instead of mutating filename

benlangmuir added a comment. Thanks for the timely review! > the DenseMaps that need this unique-by-FileEntry* behaviour should probably > be the ones using a custom DenseMapInfo. Yeah, that's the way I'm currently leaning. Make the default for `DenseMap` and `==` be path equality and have a

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

benlangmuir added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:447 +SmallString<128> CanonicalPath = FE.getNameAsRequested(); +ModMapInfo.canonicalizeModuleMapPath(CanonicalPath); +MD.ModuleMapFileDeps.emplace_ba

[PATCH] D137989: [clang][deps] Avoid leaking modulemap paths across unrelated imports

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, Bigcheese. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Use a FileEntryRef when retrieving modulemap paths in the scan

[PATCH] D137989: [clang][deps] Avoid leaking modulemap paths across unrelated imports

benlangmuir added inline comments. Comment at: clang/lib/Serialization/ASTWriter.cpp:1561 else // FIXME: The path should be taken from the FileEntryRef. PP->Diag(SourceLocation(), diag::err_module_unable_to_hash_content) jansvoboda11 wrot

[PATCH] D137989: [clang][deps] Avoid leaking modulemap paths across unrelated imports

benlangmuir updated this revision to Diff 475534. benlangmuir added a comment. - Removed outdated FIXME - Ran clang-format - Attempt to fix Windows test failure: the `StringSet` of paths was failing to match, so switch to `DenseSet`. Note: the path that ends up in the command-line is from a `Fil

[PATCH] D137989: [clang][deps] Avoid leaking modulemap paths across unrelated imports

This revision was automatically updated to reflect the committed changes. Closed by commit rG05ec16d90deb: [clang][deps] Avoid leaking modulemap paths across unrelated imports (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

benlangmuir requested changes to this revision. benlangmuir added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Driver/Options.td:3374 +def ivfsstatcache : JoinedOrSeparate<["-"], "ivfsstatcache">, Group, Flags<[CC1Option]>, + H

[PATCH] D138160: [clang] Use InMemoryModuleCache for readASTFileControlBlock NFC

benlangmuir created this revision. benlangmuir added a reviewer: jansvoboda11. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When a pcm has already been loaded from disk, reuse it from the InMemor

[PATCH] D138160: [clang] Use InMemoryModuleCache for readASTFileControlBlock NFC

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc4436f675d8f: [clang] Use InMemoryModuleCache for readASTFileControlBlock NFC (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHAN

[PATCH] D142392: [clang][deps] Add module files for input dependencies earlier

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG43854fa263d2: [clang][deps] Add module files for input dependencies earlier (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D142416: [clang][deps] NFC: Remove dead code

benlangmuir accepted this revision. benlangmuir added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:180 - // incompatible modules (e.g. with differences in search paths). - CI.getHeaderS

[PATCH] D142501: [clang][deps] Fix modulemap file path for implementation of module

benlangmuir created this revision. benlangmuir added a reviewer: jansvoboda11. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Use the name "as requested" for the path of the implemented module's mod

[PATCH] D142501: [clang][deps] Fix modulemap file path for implementation of module

benlangmuir updated this revision to Diff 491886. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142501/new/ https://reviews.llvm.org/D142501 Files: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp clang/test/ClangScanDeps/modules-implementation-vfs.m Index: clang/test/Cla

[PATCH] D142501: [clang][deps] Fix modulemap file path for implementation of module

benlangmuir marked an inline comment as done. benlangmuir added inline comments. Comment at: clang/test/ClangScanDeps/modules-implementation-vfs.m:67-68 +#import +struct A { float x; }; // expected-error {{incompatible definitions}} expected-note {{type 'float' here}} +// expec

[PATCH] D142511: [clang][test] Remove check that fails if SOURCE_DATE_EPOCH is set globally

benlangmuir created this revision. benlangmuir added a reviewer: tstellar. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The check for "no SOURCE_DATE_EPOCH" wasn't especially interesting, and I am

[PATCH] D136717: [clang] Move getenv call for SOURCE_DATE_EPOCH out of frontend NFC

benlangmuir added inline comments. Comment at: clang/test/Driver/SOURCE_DATE_EPOCH.c:2 +// RUN: %clang -E %s -### 2>&1 | FileCheck %s -check-prefix=NO_EPOCH +// NO_EPOCH-NOT: "-source-date-epoch" + tstellar wrote: > Hi @benlangmuir, this test fails in our build e

[PATCH] D142511: [clang][test] Remove check that fails if SOURCE_DATE_EPOCH is set globally

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0245dcc000fa: [clang][test] Remove check that fails if SOURCE_DATE_EPOCH is set globally (authored by benlangmuir). Repository: rG LLVM Github Mon

[PATCH] D142501: [clang][deps] Fix modulemap file path for implementation of module

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. benlangmuir marked an inline comment as done. Closed by commit rG73dba2f649a8: [clang][deps] Fix modulemap file path for implementation of module (authored by benlangmu

[PATCH] D142780: [clang] NFCI: Use FileEntryRef in FileManager's UID mapping

benlangmuir added a comment. Iterating over `SeenFileEntries` and skipping `VirtualFileEntries` looks good to me. I'm not sure about changing this to FileEntryRef though. The way this API works you only get one per unique file, which is well suited to `FileEntry *` which has the same uniquing

[PATCH] D142780: [clang] NFCI: Use FileEntryRef in FileManager's UID mapping

benlangmuir added a comment. In D142780#4087250 , @rmaz wrote: > In that case we are seeing non-deterministic header paths serialized in pcm > files. IIUC the header files are serialized based in their unique ID, so it > wouldn't be possible to handle t

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

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, Bigcheese. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We were not hashing constant strings in the command-line, only

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG223e99fb698d: [clang][deps] Fix module context hash for constant strings (authored by benlangmuir). Changed prior to commit: https://reviews.llvm.

[PATCH] D141644: [clang] Report the on-disk paths for inputs to module compiles

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. Please add a comment to each place you're doing this since it's non-obvious why the filename would be different to people not familiar with the mapping code. Otherwise LGTM Reposi

[PATCH] D142143: [Lex] For dependency directive lexing, angled includes in `__has_include` should be lexed as string literals

benlangmuir added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:4415 +if (Result.isNot(tok::header_name)) + return true; +// Advance the index of lexed tokens. This case is missing a test I think. Comment at: clang/lib/Lex/L

[PATCH] D142392: [clang][deps] Add module files for input dependencies earlier

benlangmuir created this revision. benlangmuir added a reviewer: jansvoboda11. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. I originally thought we needed to add module file inputs for modular dep

[PATCH] D142416: [clang][deps] NFC: Remove dead code

benlangmuir added a comment. Removing `ImplicitModulePCMPath` LGTM; not sure about the other change. Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:180 - // incompatible modules (e.g. with differences in search paths). - CI.getHeaderSearchOpts().Modul

[PATCH] D127379: [Lex] Keep track of skipped preprocessor blocks and advance the lexer directly if they are revisited

benlangmuir added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:514 + BeginPtr = PP.CurLexer->getBufferLocation(); + SkipRangePtr = &PP.RecordedSkippedRanges[BeginPtr]; + if (*SkipRangePtr) { Storing this pointer is only safe as long

[PATCH] D127379: [Lex] Keep track of skipped preprocessor blocks and advance the lexer directly if they are revisited

benlangmuir accepted this revision. benlangmuir added a comment. This revision is now accepted and ready to land. Couple more minor things, but basically LGTM. Comment at: clang/include/clang/Lex/Preprocessor.h:2601 + /// This is used to guard against calling this function rec

[PATCH] D127883: [clang][deps] Further canonicalize implicit modules options in dep scan

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, Bigcheese. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Disable or canonicalize compiler options that are not relevant

[PATCH] D127883: [clang][deps] Further canonicalize implicit modules options in dep scan

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG509223da6114: [clang][deps] Further canonicalize implicit modules options in dep scan (authored by benlangmuir). Repository: rG LLVM Github Monore

[PATCH] D128008: [clang][deps] Sort submodules when calculating dependencies

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, Bigcheese. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Dependency scanning does not care about the order of submodule

[PATCH] D128008: [clang][deps] Sort submodules when calculating dependencies

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4a3a9a5fa0b2: [clang][deps] Sort submodules when calculating dependencies (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D127229: [clang][deps] Set -disable-free for module compilations

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, Bigcheese. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The command-line arguments for module builds are cc1 commands,

[PATCH] D127243: [clang][deps] Make order of module dependencies deterministic

benlangmuir created this revision. benlangmuir added reviewers: jansvoboda11, Bigcheese. Herald added a subscriber: mgrang. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This fixes the underlying m

[PATCH] D127229: [clang][deps] Set -disable-free for module compilations

benlangmuir updated this revision to Diff 434916. benlangmuir added a comment. Attempt to fix Windows path issue in test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127229/new/ https://reviews.llvm.org/D127229 Files: clang/lib/Tooling/Depende

[PATCH] D127243: [clang][deps] Make order of module dependencies deterministic

benlangmuir added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp:181 std::vector PrebuiltModuleDeps; -std::map ClangModuleDeps; +llvm::MapVector, +llvm::StringMap> jansvoboda11 wrote:

[PATCH] D127243: [clang][deps] Make order of module dependencies deterministic

benlangmuir updated this revision to Diff 435237. benlangmuir added a comment. Removed use of std::unique_ptr in DependencyScanningTool.cpp, per review feedback. @jansvoboda11 Note: there is another map containing `std::unique_ptr` in `ModuleDepCollector`, but that one is required for correct

[PATCH] D127229: [clang][deps] Set -disable-free for module compilations

benlangmuir added inline comments. Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:338 +// always true for a driver invocation. +bool DisableFree = true; DependencyScanningActi

[PATCH] D127229: [clang][deps] Set -disable-free for module compilations

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7a72dca74a27: [clang][deps] Set -disable-free for module compilations (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D127243: [clang][deps] Make order of module dependencies deterministic

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG835fcf2aa512: [clang][deps] Make order of module dependencies deterministic (authored by benlangmuir). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D154130: [lit][clang] Avoid realpath on Windows due to MAX_PATH limitations

benlangmuir added inline comments. Comment at: clang/lib/Basic/FileManager.cpp:655 + SmallString<4096> AbsPathBuf = Name; + SmallString<4096> RealPathBuf; + if (!FS->makeAbsolute(AbsPathBuf)) { 8k is a lot of stack space. The only reason this was 4k in the fir

[PATCH] D154130: [lit][clang] Avoid realpath on Windows due to MAX_PATH limitations

benlangmuir added inline comments. Comment at: clang/lib/Basic/FileManager.cpp:663 +} else { + llvm::sys::path::remove_dots(AbsPathBuf, /*remove_dot_dot=*/true); + CanonicalName = AbsPathBuf.str().copy(CanonicalNameStorage); MrTrillian wrote: > benl

[PATCH] D158021: [clang][modules] Mark builtin header 'inttypes.h' for modules

benlangmuir created this revision. benlangmuir added reviewers: iana, jansvoboda11. Herald added a project: All. benlangmuir requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Like the other enumerated builtin headers, inttypes.h can be declar

<    1   2   3   4   >