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/CompilerInstance.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/CompilerInstance.cpp')
-rw-r--r-- | clang/lib/Frontend/CompilerInstance.cpp | 27 |
1 files changed, 13 insertions, 14 deletions
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 503d364..cc39049 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -280,20 +280,20 @@ static void collectVFSEntries(CompilerInstance &CI, } // Diagnostics -static void SetUpDiagnosticLog(DiagnosticOptions *DiagOpts, +static void SetUpDiagnosticLog(DiagnosticOptions &DiagOpts, const CodeGenOptions *CodeGenOpts, DiagnosticsEngine &Diags) { std::error_code EC; std::unique_ptr<raw_ostream> StreamOwner; raw_ostream *OS = &llvm::errs(); - if (DiagOpts->DiagnosticLogFile != "-") { + if (DiagOpts.DiagnosticLogFile != "-") { // Create the output stream. auto FileOS = std::make_unique<llvm::raw_fd_ostream>( - DiagOpts->DiagnosticLogFile, EC, + DiagOpts.DiagnosticLogFile, EC, llvm::sys::fs::OF_Append | llvm::sys::fs::OF_TextWithCRLF); if (EC) { Diags.Report(diag::warn_fe_cc_log_diagnostics_failure) - << DiagOpts->DiagnosticLogFile << EC.message(); + << DiagOpts.DiagnosticLogFile << EC.message(); } else { FileOS->SetUnbuffered(); OS = FileOS.get(); @@ -315,7 +315,7 @@ static void SetUpDiagnosticLog(DiagnosticOptions *DiagOpts, } } -static void SetupSerializedDiagnostics(DiagnosticOptions *DiagOpts, +static void SetupSerializedDiagnostics(DiagnosticOptions &DiagOpts, DiagnosticsEngine &Diags, StringRef OutputFile) { auto SerializedConsumer = @@ -333,12 +333,12 @@ static void SetupSerializedDiagnostics(DiagnosticOptions *DiagOpts, void CompilerInstance::createDiagnostics(llvm::vfs::FileSystem &VFS, DiagnosticConsumer *Client, bool ShouldOwnClient) { - Diagnostics = createDiagnostics(VFS, &getDiagnosticOpts(), Client, + Diagnostics = createDiagnostics(VFS, getDiagnosticOpts(), Client, ShouldOwnClient, &getCodeGenOpts()); } IntrusiveRefCntPtr<DiagnosticsEngine> CompilerInstance::createDiagnostics( - llvm::vfs::FileSystem &VFS, DiagnosticOptions *Opts, + llvm::vfs::FileSystem &VFS, DiagnosticOptions &Opts, DiagnosticConsumer *Client, bool ShouldOwnClient, const CodeGenOptions *CodeGenOpts) { IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs()); @@ -349,25 +349,24 @@ IntrusiveRefCntPtr<DiagnosticsEngine> CompilerInstance::createDiagnostics( // implementing -verify. if (Client) { Diags->setClient(Client, ShouldOwnClient); - } else if (Opts->getFormat() == DiagnosticOptions::SARIF) { + } else if (Opts.getFormat() == DiagnosticOptions::SARIF) { Diags->setClient(new SARIFDiagnosticPrinter(llvm::errs(), Opts)); } else Diags->setClient(new TextDiagnosticPrinter(llvm::errs(), Opts)); // Chain in -verify checker, if requested. - if (Opts->VerifyDiagnostics) + if (Opts.VerifyDiagnostics) Diags->setClient(new VerifyDiagnosticConsumer(*Diags)); // Chain in -diagnostic-log-file dumper, if requested. - if (!Opts->DiagnosticLogFile.empty()) + if (!Opts.DiagnosticLogFile.empty()) SetUpDiagnosticLog(Opts, CodeGenOpts, *Diags); - if (!Opts->DiagnosticSerializationFile.empty()) - SetupSerializedDiagnostics(Opts, *Diags, - Opts->DiagnosticSerializationFile); + if (!Opts.DiagnosticSerializationFile.empty()) + SetupSerializedDiagnostics(Opts, *Diags, Opts.DiagnosticSerializationFile); // Configure our handling of diagnostics. - ProcessWarningOptions(*Diags, *Opts, VFS); + ProcessWarningOptions(*Diags, Opts, VFS); return Diags; } |