Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 23, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Interface with explicit cloneFor(FileManager&) API
  • Loading branch information
jansvoboda11 committed Apr 22, 2025
commit 105e148b597907b9ef21c492f401c10725cf49e2
9 changes: 5 additions & 4 deletions clang/include/clang/Frontend/CompilerInstance.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
18 changes: 13 additions & 5 deletions clang/include/clang/Lex/DependencyDirectivesScanner.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 4 additions & 4 deletions clang/include/clang/Lex/Preprocessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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; }
Expand Down
6 changes: 4 additions & 2 deletions clang/lib/Frontend/CompilerInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) {
}

if (GetDependencyDirectives)
PP->setDependencyDirectivesGetter(GetDependencyDirectives);
PP->setDependencyDirectivesGetter(*GetDependencyDirectives);
}

std::string CompilerInstance::getSpecificModuleCachePath(StringRef ModuleHash) {
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Lex/PPLexerChange.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
77 changes: 27 additions & 50 deletions clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 =
Expand All @@ -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
Expand Down
34 changes: 24 additions & 10 deletions clang/unittests/Lex/PPDependencyDirectivesTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading