https://github.com/jansvoboda11 updated
https://github.com/llvm/llvm-project/pull/136178
>From 51449e05b19f3b0f237305159ae150bc92588fd9 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Fri, 11 Apr 2025 14:11:36 -0700
Subject: [PATCH 1/2] [clang][deps] Make dependency directives getter
thread-safe
---
.../include/clang/Frontend/CompilerInstance.h | 8 +++
.../clang/Lex/DependencyDirectivesScanner.h | 6 ++
clang/include/clang/Lex/Preprocessor.h | 10 +++
clang/include/clang/Lex/PreprocessorOptions.h | 13 ----
.../DependencyScanningFilesystem.h | 10 +++
clang/lib/Frontend/CompilerInstance.cpp | 5 ++
clang/lib/Lex/PPLexerChange.cpp | 14 ++---
.../DependencyScanningWorker.cpp | 61 ++++++++++++++++---
.../Lex/PPDependencyDirectivesTest.cpp | 12 ++--
9 files changed, 100 insertions(+), 39 deletions(-)
diff --git a/clang/include/clang/Frontend/CompilerInstance.h
b/clang/include/clang/Frontend/CompilerInstance.h
index 41dc7f1fef461..ecd1f5cabc79e 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 @@ class CompilerInstance : public ModuleLoader {
/// The cache of PCM files.
IntrusiveRefCntPtr<ModuleCache> ModCache;
+ /// Function for getting the dependency preprocessor directives of a file.
+ GetDependencyDirectivesFn GetDependencyDirectives;
+
/// The preprocessor.
std::shared_ptr<Preprocessor> PP;
@@ -697,6 +701,10 @@ class CompilerInstance : public ModuleLoader {
/// and replace any existing one with it.
void createPreprocessor(TranslationUnitKind TUKind);
+ void setDependencyDirectivesGetter(GetDependencyDirectivesFn Fn) {
+ GetDependencyDirectives = Fn;
+ }
+
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 0e115906fbfe5..c975311f8bf33 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,11 @@ void printDependencyDirectivesAsSource(
ArrayRef<dependency_directives_scan::Directive> Directives,
llvm::raw_ostream &OS);
+// FIXME: Allow returning an error.
+using GetDependencyDirectivesFn = std::function<
+ std::optional<ArrayRef<dependency_directives_scan::Directive>>(
+ FileManager &, FileEntryRef)>;
+
} // 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 24bb524783e93..8554068b19607 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -140,6 +140,12 @@ class Preprocessor {
friend class VariadicMacroScopeGuard;
llvm::unique_function<void(const clang::Token &)> OnToken;
+ /// 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.
+ GetDependencyDirectivesFn GetDependencyDirectives;
const PreprocessorOptions &PPOpts;
DiagnosticsEngine *Diags;
const LangOptions &LangOpts;
@@ -1326,6 +1332,10 @@ class Preprocessor {
OnToken = std::move(F);
}
+ void setDependencyDirectivesGetter(GetDependencyDirectivesFn Fn) {
+ GetDependencyDirectives = Fn;
+ }
+
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 c2e3d68333024..d4c4e1ccbf2c4 100644
--- a/clang/include/clang/Lex/PreprocessorOptions.h
+++ b/clang/include/clang/Lex/PreprocessorOptions.h
@@ -189,19 +189,6 @@ class PreprocessorOptions {
/// 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 d12814e7c9253..f07bd5e820dab 100644
---
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
+++
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h
@@ -371,6 +371,16 @@ class DependencyScanningWorkerFilesystem
/// 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 243e0a3c15b05..0efe0ada4873f 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -535,6 +535,9 @@ void
CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) {
/*ShowAllHeaders=*/true, /*OutputPath=*/"",
/*ShowDepth=*/true, /*MSStyle=*/true);
}
+
+ if (GetDependencyDirectives)
+ PP->setDependencyDirectivesGetter(GetDependencyDirectives);
}
std::string CompilerInstance::getSpecificModuleCachePath(StringRef ModuleHash)
{
@@ -1237,6 +1240,8 @@ std::unique_ptr<CompilerInstance>
CompilerInstance::cloneForModuleCompileImpl(
// Make a copy for the new instance.
Instance.FailedModules = FailedModules;
+ Instance.GetDependencyDirectives = GetDependencyDirectives;
+
// 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 a373a52506a24..09ef921f67202 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(FileMgr, *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 6595f8ff5dc55..a9e35e0484ac9 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -357,6 +357,57 @@ static void canonicalizeDefines(PreprocessorOptions
&PPOpts) {
std::swap(PPOpts.Macros, NewMacros);
}
+static GetDependencyDirectivesFn makeDepDirectivesGetter() {
+ /// This is a functor that conforms to \c GetDependencyDirectivesFn.
+ /// It ensures it's always invoked with the same \c FileManager and caches
the
+ /// extraction of the scanning VFS for better performance.
+ struct DepDirectivesGetter {
+ DepDirectivesGetter() : DepFS(nullptr), FM(nullptr) {}
+
+ /// It's important copies do not carry over the cached members. The copies
+ /// are likely to be used from distinct \c CompilerInstance objects with
+ /// distinct \c FileManager \c llvm::vfs::FileSystem.
+ DepDirectivesGetter(const DepDirectivesGetter &)
+ : DepFS(nullptr), FM(nullptr) {}
+ DepDirectivesGetter &operator=(const DepDirectivesGetter &) {
+ DepFS = nullptr;
+ FM = nullptr;
+ return *this;
+ }
+
+ auto operator()(FileManager &FileMgr, FileEntryRef File) {
+ ensureConsistentFileManager(FileMgr);
+ ensurePopulatedFileSystem(FileMgr);
+ return DepFS->getDirectiveTokens(File.getName());
+ }
+
+ private:
+ DependencyScanningWorkerFilesystem *DepFS;
+ FileManager *FM;
+
+ void ensureConsistentFileManager(FileManager &FileMgr) {
+ if (!FM)
+ FM = &FileMgr;
+ assert(&FileMgr == FM);
+ }
+
+ void ensurePopulatedFileSystem(FileManager &FM) {
+ if (DepFS)
+ return;
+ FM.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");
+ }
+ };
+
+ return DepDirectivesGetter{};
+}
+
/// 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 {
@@ -433,15 +484,7 @@ class DependencyScanningAction : public
tooling::ToolAction {
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(makeDepDirectivesGetter());
}
// Create a new FileManager to match the invocation's FileSystemOptions.
diff --git a/clang/unittests/Lex/PPDependencyDirectivesTest.cpp
b/clang/unittests/Lex/PPDependencyDirectivesTest.cpp
index 03f1432d990cb..c52d5091eabfb 100644
--- a/clang/unittests/Lex/PPDependencyDirectivesTest.cpp
+++ b/clang/unittests/Lex/PPDependencyDirectivesTest.cpp
@@ -105,8 +105,7 @@ TEST_F(PPDependencyDirectivesTest, MacroGuard) {
};
SmallVector<std::unique_ptr<DepDirectives>> DepDirectivesObjects;
- auto getDependencyDirectives = [&](FileEntryRef File)
- -> std::optional<ArrayRef<dependency_directives_scan::Directive>> {
+ auto GetDependencyDirectives = [&](FileManager &FileMgr, FileEntryRef File) {
DepDirectivesObjects.push_back(std::make_unique<DepDirectives>());
StringRef Input = (*FileMgr.getBufferForFile(File))->getBuffer();
bool Err = scanSourceForDependencyDirectives(
@@ -117,11 +116,6 @@ TEST_F(PPDependencyDirectivesTest, MacroGuard) {
};
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 +124,10 @@ TEST_F(PPDependencyDirectivesTest, MacroGuard) {
/*OwnsHeaderSearch =*/false);
PP.Initialize(*Target);
+ PP.setDependencyDirectivesGetter([&](FileManager &FM, FileEntryRef File) {
+ return GetDependencyDirectives(FM, File);
+ });
+
SmallVector<StringRef> IncludedFiles;
PP.addPPCallbacks(std::make_unique<IncludeCollector>(PP, IncludedFiles));
PP.EnterMainSourceFile();
>From 00099ccf341ba443e356b514a4f81dae2b01e0bd Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svob...@apple.com>
Date: Thu, 17 Apr 2025 11:44:01 -0700
Subject: [PATCH 2/2] Simplify test
---
clang/unittests/Lex/PPDependencyDirectivesTest.cpp | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/clang/unittests/Lex/PPDependencyDirectivesTest.cpp
b/clang/unittests/Lex/PPDependencyDirectivesTest.cpp
index c52d5091eabfb..5f4c2706a519d 100644
--- a/clang/unittests/Lex/PPDependencyDirectivesTest.cpp
+++ b/clang/unittests/Lex/PPDependencyDirectivesTest.cpp
@@ -124,9 +124,7 @@ TEST_F(PPDependencyDirectivesTest, MacroGuard) {
/*OwnsHeaderSearch =*/false);
PP.Initialize(*Target);
- PP.setDependencyDirectivesGetter([&](FileManager &FM, FileEntryRef File) {
- return GetDependencyDirectives(FM, File);
- });
+ PP.setDependencyDirectivesGetter(GetDependencyDirectives);
SmallVector<StringRef> IncludedFiles;
PP.addPPCallbacks(std::make_unique<IncludeCollector>(PP, IncludedFiles));
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
