Revert "[clang] Move state out of PreprocessorOptions (1/n) (#86358)"

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.

Author: jansvoboda11

clang/include/clang/Frontend/FrontendActions.h

@@ -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; }
 };
 
@@ -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

clang/include/clang/Lex/Preprocessor.h

@@ -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.
   ///
@@ -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;

clang/include/clang/Lex/PreprocessorOptions.h

@@ -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;
 

clang/lib/Frontend/FrontendActions.cpp

@@ -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();
@@ -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();

clang/lib/Lex/PPLexerChange.cpp

@@ -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;

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

@@ -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.
@@ -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;

clang/test/ClangScanDeps/modules-extension.c renamed to clang/test/ClangScanDeps/modules-minimize-extension.c

@@ -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

clang/test/ClangScanDeps/modules-minimize-module.c

@@ -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());
@@ -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();