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

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


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Jan Svoboda (jansvoboda11)

<details>
<summary>Changes</summary>

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/106274.diff


3 Files Affected:

- (modified) clang-tools-extra/clangd/Preamble.cpp (-1) 
- (modified) clang/include/clang/Basic/DiagnosticOptions.h (+2-1) 
- (modified) clang/include/clang/Frontend/CompilerInvocation.h (-1) 


``````````diff
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);
 
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;
   }

``````````

</details>


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


More information about the cfe-commits mailing list