Interface with explicit cloneFor(FileManager&) API

Author: jansvoboda11

clang/include/clang/Frontend/CompilerInstance.h

@@ -100,8 +100,8 @@ class CompilerInstance : public ModuleLoader {
   /// The cache of PCM files.
   IntrusiveRefCntPtr<ModuleCache> ModCache;
 
-  /// Function for getting the dependency preprocessor directives of a file.
-  GetDependencyDirectivesFn GetDependencyDirectives;
+  /// Functor for getting the dependency preprocessor directives of a file.
+  std::unique_ptr<DependencyDirectivesGetter> GetDependencyDirectives;
 
   /// The preprocessor.
   std::shared_ptr<Preprocessor> PP;
@@ -701,8 +701,9 @@ class CompilerInstance : public ModuleLoader {
   /// and replace any existing one with it.
   void createPreprocessor(TranslationUnitKind TUKind);
 
-  void setDependencyDirectivesGetter(GetDependencyDirectivesFn Fn) {
-    GetDependencyDirectives = Fn;
+  void setDependencyDirectivesGetter(
+      std::unique_ptr<DependencyDirectivesGetter> Getter) {
+    GetDependencyDirectives = std::move(Getter);
   }
 
   std::string getSpecificModuleCachePath(StringRef ModuleHash);

clang/include/clang/Lex/DependencyDirectivesScanner.h

@@ -136,11 +136,19 @@ 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)>;
-
+/// Functor that returns the dependency directives for a given file.
+class DependencyDirectivesGetter {
+public:
+  /// Clone the getter for a new \c FileManager instance.
+  virtual std::unique_ptr<DependencyDirectivesGetter>
+  cloneFor(FileManager &FileMgr) = 0;
+
+  /// Get the dependency directives for the given file.
+  virtual std::optional<ArrayRef<dependency_directives_scan::Directive>>
+  operator()(FileEntryRef File) = 0;
+
+  virtual ~DependencyDirectivesGetter() = default;
+};
 } // end namespace clang
 
 #endif // LLVM_CLANG_LEX_DEPENDENCYDIRECTIVESSCANNER_H

clang/include/clang/Lex/Preprocessor.h

@@ -140,12 +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.
+  /// Functor 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;
+  DependencyDirectivesGetter *GetDependencyDirectives = nullptr;
   const PreprocessorOptions &PPOpts;
   DiagnosticsEngine        *Diags;
   const LangOptions &LangOpts;
@@ -1332,8 +1332,8 @@ class Preprocessor {
     OnToken = std::move(F);
   }
 
-  void setDependencyDirectivesGetter(GetDependencyDirectivesFn Fn) {
-    GetDependencyDirectives = Fn;
+  void setDependencyDirectivesGetter(DependencyDirectivesGetter &Get) {
+    GetDependencyDirectives = &Get;
   }
 
   void setPreprocessToken(bool Preprocess) { PreprocessToken = Preprocess; }

clang/lib/Frontend/CompilerInstance.cpp

@@ -537,7 +537,7 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) {
   }
 
   if (GetDependencyDirectives)
-    PP->setDependencyDirectivesGetter(GetDependencyDirectives);
+    PP->setDependencyDirectivesGetter(*GetDependencyDirectives);
 }
 
 std::string CompilerInstance::getSpecificModuleCachePath(StringRef ModuleHash) {
@@ -1240,7 +1240,9 @@ std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompileImpl(
   // Make a copy for the new instance.
   Instance.FailedModules = FailedModules;
 
-  Instance.GetDependencyDirectives = GetDependencyDirectives;
+  if (GetDependencyDirectives)
+    Instance.GetDependencyDirectives =
+        GetDependencyDirectives->cloneFor(Instance.getFileManager());
 
   // If we're collecting module dependencies, we need to share a collector
   // between all of the module CompilerInstances. Other than that, we don't

clang/lib/Lex/PPLexerChange.cpp

@@ -94,7 +94,7 @@ bool Preprocessor::EnterSourceFile(FileID FID, ConstSearchDirIterator CurDir,
   Lexer *TheLexer = new Lexer(FID, *InputFile, *this, IsFirstIncludeOfFile);
   if (GetDependencyDirectives && FID != PredefinesFileID)
     if (OptionalFileEntryRef File = SourceMgr.getFileEntryRefForID(FID))
-      if (auto MaybeDepDirectives = GetDependencyDirectives(FileMgr, *File))
+      if (auto MaybeDepDirectives = (*GetDependencyDirectives)(*File))
         TheLexer->DepDirectives = *MaybeDepDirectives;
 
   EnterSourceFileWithLexer(TheLexer, CurDir);

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

@@ -357,56 +357,31 @@ 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());
-    }
+class ActualDependencyDirectivesGetter : public DependencyDirectivesGetter {
+  DependencyScanningWorkerFilesystem *DepFS;
 
-  private:
-    DependencyScanningWorkerFilesystem *DepFS;
-    FileManager *FM;
-
-    void ensureConsistentFileManager(FileManager &FileMgr) {
-      if (!FM)
-        FM = &FileMgr;
-      assert(&FileMgr == FM);
-    }
+public:
+  ActualDependencyDirectivesGetter(FileManager &FileMgr) : DepFS(nullptr) {
+    FileMgr.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");
+  }
 
-    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");
-    }
-  };
+  std::unique_ptr<DependencyDirectivesGetter>
+  cloneFor(FileManager &FileMgr) override {
+    return std::make_unique<ActualDependencyDirectivesGetter>(FileMgr);
+  }
 
-  return DepDirectivesGetter{};
-}
+  std::optional<ArrayRef<dependency_directives_scan::Directive>>
+  operator()(FileEntryRef File) override {
+    return DepFS->getDirectiveTokens(File.getName());
+  }
+};
 
 /// A clang tool that runs the preprocessor in a mode that's optimized for
 /// dependency scanning for the given compiler invocation.
@@ -475,6 +450,9 @@ class DependencyScanningAction : public tooling::ToolAction {
         ScanInstance.getInvocation(), ScanInstance.getDiagnostics(),
         DriverFileMgr->getVirtualFileSystemPtr());
 
+    // Create a new FileManager to match the invocation's FileSystemOptions.
+    auto *FileMgr = ScanInstance.createFileManager(FS);
+
     // Use the dependency scanning optimized file system if requested to do so.
     if (DepFS) {
       StringRef ModulesCachePath =
@@ -484,11 +462,10 @@ class DependencyScanningAction : public tooling::ToolAction {
       if (!ModulesCachePath.empty())
         DepFS->setBypassedPathPrefix(ModulesCachePath);
 
-      ScanInstance.setDependencyDirectivesGetter(makeDepDirectivesGetter());
+      ScanInstance.setDependencyDirectivesGetter(
+          std::make_unique<ActualDependencyDirectivesGetter>(*FileMgr));
     }
 
-    // Create a new FileManager to match the invocation's FileSystemOptions.
-    auto *FileMgr = ScanInstance.createFileManager(FS);
     ScanInstance.createSourceManager(*FileMgr);
 
     // Store a mapping of prebuilt module files and their properties like header

clang/unittests/Lex/PPDependencyDirectivesTest.cpp

@@ -103,17 +103,31 @@ TEST_F(PPDependencyDirectivesTest, MacroGuard) {
     SmallVector<dependency_directives_scan::Token> Tokens;
     SmallVector<dependency_directives_scan::Directive> Directives;
   };
-  SmallVector<std::unique_ptr<DepDirectives>> DepDirectivesObjects;
-
-  auto GetDependencyDirectives = [&](FileManager &FileMgr, FileEntryRef File) {
-    DepDirectivesObjects.push_back(std::make_unique<DepDirectives>());
-    StringRef Input = (*FileMgr.getBufferForFile(File))->getBuffer();
-    bool Err = scanSourceForDependencyDirectives(
-        Input, DepDirectivesObjects.back()->Tokens,
-        DepDirectivesObjects.back()->Directives);
-    EXPECT_FALSE(Err);
-    return llvm::ArrayRef(DepDirectivesObjects.back()->Directives);
+
+  class TestDependencyDirectivesGetter : public DependencyDirectivesGetter {
+    FileManager &FileMgr;
+    SmallVector<std::unique_ptr<DepDirectives>> DepDirectivesObjects;
+
+  public:
+    TestDependencyDirectivesGetter(FileManager &FileMgr) : FileMgr(FileMgr) {}
+
+    std::unique_ptr<DependencyDirectivesGetter>
+    cloneFor(FileManager &FileMgr) override {
+      return std::make_unique<TestDependencyDirectivesGetter>(FileMgr);
+    }
+
+    std::optional<ArrayRef<dependency_directives_scan::Directive>>
+    operator()(FileEntryRef File) override {
+      DepDirectivesObjects.push_back(std::make_unique<DepDirectives>());
+      StringRef Input = (*FileMgr.getBufferForFile(File))->getBuffer();
+      bool Err = scanSourceForDependencyDirectives(
+          Input, DepDirectivesObjects.back()->Tokens,
+          DepDirectivesObjects.back()->Directives);
+      EXPECT_FALSE(Err);
+      return DepDirectivesObjects.back()->Directives;
+    }
   };
+  TestDependencyDirectivesGetter GetDependencyDirectives(FileMgr);
 
   PreprocessorOptions PPOpts;
   HeaderSearchOptions HSOpts;