[clang] [clang] Move state out of `PreprocessorOptions` (1/n) (PR #86358)
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 22 16:26:18 PDT 2024
https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/86358
An instance of `PreprocessorOptions` is part of `CompilerInvocation` which is supposed to be a value type. The `DependencyDirectivesForFile` member is problematic, since it holds an owning reference of the scanning VFS. This makes it not a true value type, and it can keep potentially large chunk of memory (the local cache in the scanning VFS) alive for longer than clients might expect. Let's move it into the `Preprocessor` instead.
>From 109e6039abe8bd3b598f1fdc2dbd7bca783ff4e6 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Fri, 22 Mar 2024 15:54:14 -0700
Subject: [PATCH] [clang] Move state out of `PreprocessorOptions` (1/n)
An instance of `PreprocessorOptions` is part of `CompilerInvocation` which is supposed to be a value type. The `DependencyDirectivesForFile` member is problematic, since it holds an owning reference of the scanning VFS. This makes it not a true value type, and it can keep potentially large chunk of memory (the local cache in the scanning VFS) alive for longer than clients might expect. Let's move it into the `Preprocessor` instead.
---
.../include/clang/Frontend/FrontendActions.h | 12 +++++++++--
clang/include/clang/Lex/Preprocessor.h | 18 ++++++++++++++++
clang/include/clang/Lex/PreprocessorOptions.h | 13 ------------
clang/lib/Frontend/FrontendActions.cpp | 7 ++++++-
clang/lib/Lex/PPLexerChange.cpp | 12 +++--------
.../DependencyScanningWorker.cpp | 21 +++++++++++--------
6 files changed, 49 insertions(+), 34 deletions(-)
diff --git a/clang/include/clang/Frontend/FrontendActions.h b/clang/include/clang/Frontend/FrontendActions.h
index a620ddfc40447d..575cf9ed2ac777 100644
--- a/clang/include/clang/Frontend/FrontendActions.h
+++ b/clang/include/clang/Frontend/FrontendActions.h
@@ -34,12 +34,17 @@ class InitOnlyAction : public FrontendAction {
/// Preprocessor-based frontend action that also loads PCH files.
class ReadPCHAndPreprocessAction : public FrontendAction {
+ llvm::function_ref<void(CompilerInstance &)> OnCI;
+
void ExecuteAction() override;
std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
StringRef InFile) override;
public:
+ ReadPCHAndPreprocessAction(llvm::function_ref<void(CompilerInstance &)> OnCI)
+ : OnCI(OnCI) {}
+
bool usesPreprocessorOnly() const override { return false; }
};
@@ -321,11 +326,14 @@ class PrintPreprocessedAction : public PreprocessorFrontendAction {
class GetDependenciesByModuleNameAction : public PreprocessOnlyAction {
StringRef ModuleName;
+ llvm::function_ref<void(CompilerInstance &)> OnCI;
+
void ExecuteAction() override;
public:
- GetDependenciesByModuleNameAction(StringRef ModuleName)
- : ModuleName(ModuleName) {}
+ GetDependenciesByModuleNameAction(
+ StringRef ModuleName, llvm::function_ref<void(CompilerInstance &)> OnCI)
+ : ModuleName(ModuleName), OnCI(OnCI) {}
};
} // end namespace clang
diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index 0836b7d439bb04..10a7ba4db25625 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -736,6 +736,19 @@ class Preprocessor {
State ConditionalStackState = Off;
} PreambleConditionalStack;
+ /// 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.
+ using DependencyDirectivesFn = std::function<std::optional<
+ ArrayRef<dependency_directives_scan::Directive>>(FileEntryRef)>;
+ DependencyDirectivesFn DependencyDirectivesForFile;
+
/// The current top of the stack that we're lexing from if
/// not expanding a macro and we are lexing directly from source code.
///
@@ -1270,6 +1283,11 @@ class Preprocessor {
/// false if it is producing tokens to be consumed by Parse and Sema.
bool isPreprocessedOutput() const { return PreprocessedOutput; }
+ /// Set the function used to get dependency directives for a file.
+ void setDependencyDirectivesFn(DependencyDirectivesFn Fn) {
+ DependencyDirectivesForFile = std::move(Fn);
+ }
+
/// Return true if we are lexing directly from the specified lexer.
bool isCurrentLexer(const PreprocessorLexer *L) const {
return CurPPLexer == L;
diff --git a/clang/include/clang/Lex/PreprocessorOptions.h b/clang/include/clang/Lex/PreprocessorOptions.h
index f841e4a028df50..24e0ca61d41432 100644
--- a/clang/include/clang/Lex/PreprocessorOptions.h
+++ b/clang/include/clang/Lex/PreprocessorOptions.h
@@ -208,19 +208,6 @@ class PreprocessorOptions {
/// build it again.
std::shared_ptr<FailedModulesSet> FailedModules;
- /// 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/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp
index 3fd1cdd3b47942..b31e877b9f11f8 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -69,7 +69,10 @@ void InitOnlyAction::ExecuteAction() {
// Basically PreprocessOnlyAction::ExecuteAction.
void ReadPCHAndPreprocessAction::ExecuteAction() {
- Preprocessor &PP = getCompilerInstance().getPreprocessor();
+ CompilerInstance &CI = getCompilerInstance();
+ OnCI(CI);
+
+ Preprocessor &PP = CI.getPreprocessor();
// Ignore unknown pragmas.
PP.IgnorePragmas();
@@ -1188,6 +1191,8 @@ void PrintDependencyDirectivesSourceMinimizerAction::ExecuteAction() {
void GetDependenciesByModuleNameAction::ExecuteAction() {
CompilerInstance &CI = getCompilerInstance();
+ OnCI(CI);
+
Preprocessor &PP = CI.getPreprocessor();
SourceManager &SM = PP.getSourceManager();
FileID MainFileID = SM.getMainFileID();
diff --git a/clang/lib/Lex/PPLexerChange.cpp b/clang/lib/Lex/PPLexerChange.cpp
index 3b1b6df1dbae4e..a0cc2b516574c1 100644
--- a/clang/lib/Lex/PPLexerChange.cpp
+++ b/clang/lib/Lex/PPLexerChange.cpp
@@ -93,16 +93,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)) {
+ if (DependencyDirectivesForFile && FID != PredefinesFileID)
+ if (OptionalFileEntryRef File = SourceMgr.getFileEntryRefForID(FID))
+ if (auto DepDirectives = DependencyDirectivesForFile(*File))
TheLexer->DepDirectives = *DepDirectives;
- }
- }
- }
EnterSourceFileWithLexer(TheLexer, CurDir);
return false;
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 33b43417a6613d..d62014a4434bd0 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -363,12 +363,12 @@ class DependencyScanningAction : public tooling::ToolAction {
PrebuiltModuleVFSMap, ScanInstance.getDiagnostics()))
return false;
- // Use the dependency scanning optimized file system if requested to do so.
- if (DepFS) {
- llvm::IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> LocalDepFS =
- DepFS;
- ScanInstance.getPreprocessorOpts().DependencyDirectivesForFile =
- [LocalDepFS = std::move(LocalDepFS)](FileEntryRef File)
+ // Set up the dependency scanning file system callback if requested.
+ auto AdjustCI = [&](CompilerInstance &CI) {
+ if (!DepFS)
+ return;
+
+ auto GetDependencyDirectives = [LocalDepFS = DepFS](FileEntryRef File)
-> std::optional<ArrayRef<dependency_directives_scan::Directive>> {
if (llvm::ErrorOr<EntryRef> Entry =
LocalDepFS->getOrCreateFileSystemEntry(File.getName()))
@@ -376,7 +376,9 @@ class DependencyScanningAction : public tooling::ToolAction {
return Entry->getDirectiveTokens();
return std::nullopt;
};
- }
+
+ CI.getPreprocessor().setDependencyDirectivesFn(GetDependencyDirectives);
+ };
// Create the dependency collector that will collect the produced
// dependencies.
@@ -428,9 +430,10 @@ class DependencyScanningAction : public tooling::ToolAction {
std::unique_ptr<FrontendAction> Action;
if (ModuleName)
- Action = std::make_unique<GetDependenciesByModuleNameAction>(*ModuleName);
+ Action = std::make_unique<GetDependenciesByModuleNameAction>(*ModuleName,
+ AdjustCI);
else
- Action = std::make_unique<ReadPCHAndPreprocessAction>();
+ Action = std::make_unique<ReadPCHAndPreprocessAction>(AdjustCI);
if (ScanInstance.getDiagnostics().hasErrorOccurred())
return false;
More information about the cfe-commits
mailing list