From 7c47b4a113047af971252da41548b2162e5dcbe3 Mon Sep 17 00:00:00 2001 From: Raphael Isemann Date: Thu, 10 Oct 2019 08:30:10 +0000 Subject: [PATCH] [lldb][NFC] Use unique_ptr in DiagnosticManager to express ownership llvm-svn: 374289 --- lldb/include/lldb/Expression/DiagnosticManager.h | 28 +++------ lldb/source/Expression/DiagnosticManager.cpp | 2 +- .../Clang/ClangExpressionParser.cpp | 11 ++-- .../unittests/Expression/DiagnosticManagerTest.cpp | 72 +++++++++++++--------- 4 files changed, 61 insertions(+), 52 deletions(-) diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h b/lldb/include/lldb/Expression/DiagnosticManager.h index 91fe8a4..e5aecce 100644 --- a/lldb/include/lldb/Expression/DiagnosticManager.h +++ b/lldb/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 @@ -87,7 +88,7 @@ protected: uint32_t m_compiler_id; // Compiler-specific diagnostic ID }; -typedef std::vector DiagnosticList; +typedef std::vector> 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 &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(message, severity, origin, compiler_id)); } - void AddDiagnostic(Diagnostic *diagnostic) { - m_diagnostics.push_back(diagnostic); + void AddDiagnostic(std::unique_ptr diagnostic) { + m_diagnostics.push_back(std::move(diagnostic)); } size_t Printf(DiagnosticSeverity severity, const char *format, ...) diff --git a/lldb/source/Expression/DiagnosticManager.cpp b/lldb/source/Expression/DiagnosticManager.cpp index 5333e3e..48eba35 100644 --- a/lldb/source/Expression/DiagnosticManager.cpp +++ b/lldb/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/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp index db3f139..6e2a379 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ b/lldb/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( + 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(diag); + for (const auto &diag : diagnostic_manager.Diagnostics()) { + const auto *diagnostic = llvm::dyn_cast(diag.get()); if (diagnostic && diagnostic->HasFixIts()) { for (const FixItHint &fixit : diagnostic->FixIts()) { // This is cobbed from clang::Rewrite::FixItRewriter. diff --git a/lldb/unittests/Expression/DiagnosticManagerTest.cpp b/lldb/unittests/Expression/DiagnosticManagerTest.cpp index 843d555..f012632 100644 --- a/lldb/unittests/Expression/DiagnosticManagerTest.cpp +++ b/lldb/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(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("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("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("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("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("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("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("abc", eDiagnosticSeverityError)); EXPECT_EQ("error: abc\n", mgr.GetString()); - mgr.AddDiagnostic(new TextDiag("def", eDiagnosticSeverityError)); + mgr.AddDiagnostic( + std::make_unique("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("foo", eDiagnosticSeverityError)); + mgr.AddDiagnostic( + std::make_unique("bar", eDiagnosticSeverityWarning)); // Remarks have no labels. - mgr.AddDiagnostic(new TextDiag("baz", eDiagnosticSeverityRemark)); + mgr.AddDiagnostic( + std::make_unique("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("baz", eDiagnosticSeverityRemark)); + mgr.AddDiagnostic( + std::make_unique("bar", eDiagnosticSeverityWarning)); + mgr.AddDiagnostic( + std::make_unique("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("foo", eDiagnosticSeverityError)); + mgr.AddDiagnostic( + std::make_unique("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("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("foo", eDiagnosticSeverityError)); EXPECT_EQ("error: bar\nmessage text\nerror: foo\n", mgr.GetString()); } -- 2.7.4