[clang] [clang][deps] Make dependency directives getter thread-safe (PR #136178)

Jan Svoboda via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 18 13:51:38 PDT 2025


================
@@ -92,16 +92,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)) {
-        TheLexer->DepDirectives = *DepDirectives;
-      }
-    }
-  }
+  if (GetDependencyDirectives && FID != PredefinesFileID)
+    if (OptionalFileEntryRef File = SourceMgr.getFileEntryRefForID(FID))
+      if (auto MaybeDepDirectives = GetDependencyDirectives(FileMgr, *File))
----------------
jansvoboda11 wrote:

> Apologies if this is obvious, but what is the difference between the `FileMgr` in the `Preprocessor` compared to the `FileMgr` that's tied to the CompilerInstance object created in https://github.com/llvm/llvm-project/pull/136178/files#diff-9190b5361d3722e679a0ea59ff46cc21f6d10d6e61049d73b3bc2e1ea56eb8f6R1243 ?

There is none, it's the same object.

> IOW, why is the potential assignment of DepDirectivesGetter's attributes on each call to the function & not during assignment of the function object?

Because there's no other `API` on `std::function` besides `operator()` so there's no way to set the new `FileManager` while making the copy in `CompilerInstance::cloneForModuleCompileImpl()`.

Let me know if this explanation makes sense.

Maybe there is an argument to be made to replace `std::function` with something like:

```c++
struct DepDirectivesGetter {
  virtual std::unique_ptr<DepDirectivesGetter> cloneFor(FileManager &) = 0;
  virtual std::optional<ArrayRef<dependency_directives_scan::Directive>> operator()(FileEntryRef) = 0;
}
```

https://github.com/llvm/llvm-project/pull/136178


More information about the cfe-commits mailing list