[clang] 407a2f2 - [clang] Move state out of `PreprocessorOptions` (1/n) (#86358)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 29 11:21:00 PDT 2024
Author: Jan Svoboda
Date: 2024-03-29T11:20:55-07:00
New Revision: 407a2f231a81862e20d80059870c48d818b61ec2
URL: https://github.com/llvm/llvm-project/commit/407a2f231a81862e20d80059870c48d818b61ec2
DIFF: https://github.com/llvm/llvm-project/commit/407a2f231a81862e20d80059870c48d818b61ec2.diff
LOG: [clang] Move state out of `PreprocessorOptions` (1/n) (#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.
Added:
Modified:
clang/include/clang/Frontend/FrontendActions.h
clang/include/clang/Lex/Preprocessor.h
clang/include/clang/Lex/PreprocessorOptions.h
clang/lib/Frontend/FrontendActions.cpp
clang/lib/Lex/PPLexerChange.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/unittests/Lex/PPDependencyDirectivesTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Frontend/FrontendActions.h b/clang/include/clang/Frontend/FrontendActions.h
index a620ddfc40447d..0518a8823a03ea 100644
--- a/clang/include/clang/Frontend/FrontendActions.h
+++ b/clang/include/clang/Frontend/FrontendActions.h
@@ -34,12 +34,18 @@ class InitOnlyAction : public FrontendAction {
/// Preprocessor-based frontend action that also loads PCH files.
class ReadPCHAndPreprocessAction : public FrontendAction {
+ llvm::unique_function<void(CompilerInstance &)> AdjustCI;
+
void ExecuteAction() override;
std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
StringRef InFile) override;
public:
+ ReadPCHAndPreprocessAction(
+ llvm::unique_function<void(CompilerInstance &)> AdjustCI)
+ : AdjustCI(std::move(AdjustCI)) {}
+
bool usesPreprocessorOnly() const override { return false; }
};
@@ -321,11 +327,15 @@ class PrintPreprocessedAction : public PreprocessorFrontendAction {
class GetDependenciesByModuleNameAction : public PreprocessOnlyAction {
StringRef ModuleName;
+ llvm::unique_function<void(CompilerInstance &)> AdjustCI;
+
void ExecuteAction() override;
public:
- GetDependenciesByModuleNameAction(StringRef ModuleName)
- : ModuleName(ModuleName) {}
+ GetDependenciesByModuleNameAction(
+ StringRef ModuleName,
+ llvm::unique_function<void(CompilerInstance &)> AdjustCI)
+ : ModuleName(ModuleName), AdjustCI(std::move(AdjustCI)) {}
};
} // end namespace clang
diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index 0836b7d439bb04..24e146a589a758 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 = llvm::unique_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..0bc26b694cfc8d 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();
+ AdjustCI(CI);
+
+ Preprocessor &PP = CI.getPreprocessor();
// Ignore unknown pragmas.
PP.IgnorePragmas();
@@ -1188,6 +1191,8 @@ void PrintDependencyDirectivesSourceMinimizerAction::ExecuteAction() {
void GetDependenciesByModuleNameAction::ExecuteAction() {
CompilerInstance &CI = getCompilerInstance();
+ AdjustCI(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..492b8f1e2b3863 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -363,20 +363,22 @@ 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)
- -> 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;
- };
- }
+ auto AdjustCI = [&](CompilerInstance &CI) {
+ // Set up the dependency scanning file system callback if requested.
+ if (DepFS) {
+ auto GetDependencyDirectives = [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;
+ };
+
+ CI.getPreprocessor().setDependencyDirectivesFn(
+ std::move(GetDependencyDirectives));
+ }
+ };
// Create the dependency collector that will collect the produced
// dependencies.
@@ -428,9 +430,11 @@ class DependencyScanningAction : public tooling::ToolAction {
std::unique_ptr<FrontendAction> Action;
if (ModuleName)
- Action = std::make_unique<GetDependenciesByModuleNameAction>(*ModuleName);
+ Action = std::make_unique<GetDependenciesByModuleNameAction>(
+ *ModuleName, std::move(AdjustCI));
else
- Action = std::make_unique<ReadPCHAndPreprocessAction>();
+ Action =
+ std::make_unique<ReadPCHAndPreprocessAction>(std::move(AdjustCI));
if (ScanInstance.getDiagnostics().hasErrorOccurred())
return false;
diff --git a/clang/unittests/Lex/PPDependencyDirectivesTest.cpp b/clang/unittests/Lex/PPDependencyDirectivesTest.cpp
index 6ff87f720a559f..0c396720ece66d 100644
--- a/clang/unittests/Lex/PPDependencyDirectivesTest.cpp
+++ b/clang/unittests/Lex/PPDependencyDirectivesTest.cpp
@@ -117,11 +117,6 @@ TEST_F(PPDependencyDirectivesTest, MacroGuard) {
};
auto PPOpts = std::make_shared<PreprocessorOptions>();
- PPOpts->DependencyDirectivesForFile = [&](FileEntryRef File)
- -> std::optional<ArrayRef<dependency_directives_scan::Directive>> {
- return getDependencyDirectives(File);
- };
-
TrivialModuleLoader ModLoader;
HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
Diags, LangOpts, Target.get());
@@ -130,6 +125,12 @@ TEST_F(PPDependencyDirectivesTest, MacroGuard) {
/*OwnsHeaderSearch =*/false);
PP.Initialize(*Target);
+ PP.setDependencyDirectivesFn(
+ [&](FileEntryRef File)
+ -> std::optional<ArrayRef<dependency_directives_scan::Directive>> {
+ return getDependencyDirectives(File);
+ });
+
SmallVector<StringRef> IncludedFiles;
PP.addPPCallbacks(std::make_unique<IncludeCollector>(PP, IncludedFiles));
PP.EnterMainSourceFile();
More information about the cfe-commits
mailing list