[lldb][NFC] Use unique_ptr in DiagnosticManager to express ownership

git-svn-id: https://llvm.org/svn/llvm-project/lldb/trunk@374289 91177308-0d34-0410-b5e6-96231b3b80d8

4 files changed, 61 insertions, 52 deletions

diff --git a/include/lldb/Expression/DiagnosticManager.h b/include/lldb/Expression/DiagnosticManager.h
index 91fe8a4b9..e5aecce08 100644
--- a/include/lldb/Expression/DiagnosticManager.h
+++ b/include/lldb/Expression/DiagnosticManager.h
@@ -12,6 +12,7 @@
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-types.h"
 
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 
 #include <string>
@@ -87,7 +88,7 @@ protected:
   uint32_t m_compiler_id; // Compiler-specific diagnostic ID
 };
 
-typedef std::vector<Diagnostic *> DiagnosticList;
+typedef std::vector<std::unique_ptr<Diagnostic>> DiagnosticList;
 
 class DiagnosticManager {
 public:
@@ -96,33 +97,24 @@ public:
     m_fixed_expression.clear();
   }
 
-  // The diagnostic manager holds a list of diagnostics, which are owned by the
-  // manager.
   const DiagnosticList &Diagnostics() { return m_diagnostics; }
 
-  ~DiagnosticManager() {
-    for (Diagnostic *diag : m_diagnostics) {
-      delete diag;
-    }
-  }
-
   bool HasFixIts() const {
-    for (Diagnostic *diag : m_diagnostics) {
-      if (diag->HasFixIts())
-        return true;
-    }
-    return false;
+    return llvm::any_of(m_diagnostics,
+                        [](const std::unique_ptr<Diagnostic> &diag) {
+                          return diag->HasFixIts();
+                        });
   }
 
   void AddDiagnostic(llvm::StringRef message, DiagnosticSeverity severity,
                      DiagnosticOrigin origin,
                      uint32_t compiler_id = LLDB_INVALID_COMPILER_ID) {
-    m_diagnostics.push_back(
-        new Diagnostic(message, severity, origin, compiler_id));
+    m_diagnostics.emplace_back(
+        std::make_unique<Diagnostic>(message, severity, origin, compiler_id));
   }
 
-  void AddDiagnostic(Diagnostic *diagnostic) {
-    m_diagnostics.push_back(diagnostic);
+  void AddDiagnostic(std::unique_ptr<Diagnostic> diagnostic) {
+    m_diagnostics.push_back(std::move(diagnostic));
   }
 
   size_t Printf(DiagnosticSeverity severity, const char *format, ...)
diff --git a/source/Expression/DiagnosticManager.cpp b/source/Expression/DiagnosticManager.cpp
index 5333e3e54..48eba3586 100644
--- a/source/Expression/DiagnosticManager.cpp
+++ b/source/Expression/DiagnosticManager.cpp
@@ -47,7 +47,7 @@ static const char *StringForSeverity(DiagnosticSeverity severity) {
 std::string DiagnosticManager::GetString(char separator) {
   std::string ret;
 
-  for (const Diagnostic *diagnostic : Diagnostics()) {
+  for (const auto &diagnostic : Diagnostics()) {
     ret.append(StringForSeverity(diagnostic->GetSeverity()));
     ret.append(diagnostic->GetMessage());
     ret.push_back(separator);
diff --git a/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index db3f13947..6e2a379d9 100644
--- a/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -208,9 +208,8 @@ public:
       // around them.
       std::string stripped_output = llvm::StringRef(m_output).trim();
 
-      ClangDiagnostic *new_diagnostic =
-          new ClangDiagnostic(stripped_output, severity, Info.getID());
-      m_manager->AddDiagnostic(new_diagnostic);
+      auto new_diagnostic = std::make_unique<ClangDiagnostic>(
+          stripped_output, severity, Info.getID());
 
       // Don't store away warning fixits, since the compiler doesn't have
       // enough context in an expression for the warning to be useful.
@@ -224,6 +223,8 @@ public:
             new_diagnostic->AddFixitHint(fixit);
         }
       }
+
+      m_manager->AddDiagnostic(std::move(new_diagnostic));
     }
   }
 
@@ -1100,8 +1101,8 @@ bool ClangExpressionParser::RewriteExpression(
   if (num_diags == 0)
     return false;
 
-  for (const Diagnostic *diag : diagnostic_manager.Diagnostics()) {
-    const ClangDiagnostic *diagnostic = llvm::dyn_cast<ClangDiagnostic>(diag);
+  for (const auto &diag : diagnostic_manager.Diagnostics()) {
+    const auto *diagnostic = llvm::dyn_cast<ClangDiagnostic>(diag.get());
     if (diagnostic && diagnostic->HasFixIts()) {
       for (const FixItHint &fixit : diagnostic->FixIts()) {
         // This is cobbed from clang::Rewrite::FixItRewriter.
diff --git a/unittests/Expression/DiagnosticManagerTest.cpp b/unittests/Expression/DiagnosticManagerTest.cpp
index 843d55503..f012632e6 100644
--- a/unittests/Expression/DiagnosticManagerTest.cpp
+++ b/unittests/Expression/DiagnosticManagerTest.cpp
@@ -39,17 +39,19 @@ TEST(DiagnosticManagerTest, AddDiagnostic) {
   DiagnosticManager mgr;
   EXPECT_EQ(0U, mgr.Diagnostics().size());
 
-  Diagnostic *diag = new Diagnostic(
-      "foo bar has happened", DiagnosticSeverity::eDiagnosticSeverityError,
-      DiagnosticOrigin::eDiagnosticOriginLLDB, custom_diag_id);
-  mgr.AddDiagnostic(diag);
+  std::string msg = "foo bar has happened";
+  DiagnosticSeverity severity = DiagnosticSeverity::eDiagnosticSeverityError;
+  DiagnosticOrigin origin = DiagnosticOrigin::eDiagnosticOriginLLDB;
+  auto diag =
+      std::make_unique<Diagnostic>(msg, severity, origin, custom_diag_id);
+  mgr.AddDiagnostic(std::move(diag));
   EXPECT_EQ(1U, mgr.Diagnostics().size());
-  Diagnostic *got = mgr.Diagnostics().front();
-  EXPECT_EQ(diag->getKind(), got->getKind());
-  EXPECT_EQ(diag->GetMessage(), got->GetMessage());
-  EXPECT_EQ(diag->GetSeverity(), got->GetSeverity());
-  EXPECT_EQ(diag->GetCompilerID(), got->GetCompilerID());
-  EXPECT_EQ(diag->HasFixIts(), got->HasFixIts());
+  const Diagnostic *got = mgr.Diagnostics().front().get();
+  EXPECT_EQ(DiagnosticOrigin::eDiagnosticOriginLLDB, got->getKind());
+  EXPECT_EQ(msg, got->GetMessage());
+  EXPECT_EQ(severity, got->GetSeverity());
+  EXPECT_EQ(custom_diag_id, got->GetCompilerID());
+  EXPECT_EQ(false, got->HasFixIts());
 }
 
 TEST(DiagnosticManagerTest, HasFixits) {
@@ -57,16 +59,16 @@ TEST(DiagnosticManagerTest, HasFixits) {
   // By default we shouldn't have any fixits.
   EXPECT_FALSE(mgr.HasFixIts());
   // Adding a diag without fixits shouldn't make HasFixIts return true.
-  mgr.AddDiagnostic(new FixItDiag("no fixit", false));
+  mgr.AddDiagnostic(std::make_unique<FixItDiag>("no fixit", false));
   EXPECT_FALSE(mgr.HasFixIts());
   // Adding a diag with fixits will mark the manager as containing fixits.
-  mgr.AddDiagnostic(new FixItDiag("fixit", true));
+  mgr.AddDiagnostic(std::make_unique<FixItDiag>("fixit", true));
   EXPECT_TRUE(mgr.HasFixIts());
   // Adding another diag without fixit shouldn't make it return false.
-  mgr.AddDiagnostic(new FixItDiag("no fixit", false));
+  mgr.AddDiagnostic(std::make_unique<FixItDiag>("no fixit", false));
   EXPECT_TRUE(mgr.HasFixIts());
   // Adding a diag with fixits. The manager should still return true.
-  mgr.AddDiagnostic(new FixItDiag("fixit", true));
+  mgr.AddDiagnostic(std::make_unique<FixItDiag>("fixit", true));
   EXPECT_TRUE(mgr.HasFixIts());
 }
 
@@ -77,7 +79,8 @@ TEST(DiagnosticManagerTest, GetStringNoDiags) {
 
 TEST(DiagnosticManagerTest, GetStringBasic) {
   DiagnosticManager mgr;
-  mgr.AddDiagnostic(new TextDiag("abc", eDiagnosticSeverityError));
+  mgr.AddDiagnostic(
+      std::make_unique<TextDiag>("abc", eDiagnosticSeverityError));
   EXPECT_EQ("error: abc\n", mgr.GetString());
 }
 
@@ -85,15 +88,18 @@ TEST(DiagnosticManagerTest, GetStringMultiline) {
   DiagnosticManager mgr;
 
   // Multiline diagnostics should only get one severity label.
-  mgr.AddDiagnostic(new TextDiag("b\nc", eDiagnosticSeverityError));
+  mgr.AddDiagnostic(
+      std::make_unique<TextDiag>("b\nc", eDiagnosticSeverityError));
   EXPECT_EQ("error: b\nc\n", mgr.GetString());
 }
 
 TEST(DiagnosticManagerTest, GetStringMultipleDiags) {
   DiagnosticManager mgr;
-  mgr.AddDiagnostic(new TextDiag("abc", eDiagnosticSeverityError));
+  mgr.AddDiagnostic(
+      std::make_unique<TextDiag>("abc", eDiagnosticSeverityError));
   EXPECT_EQ("error: abc\n", mgr.GetString());
-  mgr.AddDiagnostic(new TextDiag("def", eDiagnosticSeverityError));
+  mgr.AddDiagnostic(
+      std::make_unique<TextDiag>("def", eDiagnosticSeverityError));
   EXPECT_EQ("error: abc\nerror: def\n", mgr.GetString());
 }
 
@@ -101,10 +107,13 @@ TEST(DiagnosticManagerTest, GetStringSeverityLabels) {
   DiagnosticManager mgr;
 
   // Different severities should cause different labels.
-  mgr.AddDiagnostic(new TextDiag("foo", eDiagnosticSeverityError));
-  mgr.AddDiagnostic(new TextDiag("bar", eDiagnosticSeverityWarning));
+  mgr.AddDiagnostic(
+      std::make_unique<TextDiag>("foo", eDiagnosticSeverityError));
+  mgr.AddDiagnostic(
+      std::make_unique<TextDiag>("bar", eDiagnosticSeverityWarning));
   // Remarks have no labels.
-  mgr.AddDiagnostic(new TextDiag("baz", eDiagnosticSeverityRemark));
+  mgr.AddDiagnostic(
+      std::make_unique<TextDiag>("baz", eDiagnosticSeverityRemark));
   EXPECT_EQ("error: foo\nwarning: bar\nbaz\n", mgr.GetString());
 }
 
@@ -112,9 +121,12 @@ TEST(DiagnosticManagerTest, GetStringPreserveOrder) {
   DiagnosticManager mgr;
 
   // Make sure we preserve the diagnostic order and do not sort them in any way.
-  mgr.AddDiagnostic(new TextDiag("baz", eDiagnosticSeverityRemark));
-  mgr.AddDiagnostic(new TextDiag("bar", eDiagnosticSeverityWarning));
-  mgr.AddDiagnostic(new TextDiag("foo", eDiagnosticSeverityError));
+  mgr.AddDiagnostic(
+      std::make_unique<TextDiag>("baz", eDiagnosticSeverityRemark));
+  mgr.AddDiagnostic(
+      std::make_unique<TextDiag>("bar", eDiagnosticSeverityWarning));
+  mgr.AddDiagnostic(
+      std::make_unique<TextDiag>("foo", eDiagnosticSeverityError));
   EXPECT_EQ("baz\nwarning: bar\nerror: foo\n", mgr.GetString());
 }
 
@@ -129,8 +141,10 @@ TEST(DiagnosticManagerTest, AppendMessageNoDiag) {
 TEST(DiagnosticManagerTest, AppendMessageAttachToLastDiag) {
   DiagnosticManager mgr;
 
-  mgr.AddDiagnostic(new TextDiag("foo", eDiagnosticSeverityError));
-  mgr.AddDiagnostic(new TextDiag("bar", eDiagnosticSeverityError));
+  mgr.AddDiagnostic(
+      std::make_unique<TextDiag>("foo", eDiagnosticSeverityError));
+  mgr.AddDiagnostic(
+      std::make_unique<TextDiag>("bar", eDiagnosticSeverityError));
   // This should append to 'bar' and not to 'foo'.
   mgr.AppendMessageToDiagnostic("message text");
 
@@ -140,10 +154,12 @@ TEST(DiagnosticManagerTest, AppendMessageAttachToLastDiag) {
 TEST(DiagnosticManagerTest, AppendMessageSubsequentDiags) {
   DiagnosticManager mgr;
 
-  mgr.AddDiagnostic(new TextDiag("bar", eDiagnosticSeverityError));
+  mgr.AddDiagnostic(
+      std::make_unique<TextDiag>("bar", eDiagnosticSeverityError));
   mgr.AppendMessageToDiagnostic("message text");
   // Pushing another diag after the message should work fine.
-  mgr.AddDiagnostic(new TextDiag("foo", eDiagnosticSeverityError));
+  mgr.AddDiagnostic(
+      std::make_unique<TextDiag>("foo", eDiagnosticSeverityError));
 
   EXPECT_EQ("error: bar\nmessage text\nerror: foo\n", mgr.GetString());
 }