diff options
author | Jan Svoboda <jan_svoboda@apple.com> | 2025-05-22 12:33:52 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2025-05-22 12:33:52 -0700 |
commit | 9e306ad4600c4d3392c194a8be88919ee758425c (patch) | |
tree | f8b15171e92d056e9006bcbe36d534a9d73155d9 /clang/lib/Frontend/ASTUnit.cpp | |
parent | 2b8bff6f66fd90ac658d0ae0d7f9a83ffadfd77f (diff) | |
download | llvm-9e306ad4600c4d3392c194a8be88919ee758425c.zip llvm-9e306ad4600c4d3392c194a8be88919ee758425c.tar.gz llvm-9e306ad4600c4d3392c194a8be88919ee758425c.tar.bz2 |
[clang] Remove intrusive reference count from `DiagnosticOptions` (#139584)
The `DiagnosticOptions` class is currently intrusively
reference-counted, which makes reasoning about its lifetime very
difficult in some cases. For example, `CompilerInvocation` owns the
`DiagnosticOptions` instance (wrapped in `llvm::IntrusiveRefCntPtr`) and
only exposes an accessor returning `DiagnosticOptions &`. One would
think this gives `CompilerInvocation` exclusive ownership of the object,
but that's not the case:
```c++
void shareOwnership(CompilerInvocation &CI) {
llvm::IntrusiveRefCntPtr<DiagnosticOptions> CoOwner = &CI.getDiagnosticOptions();
// ...
}
```
This is a perfectly valid pattern that is being actually used in the
codebase.
I would like to ensure the ownership of `DiagnosticOptions` by
`CompilerInvocation` is guaranteed to be exclusive. This can be
leveraged for a copy-on-write optimization later on. This PR changes
usages of `DiagnosticOptions` across `clang`, `clang-tools-extra` and
`lldb` to not be intrusively reference-counted.
Diffstat (limited to 'clang/lib/Frontend/ASTUnit.cpp')
-rw-r--r-- | clang/lib/Frontend/ASTUnit.cpp | 14 |
1 files changed, 12 insertions, 2 deletions
diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp index 5a79fe0..457043c 100644 --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -803,7 +803,8 @@ void ASTUnit::ConfigureDiags(IntrusiveRefCntPtr<DiagnosticsEngine> Diags, std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile( StringRef Filename, const PCHContainerReader &PCHContainerRdr, - WhatToLoad ToLoad, IntrusiveRefCntPtr<DiagnosticsEngine> Diags, + WhatToLoad ToLoad, std::shared_ptr<DiagnosticOptions> DiagOpts, + IntrusiveRefCntPtr<DiagnosticsEngine> Diags, const FileSystemOptions &FileSystemOpts, const HeaderSearchOptions &HSOpts, const LangOptions *LangOpts, bool OnlyLocalDecls, CaptureDiagsKind CaptureDiagnostics, bool AllowASTWithCompilerErrors, @@ -823,6 +824,7 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile( : std::make_unique<LangOptions>(); AST->OnlyLocalDecls = OnlyLocalDecls; AST->CaptureDiagnostics = CaptureDiagnostics; + AST->DiagOpts = DiagOpts; AST->Diagnostics = Diags; AST->FileMgr = new FileManager(FileSystemOpts, VFS); AST->UserFilesAreVolatile = UserFilesAreVolatile; @@ -1534,6 +1536,7 @@ StringRef ASTUnit::getASTFileName() const { std::unique_ptr<ASTUnit> ASTUnit::create(std::shared_ptr<CompilerInvocation> CI, + std::shared_ptr<DiagnosticOptions> DiagOpts, IntrusiveRefCntPtr<DiagnosticsEngine> Diags, CaptureDiagsKind CaptureDiagnostics, bool UserFilesAreVolatile) { @@ -1541,6 +1544,7 @@ ASTUnit::create(std::shared_ptr<CompilerInvocation> CI, ConfigureDiags(Diags, *AST, CaptureDiagnostics); IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = createVFSFromCompilerInvocation(*CI, *Diags); + AST->DiagOpts = DiagOpts; AST->Diagnostics = Diags; AST->FileSystemOpts = CI->getFileSystemOpts(); AST->Invocation = std::move(CI); @@ -1556,6 +1560,7 @@ ASTUnit::create(std::shared_ptr<CompilerInvocation> CI, ASTUnit *ASTUnit::LoadFromCompilerInvocationAction( std::shared_ptr<CompilerInvocation> CI, std::shared_ptr<PCHContainerOperations> PCHContainerOps, + std::shared_ptr<DiagnosticOptions> DiagOpts, IntrusiveRefCntPtr<DiagnosticsEngine> Diags, FrontendAction *Action, ASTUnit *Unit, bool Persistent, StringRef ResourceFilesPath, bool OnlyLocalDecls, CaptureDiagsKind CaptureDiagnostics, @@ -1567,7 +1572,8 @@ ASTUnit *ASTUnit::LoadFromCompilerInvocationAction( ASTUnit *AST = Unit; if (!AST) { // Create the AST unit. - OwnAST = create(CI, Diags, CaptureDiagnostics, UserFilesAreVolatile); + OwnAST = + create(CI, DiagOpts, Diags, CaptureDiagnostics, UserFilesAreVolatile); AST = OwnAST.get(); if (!AST) return nullptr; @@ -1729,6 +1735,7 @@ bool ASTUnit::LoadFromCompilerInvocation( std::unique_ptr<ASTUnit> ASTUnit::LoadFromCompilerInvocation( std::shared_ptr<CompilerInvocation> CI, std::shared_ptr<PCHContainerOperations> PCHContainerOps, + std::shared_ptr<DiagnosticOptions> DiagOpts, IntrusiveRefCntPtr<DiagnosticsEngine> Diags, FileManager *FileMgr, bool OnlyLocalDecls, CaptureDiagsKind CaptureDiagnostics, unsigned PrecompilePreambleAfterNParses, TranslationUnitKind TUKind, @@ -1737,6 +1744,7 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromCompilerInvocation( // Create the AST unit. std::unique_ptr<ASTUnit> AST(new ASTUnit(false)); ConfigureDiags(Diags, *AST, CaptureDiagnostics); + AST->DiagOpts = DiagOpts; AST->Diagnostics = Diags; AST->OnlyLocalDecls = OnlyLocalDecls; AST->CaptureDiagnostics = CaptureDiagnostics; @@ -1766,6 +1774,7 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromCompilerInvocation( std::unique_ptr<ASTUnit> ASTUnit::LoadFromCommandLine( const char **ArgBegin, const char **ArgEnd, std::shared_ptr<PCHContainerOperations> PCHContainerOps, + std::shared_ptr<DiagnosticOptions> DiagOpts, IntrusiveRefCntPtr<DiagnosticsEngine> Diags, StringRef ResourceFilesPath, bool StorePreamblesInMemory, StringRef PreambleStoragePath, bool OnlyLocalDecls, CaptureDiagsKind CaptureDiagnostics, @@ -1828,6 +1837,7 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromCommandLine( AST->NumStoredDiagnosticsFromDriver = StoredDiagnostics.size(); AST->StoredDiagnostics.swap(StoredDiagnostics); ConfigureDiags(Diags, *AST, CaptureDiagnostics); + AST->DiagOpts = DiagOpts; AST->Diagnostics = Diags; AST->FileSystemOpts = CI->getFileSystemOpts(); VFS = createVFSFromCompilerInvocation(*CI, *Diags, VFS); |