Author: teemperor Date: Thu Oct 10 01:30:10 2019 New Revision: 374289 URL: http://llvm.org/viewvc/llvm-project?rev=374289&view=rev Log: [lldb][NFC] Use unique_ptr in DiagnosticManager to express ownership
Modified: lldb/trunk/include/lldb/Expression/DiagnosticManager.h lldb/trunk/source/Expression/DiagnosticManager.cpp lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp lldb/trunk/unittests/Expression/DiagnosticManagerTest.cpp Modified: lldb/trunk/include/lldb/Expression/DiagnosticManager.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/DiagnosticManager.h?rev=374289&r1=374288&r2=374289&view=diff ============================================================================== --- lldb/trunk/include/lldb/Expression/DiagnosticManager.h (original) +++ lldb/trunk/include/lldb/Expression/DiagnosticManager.h Thu Oct 10 01:30:10 2019 @@ -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, ...) Modified: lldb/trunk/source/Expression/DiagnosticManager.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/DiagnosticManager.cpp?rev=374289&r1=374288&r2=374289&view=diff ============================================================================== --- lldb/trunk/source/Expression/DiagnosticManager.cpp (original) +++ lldb/trunk/source/Expression/DiagnosticManager.cpp Thu Oct 10 01:30:10 2019 @@ -47,7 +47,7 @@ static const char *StringForSeverity(Dia 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); Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp?rev=374289&r1=374288&r2=374289&view=diff ============================================================================== --- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp (original) +++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp Thu Oct 10 01:30:10 2019 @@ -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::RewriteExpre 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. Modified: lldb/trunk/unittests/Expression/DiagnosticManagerTest.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Expression/DiagnosticManagerTest.cpp?rev=374289&r1=374288&r2=374289&view=diff ============================================================================== --- lldb/trunk/unittests/Expression/DiagnosticManagerTest.cpp (original) +++ lldb/trunk/unittests/Expression/DiagnosticManagerTest.cpp Thu Oct 10 01:30:10 2019 @@ -39,17 +39,19 @@ TEST(DiagnosticManagerTest, AddDiagnosti 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, GetStringNoD 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, GetStringMul 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, GetStringSev 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, GetStringPre 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, AppendMessag 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, AppendMessag 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()); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits