From 9e306ad4600c4d3392c194a8be88919ee758425c Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 22 May 2025 12:33:52 -0700 Subject: [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 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. --- clang/unittests/Serialization/ModuleCacheTest.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'clang/unittests/Serialization/ModuleCacheTest.cpp') diff --git a/clang/unittests/Serialization/ModuleCacheTest.cpp b/clang/unittests/Serialization/ModuleCacheTest.cpp index 38003e9..de6e13a 100644 --- a/clang/unittests/Serialization/ModuleCacheTest.cpp +++ b/clang/unittests/Serialization/ModuleCacheTest.cpp @@ -108,8 +108,9 @@ TEST_F(ModuleCacheTest, CachedModuleNewPath) { MCPArg.append(ModuleCachePath); CreateInvocationOptions CIOpts; CIOpts.VFS = llvm::vfs::createPhysicalFileSystem(); + DiagnosticOptions DiagOpts; IntrusiveRefCntPtr Diags = - CompilerInstance::createDiagnostics(*CIOpts.VFS, new DiagnosticOptions()); + CompilerInstance::createDiagnostics(*CIOpts.VFS, DiagOpts); CIOpts.Diags = Diags; // First run should pass with no errors @@ -157,8 +158,9 @@ TEST_F(ModuleCacheTest, CachedModuleNewPathAllowErrors) { MCPArg.append(ModuleCachePath); CreateInvocationOptions CIOpts; CIOpts.VFS = llvm::vfs::createPhysicalFileSystem(); + DiagnosticOptions DiagOpts; IntrusiveRefCntPtr Diags = - CompilerInstance::createDiagnostics(*CIOpts.VFS, new DiagnosticOptions()); + CompilerInstance::createDiagnostics(*CIOpts.VFS, DiagOpts); CIOpts.Diags = Diags; // First run should pass with no errors -- cgit v1.1