[clang] [clang-tools-extra] [clang] Hide the `DiagnosticOptions` pointer from `CompilerInvocation` (PR #106274)

Jan Svoboda via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 27 12:22:05 PDT 2024


https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/106274

This PR hides the reference-counter pointer that holds `DiagnosticOptions` from the public API of `CompilerInvocation`. This gives `CompilerInvocation` an exclusive control over the lifetime of this member, which will eventually be leveraged to implement a copy-on-write behavior.

The only client that currently accesses that pointer is `clangd::buildPreamble()` which takes care to reset it so that it's not reset concurrently. This code is made redundant by making the reference count of `DiagnosticOptions` atomic.

>From 49fabcd6a91053633dbed7a56696a56b87e25518 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Tue, 27 Aug 2024 11:57:05 -0700
Subject: [PATCH 1/2] [clang] Count references to `DiagnosticOptions`
 atomically

---
 clang-tools-extra/clangd/Preamble.cpp         | 1 -
 clang/include/clang/Basic/DiagnosticOptions.h | 3 ++-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp
index dd13b1a9e5613d..51d478cf4efc1d 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -673,7 +673,6 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
   // Reset references to ref-counted-ptrs before executing the callbacks, to
   // prevent resetting them concurrently.
   PreambleDiagsEngine.reset();
-  CI.DiagnosticOpts.reset();
 
   // When building the AST for the main file, we do want the function
   // bodies.
diff --git a/clang/include/clang/Basic/DiagnosticOptions.h b/clang/include/clang/Basic/DiagnosticOptions.h
index 30141c2b8f4475..ea8da70e081f01 100644
--- a/clang/include/clang/Basic/DiagnosticOptions.h
+++ b/clang/include/clang/Basic/DiagnosticOptions.h
@@ -67,7 +67,8 @@ inline DiagnosticLevelMask operator&(DiagnosticLevelMask LHS,
 raw_ostream& operator<<(raw_ostream& Out, DiagnosticLevelMask M);
 
 /// Options for controlling the compiler diagnostics engine.
-class DiagnosticOptions : public RefCountedBase<DiagnosticOptions>{
+class DiagnosticOptions
+    : public llvm::ThreadSafeRefCountedBase<DiagnosticOptions> {
   friend bool ParseDiagnosticArgs(DiagnosticOptions &, llvm::opt::ArgList &,
                                   clang::DiagnosticsEngine *, bool);
 

>From e0a01b85498dc0ce3fb46349e2f8c0b456c077c4 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Tue, 27 Aug 2024 11:58:22 -0700
Subject: [PATCH 2/2] [clang] Hide `CompilerInvocation::DiagnosticOpts`

---
 clang/include/clang/Frontend/CompilerInvocation.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/clang/include/clang/Frontend/CompilerInvocation.h b/clang/include/clang/Frontend/CompilerInvocation.h
index 9daa0a1ecf9488..1e4d2da86c2bee 100644
--- a/clang/include/clang/Frontend/CompilerInvocation.h
+++ b/clang/include/clang/Frontend/CompilerInvocation.h
@@ -269,7 +269,6 @@ class CompilerInvocation : public CompilerInvocationBase {
   /// @{
   using CompilerInvocationBase::LangOpts;
   using CompilerInvocationBase::TargetOpts;
-  using CompilerInvocationBase::DiagnosticOpts;
   std::shared_ptr<HeaderSearchOptions> getHeaderSearchOptsPtr() {
     return HSOpts;
   }



More information about the cfe-commits mailing list