diff options
author | Jan Svoboda <jan_svoboda@apple.com> | 2025-04-23 10:33:12 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2025-04-23 10:33:12 -0700 |
commit | 060f3f0dd1614b624b527e871019970e4303de11 (patch) | |
tree | b55b9c6a2dba1245f0d8af9636ee80472b8a7802 /clang/lib | |
parent | 1041d54bd4f693c1ac03077680ece67e03c99e22 (diff) | |
download | llvm-060f3f0dd1614b624b527e871019970e4303de11.zip llvm-060f3f0dd1614b624b527e871019970e4303de11.tar.gz llvm-060f3f0dd1614b624b527e871019970e4303de11.tar.bz2 |
[clang][deps] Make dependency directives getter thread-safe (#136178)
This PR fixes two issues in one go:
1. The dependency directives getter (a `std::function`) was being stored
in `PreprocessorOptions`. This goes against the principle where the
options classes are supposed to be value-objects representing the `-cc1`
command line arguments. This is fixed by moving the getter directly to
`CompilerInstance` and propagating it explicitly.
2. The getter was capturing the `ScanInstance` VFS. That's fine in
synchronous implicit module builds where the same VFS instance is used
throughout, but breaks down once you try to build modules asynchronously
(which forces the use of separate VFS instances). This is fixed by
explicitly passing a `FileManager` into the getter and extracting the
right instance of the scanning VFS out of it.
Diffstat (limited to 'clang/lib')
-rw-r--r-- | clang/lib/Frontend/CompilerInstance.cpp | 7 | ||||
-rw-r--r-- | clang/lib/Lex/PPLexerChange.cpp | 14 | ||||
-rw-r--r-- | clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp | 42 |
3 files changed, 42 insertions, 21 deletions
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index de633f0..8596dd0 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -536,6 +536,9 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) { /*ShowAllHeaders=*/true, /*OutputPath=*/"", /*ShowDepth=*/true, /*MSStyle=*/true); } + + if (GetDependencyDirectives) + PP->setDependencyDirectivesGetter(*GetDependencyDirectives); } std::string CompilerInstance::getSpecificModuleCachePath(StringRef ModuleHash) { @@ -1246,6 +1249,10 @@ std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompileImpl( // Make a copy for the new instance. Instance.FailedModules = FailedModules; + if (GetDependencyDirectives) + Instance.GetDependencyDirectives = + GetDependencyDirectives->cloneFor(Instance.getFileManager()); + // If we're collecting module dependencies, we need to share a collector // between all of the module CompilerInstances. Other than that, we don't // want to produce any dependency output from the module build. diff --git a/clang/lib/Lex/PPLexerChange.cpp b/clang/lib/Lex/PPLexerChange.cpp index db6069e3..44b5fa8 100644 --- a/clang/lib/Lex/PPLexerChange.cpp +++ b/clang/lib/Lex/PPLexerChange.cpp @@ -92,16 +92,10 @@ bool Preprocessor::EnterSourceFile(FileID FID, ConstSearchDirIterator CurDir, } Lexer *TheLexer = new Lexer(FID, *InputFile, *this, IsFirstIncludeOfFile); - if (getPreprocessorOpts().DependencyDirectivesForFile && - FID != PredefinesFileID) { - if (OptionalFileEntryRef File = SourceMgr.getFileEntryRefForID(FID)) { - if (std::optional<ArrayRef<dependency_directives_scan::Directive>> - DepDirectives = - getPreprocessorOpts().DependencyDirectivesForFile(*File)) { - TheLexer->DepDirectives = *DepDirectives; - } - } - } + if (GetDependencyDirectives && FID != PredefinesFileID) + if (OptionalFileEntryRef File = SourceMgr.getFileEntryRefForID(FID)) + if (auto MaybeDepDirectives = (*GetDependencyDirectives)(*File)) + TheLexer->DepDirectives = *MaybeDepDirectives; EnterSourceFileWithLexer(TheLexer, CurDir); return false; diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp index b88a7cb..8e05a67 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp @@ -349,6 +349,32 @@ static void canonicalizeDefines(PreprocessorOptions &PPOpts) { std::swap(PPOpts.Macros, NewMacros); } +class ScanningDependencyDirectivesGetter : public DependencyDirectivesGetter { + DependencyScanningWorkerFilesystem *DepFS; + +public: + ScanningDependencyDirectivesGetter(FileManager &FileMgr) : DepFS(nullptr) { + FileMgr.getVirtualFileSystem().visit([&](llvm::vfs::FileSystem &FS) { + auto *DFS = llvm::dyn_cast<DependencyScanningWorkerFilesystem>(&FS); + if (DFS) { + assert(!DepFS && "Found multiple scanning VFSs"); + DepFS = DFS; + } + }); + assert(DepFS && "Did not find scanning VFS"); + } + + std::unique_ptr<DependencyDirectivesGetter> + cloneFor(FileManager &FileMgr) override { + return std::make_unique<ScanningDependencyDirectivesGetter>(FileMgr); + } + + std::optional<ArrayRef<dependency_directives_scan::Directive>> + operator()(FileEntryRef File) override { + return DepFS->getDirectiveTokens(File.getName()); + } +}; + /// A clang tool that runs the preprocessor in a mode that's optimized for /// dependency scanning for the given compiler invocation. class DependencyScanningAction : public tooling::ToolAction { @@ -416,6 +442,9 @@ public: ScanInstance.getInvocation(), ScanInstance.getDiagnostics(), DriverFileMgr->getVirtualFileSystemPtr()); + // Create a new FileManager to match the invocation's FileSystemOptions. + auto *FileMgr = ScanInstance.createFileManager(FS); + // Use the dependency scanning optimized file system if requested to do so. if (DepFS) { StringRef ModulesCachePath = @@ -425,19 +454,10 @@ public: if (!ModulesCachePath.empty()) DepFS->setBypassedPathPrefix(ModulesCachePath); - ScanInstance.getPreprocessorOpts().DependencyDirectivesForFile = - [LocalDepFS = DepFS](FileEntryRef File) - -> std::optional<ArrayRef<dependency_directives_scan::Directive>> { - if (llvm::ErrorOr<EntryRef> Entry = - LocalDepFS->getOrCreateFileSystemEntry(File.getName())) - if (LocalDepFS->ensureDirectiveTokensArePopulated(*Entry)) - return Entry->getDirectiveTokens(); - return std::nullopt; - }; + ScanInstance.setDependencyDirectivesGetter( + std::make_unique<ScanningDependencyDirectivesGetter>(*FileMgr)); } - // Create a new FileManager to match the invocation's FileSystemOptions. - auto *FileMgr = ScanInstance.createFileManager(FS); ScanInstance.createSourceManager(*FileMgr); // Create a collection of stable directories derived from the ScanInstance |