[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