Skip to content

Commit 0797f70

Browse files
authored
[clang] Enable making CompilerInstance diagnostics thread-safe (#136601)
The `DiagnosticConsumer` interface is not thread-safe. To enable thread-safety of `CompilerInstance` objects cloned from the same parent, this PR allows passing an explicit `DiagnosticConsumer` to `cloneForModuleCompile()`. This will be used from the dependency scanner.
1 parent a2be454 commit 0797f70

File tree

2 files changed

+37
-17
lines changed

2 files changed

+37
-17
lines changed

clang/include/clang/Frontend/CompilerInstance.h

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -825,6 +825,23 @@ class CompilerInstance : public ModuleLoader {
825825
bool loadModuleFile(StringRef FileName,
826826
serialization::ModuleFile *&LoadedModuleFile);
827827

828+
/// Configuration object for making the result of \c cloneForModuleCompile()
829+
/// thread-safe.
830+
class ThreadSafeCloneConfig {
831+
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS;
832+
DiagnosticConsumer &DiagConsumer;
833+
834+
public:
835+
ThreadSafeCloneConfig(IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
836+
DiagnosticConsumer &DiagConsumer)
837+
: VFS(std::move(VFS)), DiagConsumer(DiagConsumer) {
838+
assert(this->VFS && "Clone config requires non-null VFS");
839+
}
840+
841+
IntrusiveRefCntPtr<llvm::vfs::FileSystem> getVFS() const { return VFS; }
842+
DiagnosticConsumer &getDiagConsumer() const { return DiagConsumer; }
843+
};
844+
828845
private:
829846
/// Find a module, potentially compiling it, before reading its AST. This is
830847
/// the guts of loadModule.
@@ -845,25 +862,22 @@ class CompilerInstance : public ModuleLoader {
845862
/// Creates a \c CompilerInstance for compiling a module.
846863
///
847864
/// This expects a properly initialized \c FrontendInputFile.
848-
///
849-
/// Explicitly-specified \c VFS takes precedence over the VFS of this instance
850-
/// when creating the clone and also prevents \c FileManager sharing.
851865
std::unique_ptr<CompilerInstance> cloneForModuleCompileImpl(
852866
SourceLocation ImportLoc, StringRef ModuleName, FrontendInputFile Input,
853867
StringRef OriginalModuleMapFile, StringRef ModuleFileName,
854-
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = nullptr);
868+
std::optional<ThreadSafeCloneConfig> ThreadSafeConfig = std::nullopt);
855869

856870
public:
857871
/// Creates a new \c CompilerInstance for compiling a module.
858872
///
859873
/// This takes care of creating appropriate \c FrontendInputFile for
860874
/// public/private frameworks, inferred modules and such.
861875
///
862-
/// Explicitly-specified \c VFS takes precedence over the VFS of this instance
863-
/// when creating the clone and also prevents \c FileManager sharing.
876+
/// The \c ThreadSafeConfig takes precedence over the \c DiagnosticConsumer
877+
/// and \c FileSystem of this instance (and disables \c FileManager sharing).
864878
std::unique_ptr<CompilerInstance> cloneForModuleCompile(
865879
SourceLocation ImportLoc, Module *Module, StringRef ModuleFileName,
866-
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = nullptr);
880+
std::optional<ThreadSafeCloneConfig> ThreadSafeConfig = std::nullopt);
867881

868882
/// Compile a module file for the given module, using the options
869883
/// provided by the importing compiler instance. Returns true if the module

clang/lib/Frontend/CompilerInstance.cpp

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1153,7 +1153,7 @@ static Language getLanguageFromOptions(const LangOptions &LangOpts) {
11531153
std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompileImpl(
11541154
SourceLocation ImportLoc, StringRef ModuleName, FrontendInputFile Input,
11551155
StringRef OriginalModuleMapFile, StringRef ModuleFileName,
1156-
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) {
1156+
std::optional<ThreadSafeCloneConfig> ThreadSafeConfig) {
11571157
// Construct a compiler invocation for creating this module.
11581158
auto Invocation = std::make_shared<CompilerInvocation>(getInvocation());
11591159

@@ -1213,18 +1213,24 @@ std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompileImpl(
12131213
auto &Inv = *Invocation;
12141214
Instance.setInvocation(std::move(Invocation));
12151215

1216-
if (VFS) {
1217-
Instance.createFileManager(std::move(VFS));
1216+
if (ThreadSafeConfig) {
1217+
Instance.createFileManager(ThreadSafeConfig->getVFS());
12181218
} else if (FrontendOpts.ModulesShareFileManager) {
12191219
Instance.setFileManager(&getFileManager());
12201220
} else {
12211221
Instance.createFileManager(&getVirtualFileSystem());
12221222
}
12231223

1224-
Instance.createDiagnostics(
1225-
Instance.getVirtualFileSystem(),
1226-
new ForwardingDiagnosticConsumer(getDiagnosticClient()),
1227-
/*ShouldOwnClient=*/true);
1224+
if (ThreadSafeConfig) {
1225+
Instance.createDiagnostics(Instance.getVirtualFileSystem(),
1226+
&ThreadSafeConfig->getDiagConsumer(),
1227+
/*ShouldOwnClient=*/false);
1228+
} else {
1229+
Instance.createDiagnostics(
1230+
Instance.getVirtualFileSystem(),
1231+
new ForwardingDiagnosticConsumer(getDiagnosticClient()),
1232+
/*ShouldOwnClient=*/true);
1233+
}
12281234
if (llvm::is_contained(DiagOpts.SystemHeaderWarningsModules, ModuleName))
12291235
Instance.getDiagnostics().setSuppressSystemWarnings(false);
12301236

@@ -1322,7 +1328,7 @@ static OptionalFileEntryRef getPublicModuleMap(FileEntryRef File,
13221328

13231329
std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompile(
13241330
SourceLocation ImportLoc, Module *Module, StringRef ModuleFileName,
1325-
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) {
1331+
std::optional<ThreadSafeCloneConfig> ThreadSafeConfig) {
13261332
StringRef ModuleName = Module->getTopLevelModuleName();
13271333

13281334
InputKind IK(getLanguageFromOptions(getLangOpts()), InputKind::ModuleMap);
@@ -1368,7 +1374,7 @@ std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompile(
13681374
ImportLoc, ModuleName,
13691375
FrontendInputFile(ModuleMapFilePath, IK, IsSystem),
13701376
ModMap.getModuleMapFileForUniquing(Module)->getName(), ModuleFileName,
1371-
std::move(VFS));
1377+
std::move(ThreadSafeConfig));
13721378
}
13731379

13741380
// FIXME: We only need to fake up an input file here as a way of
@@ -1386,7 +1392,7 @@ std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompile(
13861392
ImportLoc, ModuleName,
13871393
FrontendInputFile(FakeModuleMapFile, IK, +Module->IsSystem),
13881394
ModMap.getModuleMapFileForUniquing(Module)->getName(), ModuleFileName,
1389-
std::move(VFS));
1395+
std::move(ThreadSafeConfig));
13901396

13911397
std::unique_ptr<llvm::MemoryBuffer> ModuleMapBuffer =
13921398
llvm::MemoryBuffer::getMemBufferCopy(InferredModuleMapContent);

0 commit comments

Comments
 (0)