[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.
diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h
index 6007d56..8c91a2a 100644
--- a/clang/include/clang/Frontend/CompilerInstance.h
+++ b/clang/include/clang/Frontend/CompilerInstance.h
@@ -16,6 +16,7 @@
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PCHContainerOperations.h"
#include "clang/Frontend/Utils.h"
+#include "clang/Lex/DependencyDirectivesScanner.h"
#include "clang/Lex/HeaderSearchOptions.h"
#include "clang/Lex/ModuleLoader.h"
#include "llvm/ADT/ArrayRef.h"
@@ -99,6 +100,9 @@
/// The cache of PCM files.
IntrusiveRefCntPtr<ModuleCache> ModCache;
+ /// Functor for getting the dependency preprocessor directives of a file.
+ std::unique_ptr<DependencyDirectivesGetter> GetDependencyDirectives;
+
/// The preprocessor.
std::shared_ptr<Preprocessor> PP;
@@ -697,6 +701,11 @@
/// and replace any existing one with it.
void createPreprocessor(TranslationUnitKind TUKind);
+ void setDependencyDirectivesGetter(
+ std::unique_ptr<DependencyDirectivesGetter> Getter) {
+ GetDependencyDirectives = std::move(Getter);
+ }
+
std::string getSpecificModuleCachePath(StringRef ModuleHash);
std::string getSpecificModuleCachePath() {
return getSpecificModuleCachePath(getInvocation().getModuleHash());
diff --git a/clang/include/clang/Lex/DependencyDirectivesScanner.h b/clang/include/clang/Lex/DependencyDirectivesScanner.h
index 0e11590..acdc9e2 100644
--- a/clang/include/clang/Lex/DependencyDirectivesScanner.h
+++ b/clang/include/clang/Lex/DependencyDirectivesScanner.h
@@ -21,6 +21,7 @@
#include "llvm/ADT/ArrayRef.h"
namespace clang {
+class FileManager;
namespace tok {
enum TokenKind : unsigned short;
@@ -135,6 +136,19 @@
ArrayRef<dependency_directives_scan::Directive> Directives,
llvm::raw_ostream &OS);
+/// Functor that returns the dependency directives for a given file.
+class DependencyDirectivesGetter {
+public:
+ /// Clone the getter for a new \c FileManager instance.
+ virtual std::unique_ptr<DependencyDirectivesGetter>
+ cloneFor(FileManager &FileMgr) = 0;
+
+ /// Get the dependency directives for the given file.
+ virtual std::optional<ArrayRef<dependency_directives_scan::Directive>>
+ operator()(FileEntryRef File) = 0;
+
+ virtual ~DependencyDirectivesGetter() = default;
+};
} // end namespace clang
#endif // LLVM_CLANG_LEX_DEPENDENCYDIRECTIVESSCANNER_H
diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index 10260c6..f2dfd3a 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -140,6 +140,12 @@
friend class VariadicMacroScopeGuard;
llvm::unique_function<void(const clang::Token &)> OnToken;
+ /// Functor for getting the dependency preprocessor directives of a file.
+ ///
+ /// These are directives derived from a special form of lexing where the
+ /// source input is scanned for the preprocessor directives that might have an
+ /// effect on the dependencies for a compilation unit.
+ DependencyDirectivesGetter *GetDependencyDirectives = nullptr;
const PreprocessorOptions &PPOpts;
DiagnosticsEngine *Diags;
const LangOptions &LangOpts;
@@ -1326,6 +1332,10 @@
OnToken = std::move(F);
}
+ void setDependencyDirectivesGetter(DependencyDirectivesGetter &Get) {
+ GetDependencyDirectives = &Get;
+ }
+
void setPreprocessToken(bool Preprocess) { PreprocessToken = Preprocess; }
bool isMacroDefined(StringRef Id) {
diff --git a/clang/include/clang/Lex/PreprocessorOptions.h b/clang/include/clang/Lex/PreprocessorOptions.h
index c2e3d68..d4c4e1c 100644
--- a/clang/include/clang/Lex/PreprocessorOptions.h
+++ b/clang/include/clang/Lex/PreprocessorOptions.h
@@ -189,19 +189,6 @@
/// with support for lifetime-qualified pointers.
ObjCXXARCStandardLibraryKind ObjCXXARCStandardLibrary = ARCXX_nolib;
- /// Function for getting the dependency preprocessor directives of a file.
- ///
- /// These are directives derived from a special form of lexing where the
- /// source input is scanned for the preprocessor directives that might have an
- /// effect on the dependencies for a compilation unit.
- ///
- /// Enables a client to cache the directives for a file and provide them
- /// across multiple compiler invocations.
- /// FIXME: Allow returning an error.
- std::function<std::optional<ArrayRef<dependency_directives_scan::Directive>>(
- FileEntryRef)>
- DependencyDirectivesForFile;
-
/// Set up preprocessor for RunAnalysis action.
bool SetUpStaticAnalyzer = false;
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
index 74b40f7..a20a89a 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -379,6 +379,16 @@
/// false if not (i.e. this entry is not a file or its scan fails).
bool ensureDirectiveTokensArePopulated(EntryRef Entry);
+ /// \returns The scanned preprocessor directive tokens of the file that are
+ /// used to speed up preprocessing, if available.
+ std::optional<ArrayRef<dependency_directives_scan::Directive>>
+ getDirectiveTokens(const Twine &Path) {
+ if (llvm::ErrorOr<EntryRef> Entry = getOrCreateFileSystemEntry(Path.str()))
+ if (ensureDirectiveTokensArePopulated(*Entry))
+ return Entry->getDirectiveTokens();
+ return std::nullopt;
+ }
+
/// Check whether \p Path exists. By default checks cached result of \c
/// status(), and falls back on FS if unable to do so.
bool exists(const Twine &Path) override;
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 @@
/*ShowAllHeaders=*/true, /*OutputPath=*/"",
/*ShowDepth=*/true, /*MSStyle=*/true);
}
+
+ if (GetDependencyDirectives)
+ PP->setDependencyDirectivesGetter(*GetDependencyDirectives);
}
std::string CompilerInstance::getSpecificModuleCachePath(StringRef ModuleHash) {
@@ -1246,6 +1249,10 @@
// 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 db6069e..44b5fa8 100644
--- a/clang/lib/Lex/PPLexerChange.cpp
+++ b/clang/lib/Lex/PPLexerChange.cpp
@@ -92,16 +92,10 @@
}
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 @@
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 @@
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 @@
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
diff --git a/clang/unittests/Lex/PPDependencyDirectivesTest.cpp b/clang/unittests/Lex/PPDependencyDirectivesTest.cpp
index 03f1432..6ab80ba 100644
--- a/clang/unittests/Lex/PPDependencyDirectivesTest.cpp
+++ b/clang/unittests/Lex/PPDependencyDirectivesTest.cpp
@@ -103,25 +103,33 @@
SmallVector<dependency_directives_scan::Token> Tokens;
SmallVector<dependency_directives_scan::Directive> Directives;
};
- SmallVector<std::unique_ptr<DepDirectives>> DepDirectivesObjects;
- auto getDependencyDirectives = [&](FileEntryRef File)
- -> std::optional<ArrayRef<dependency_directives_scan::Directive>> {
- DepDirectivesObjects.push_back(std::make_unique<DepDirectives>());
- StringRef Input = (*FileMgr.getBufferForFile(File))->getBuffer();
- bool Err = scanSourceForDependencyDirectives(
- Input, DepDirectivesObjects.back()->Tokens,
- DepDirectivesObjects.back()->Directives);
- EXPECT_FALSE(Err);
- return llvm::ArrayRef(DepDirectivesObjects.back()->Directives);
+ class TestDependencyDirectivesGetter : public DependencyDirectivesGetter {
+ FileManager &FileMgr;
+ SmallVector<std::unique_ptr<DepDirectives>> DepDirectivesObjects;
+
+ public:
+ TestDependencyDirectivesGetter(FileManager &FileMgr) : FileMgr(FileMgr) {}
+
+ std::unique_ptr<DependencyDirectivesGetter>
+ cloneFor(FileManager &FileMgr) override {
+ return std::make_unique<TestDependencyDirectivesGetter>(FileMgr);
+ }
+
+ std::optional<ArrayRef<dependency_directives_scan::Directive>>
+ operator()(FileEntryRef File) override {
+ DepDirectivesObjects.push_back(std::make_unique<DepDirectives>());
+ StringRef Input = (*FileMgr.getBufferForFile(File))->getBuffer();
+ bool Err = scanSourceForDependencyDirectives(
+ Input, DepDirectivesObjects.back()->Tokens,
+ DepDirectivesObjects.back()->Directives);
+ EXPECT_FALSE(Err);
+ return DepDirectivesObjects.back()->Directives;
+ }
};
+ TestDependencyDirectivesGetter GetDependencyDirectives(FileMgr);
PreprocessorOptions PPOpts;
- PPOpts.DependencyDirectivesForFile = [&](FileEntryRef File)
- -> std::optional<ArrayRef<dependency_directives_scan::Directive>> {
- return getDependencyDirectives(File);
- };
-
HeaderSearchOptions HSOpts;
TrivialModuleLoader ModLoader;
HeaderSearch HeaderInfo(HSOpts, SourceMgr, Diags, LangOpts, Target.get());
@@ -130,6 +138,8 @@
/*OwnsHeaderSearch =*/false);
PP.Initialize(*Target);
+ PP.setDependencyDirectivesGetter(GetDependencyDirectives);
+
SmallVector<StringRef> IncludedFiles;
PP.addPPCallbacks(std::make_unique<IncludeCollector>(PP, IncludedFiles));
PP.EnterMainSourceFile();
