Skip to content

Commit

Permalink
Revert "[clang] Move state out of PreprocessorOptions (1/n) (#86358)"
Browse files Browse the repository at this point in the history
This reverts commit 407a2f2 which stopped propagating the callback to module compiles, effectively disabling dependency directive scanning for all modular dependencies. Also added a regression test.
  • Loading branch information
jansvoboda11 committed Apr 9, 2024
1 parent 4ae8694 commit 2248164
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 65 deletions.
14 changes: 2 additions & 12 deletions clang/include/clang/Frontend/FrontendActions.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,12 @@ 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; }
};

Expand Down Expand Up @@ -327,15 +321,11 @@ class PrintPreprocessedAction : public PreprocessorFrontendAction {

class GetDependenciesByModuleNameAction : public PreprocessOnlyAction {
StringRef ModuleName;
llvm::unique_function<void(CompilerInstance &)> AdjustCI;

void ExecuteAction() override;

public:
GetDependenciesByModuleNameAction(
StringRef ModuleName,
llvm::unique_function<void(CompilerInstance &)> AdjustCI)
: ModuleName(ModuleName), AdjustCI(std::move(AdjustCI)) {}
GetDependenciesByModuleNameAction(StringRef ModuleName)
: ModuleName(ModuleName) {}
};

} // end namespace clang
Expand Down
18 changes: 0 additions & 18 deletions clang/include/clang/Lex/Preprocessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -736,19 +736,6 @@ 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.
///
Expand Down Expand Up @@ -1283,11 +1270,6 @@ 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;
Expand Down
13 changes: 13 additions & 0 deletions clang/include/clang/Lex/PreprocessorOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,19 @@ 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;

Expand Down
7 changes: 1 addition & 6 deletions clang/lib/Frontend/FrontendActions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,7 @@ void InitOnlyAction::ExecuteAction() {

// Basically PreprocessOnlyAction::ExecuteAction.
void ReadPCHAndPreprocessAction::ExecuteAction() {
CompilerInstance &CI = getCompilerInstance();
AdjustCI(CI);

Preprocessor &PP = CI.getPreprocessor();
Preprocessor &PP = getCompilerInstance().getPreprocessor();

// Ignore unknown pragmas.
PP.IgnorePragmas();
Expand Down Expand Up @@ -1193,8 +1190,6 @@ void PrintDependencyDirectivesSourceMinimizerAction::ExecuteAction() {

void GetDependenciesByModuleNameAction::ExecuteAction() {
CompilerInstance &CI = getCompilerInstance();
AdjustCI(CI);

Preprocessor &PP = CI.getPreprocessor();
SourceManager &SM = PP.getSourceManager();
FileID MainFileID = SM.getMainFileID();
Expand Down
12 changes: 9 additions & 3 deletions clang/lib/Lex/PPLexerChange.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,16 @@ bool Preprocessor::EnterSourceFile(FileID FID, ConstSearchDirIterator CurDir,
}

Lexer *TheLexer = new Lexer(FID, *InputFile, *this, IsFirstIncludeOfFile);
if (DependencyDirectivesForFile && FID != PredefinesFileID)
if (OptionalFileEntryRef File = SourceMgr.getFileEntryRefForID(FID))
if (auto DepDirectives = DependencyDirectivesForFile(*File))
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;
}
}
}

EnterSourceFileWithLexer(TheLexer, CurDir);
return false;
Expand Down
33 changes: 13 additions & 20 deletions clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -363,22 +363,17 @@ class DependencyScanningAction : public tooling::ToolAction {
PrebuiltModuleVFSMap, ScanInstance.getDiagnostics()))
return false;

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));
}
};
// Use the dependency scanning optimized file system if requested to do so.
if (DepFS)
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;
};

// Create the dependency collector that will collect the produced
// dependencies.
Expand Down Expand Up @@ -430,11 +425,9 @@ class DependencyScanningAction : public tooling::ToolAction {
std::unique_ptr<FrontendAction> Action;

if (ModuleName)
Action = std::make_unique<GetDependenciesByModuleNameAction>(
*ModuleName, std::move(AdjustCI));
Action = std::make_unique<GetDependenciesByModuleNameAction>(*ModuleName);
else
Action =
std::make_unique<ReadPCHAndPreprocessAction>(std::move(AdjustCI));
Action = std::make_unique<ReadPCHAndPreprocessAction>();

if (ScanInstance.getDiagnostics().hasErrorOccurred())
return false;
Expand Down
24 changes: 24 additions & 0 deletions clang/test/ClangScanDeps/modules-minimize-module.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// RUN: rm -rf %t
// RUN: split-file %s %t

// This test checks that source files of modules undergo dependency directives
// scan. If a.h would not, the scan would fail when lexing `#error`.

//--- module.modulemap
module A { header "a.h" }

//--- a.h
#error blah

//--- tu.c
#include "a.h"

//--- cdb.json.in
[{
"directory": "DIR",
"file": "DIR/tu.c",
"command": "clang -c DIR/tu.c -fmodules -fmodules-cache-path=DIR/cache"
}]

// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json
// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/deps.json
11 changes: 5 additions & 6 deletions clang/unittests/Lex/PPDependencyDirectivesTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@ 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());
Expand All @@ -125,12 +130,6 @@ 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();
Expand Down

0 comments on commit 2248164

Please sign in to comment.