[Lldb-commits] [lldb] r374289 - [lldb][NFC] Use unique_ptr in DiagnosticManager to express ownership
Raphael Isemann via lldb-commits
lldb-commits at lists.llvm.org
Thu Oct 10 01:30:11 PDT 2019
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());
}
More information about the lldb-commits
mailing list