Skip to content

Conversation

@Michael137
Copy link
Member

This aligns ClangDiagnosticManagerAdapter with how we set up the diagnostics in ClangModulesDeclVendor. We fixed lifetime issues around the same kind of setup here: #167724

This class didn't suffer from the same lifetime issue because it used shared_ptrs. So the stream wasn't freed before ~TextDiagnosticPrinter accessing it. But that begged the question of why these are shared_ptrs in the first place. This patch makes these unique_ptrs and fixes the destruction order that would now be an issue.

…iagnosticManagerAdapter

This aligns `ClangDiagnosticManagerAdapter` with how we set up the diagnostics in `ClangModulesDeclVendor`. We fixed lifetime issues around the same kind of setup here: llvm#167724

This class didn't suffer from the same lifetime issue because it used `shared_ptr`s. So the stream wasn't freed before `~TextDiagnosticPrinter` accessing it. But that begged the question of why these are `shared_ptr`s in the first place. This patch makes these `unique_ptr`s and fixes the destruction order that would now be an issue.
@llvmbot
Copy link
Member

llvmbot commented Nov 12, 2025

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

This aligns ClangDiagnosticManagerAdapter with how we set up the diagnostics in ClangModulesDeclVendor. We fixed lifetime issues around the same kind of setup here: #167724

This class didn't suffer from the same lifetime issue because it used shared_ptrs. So the stream wasn't freed before ~TextDiagnosticPrinter accessing it. But that begged the question of why these are shared_ptrs in the first place. This patch makes these unique_ptrs and fixes the destruction order that would now be an issue.


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

1 Files Affected:

  • (modified) lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp (+5-5)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 75ed87baf636a..c99f6c6e5e2c5 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -171,9 +171,9 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
       : m_options(opts), m_filename(filename) {
     m_options.ShowPresumedLoc = true;
     m_options.ShowLevel = false;
-    m_os = std::make_shared<llvm::raw_string_ostream>(m_output);
+    m_os = std::make_unique<llvm::raw_string_ostream>(m_output);
     m_passthrough =
-        std::make_shared<clang::TextDiagnosticPrinter>(*m_os, m_options);
+        std::make_unique<clang::TextDiagnosticPrinter>(*m_os, m_options);
   }
 
   void ResetManager(DiagnosticManager *manager = nullptr) {
@@ -315,11 +315,11 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
 private:
   DiagnosticManager *m_manager = nullptr;
   DiagnosticOptions m_options;
-  std::shared_ptr<clang::TextDiagnosticPrinter> m_passthrough;
-  /// Output stream of m_passthrough.
-  std::shared_ptr<llvm::raw_string_ostream> m_os;
   /// Output string filled by m_os.
   std::string m_output;
+  /// Output stream of m_passthrough.
+  std::unique_ptr<llvm::raw_string_ostream> m_os;
+  std::unique_ptr<clang::TextDiagnosticPrinter> m_passthrough;
   StringRef m_filename;
 };
 

@Michael137 Michael137 merged commit 87da620 into llvm:main Nov 12, 2025
12 checks passed
@Michael137 Michael137 deleted the lldb/expression-parser-diag-ownership branch November 12, 2025 19:02
git-crd pushed a commit to git-crd/crd-llvm-project that referenced this pull request Nov 13, 2025
…iagnosticManagerAdapter (llvm#167731)

This aligns `ClangDiagnosticManagerAdapter` with how we set up the
diagnostics in `ClangModulesDeclVendor`. We fixed lifetime issues around
the same kind of setup here:
llvm#167724

This class didn't suffer from the same lifetime issue because it used
`shared_ptr`s. So the stream wasn't freed before
`~TextDiagnosticPrinter` accessing it. But that begged the question of
why these are `shared_ptr`s in the first place. This patch makes these
`unique_ptr`s and fixes the destruction order that would now be an
issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants