Skip to content

[lldb] Create a single Severity enum in lldb-enumerations #90917

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 3, 2024
Merged

[lldb] Create a single Severity enum in lldb-enumerations #90917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
23b16ba
[lldb] Create a single Severity enum in lldb-enumerations
JDevlieghere May 2, 2024
96772fa
Add remark comment
JDevlieghere May 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions lldb/include/lldb/Core/Debugger.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,8 +630,7 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
std::optional<lldb::user_id_t> debugger_id,
uint32_t progress_category_bit = eBroadcastBitProgress);

static void ReportDiagnosticImpl(DiagnosticEventData::Type type,
std::string message,
static void ReportDiagnosticImpl(lldb::Severity severity, std::string message,
std::optional<lldb::user_id_t> debugger_id,
std::once_flag *once);

Expand Down
14 changes: 5 additions & 9 deletions lldb/include/lldb/Core/DebuggerEvents.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,19 +76,15 @@ class ProgressEventData : public EventData {

class DiagnosticEventData : public EventData {
public:
enum class Type {
Info,
Warning,
Error,
};
DiagnosticEventData(Type type, std::string message, bool debugger_specific)
: m_message(std::move(message)), m_type(type),
DiagnosticEventData(lldb::Severity severity, std::string message,
bool debugger_specific)
: m_message(std::move(message)), m_severity(severity),
m_debugger_specific(debugger_specific) {}
~DiagnosticEventData() override = default;

const std::string &GetMessage() const { return m_message; }
bool IsDebuggerSpecific() const { return m_debugger_specific; }
Type GetType() const { return m_type; }
lldb::Severity GetSeverity() const { return m_severity; }

llvm::StringRef GetPrefix() const;

Expand All @@ -105,7 +101,7 @@ class DiagnosticEventData : public EventData {

protected:
std::string m_message;
Type m_type;
lldb::Severity m_severity;
const bool m_debugger_specific;

DiagnosticEventData(const DiagnosticEventData &) = delete;
Expand Down
18 changes: 6 additions & 12 deletions lldb/include/lldb/Expression/DiagnosticManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,6 @@ enum DiagnosticOrigin {
eDiagnosticOriginLLVM
};

enum DiagnosticSeverity {
eDiagnosticSeverityError,
eDiagnosticSeverityWarning,
eDiagnosticSeverityRemark
};

const uint32_t LLDB_INVALID_COMPILER_ID = UINT32_MAX;

class Diagnostic {
Expand All @@ -55,7 +49,7 @@ class Diagnostic {
}
}

Diagnostic(llvm::StringRef message, DiagnosticSeverity severity,
Diagnostic(llvm::StringRef message, lldb::Severity severity,
DiagnosticOrigin origin, uint32_t compiler_id)
: m_message(message), m_severity(severity), m_origin(origin),
m_compiler_id(compiler_id) {}
Expand All @@ -68,7 +62,7 @@ class Diagnostic {

virtual bool HasFixIts() const { return false; }

DiagnosticSeverity GetSeverity() const { return m_severity; }
lldb::Severity GetSeverity() const { return m_severity; }

uint32_t GetCompilerID() const { return m_compiler_id; }

Expand All @@ -83,7 +77,7 @@ class Diagnostic {

protected:
std::string m_message;
DiagnosticSeverity m_severity;
lldb::Severity m_severity;
DiagnosticOrigin m_origin;
uint32_t m_compiler_id; // Compiler-specific diagnostic ID
};
Expand All @@ -106,7 +100,7 @@ class DiagnosticManager {
});
}

void AddDiagnostic(llvm::StringRef message, DiagnosticSeverity severity,
void AddDiagnostic(llvm::StringRef message, lldb::Severity severity,
DiagnosticOrigin origin,
uint32_t compiler_id = LLDB_INVALID_COMPILER_ID) {
m_diagnostics.emplace_back(
Expand All @@ -127,9 +121,9 @@ class DiagnosticManager {
other.Clear();
}

size_t Printf(DiagnosticSeverity severity, const char *format, ...)
size_t Printf(lldb::Severity severity, const char *format, ...)
__attribute__((format(printf, 3, 4)));
void PutString(DiagnosticSeverity severity, llvm::StringRef str);
void PutString(lldb::Severity severity, llvm::StringRef str);

void AppendMessageToDiagnostic(llvm::StringRef str) {
if (!m_diagnostics.empty())
Expand Down
9 changes: 1 addition & 8 deletions lldb/include/lldb/Host/Host.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +87,8 @@ class Host {
StartMonitoringChildProcess(const MonitorChildProcessCallback &callback,
lldb::pid_t pid);

/// System log level.
enum SystemLogLevel {
eSystemLogInfo,
eSystemLogWarning,
eSystemLogError,
};

/// Emit the given message to the operating system log.
static void SystemLog(SystemLogLevel log_level, llvm::StringRef message);
static void SystemLog(lldb::Severity severity, llvm::StringRef message);

/// Get the process ID for the calling process.
///
Expand Down
7 changes: 7 additions & 0 deletions lldb/include/lldb/lldb-enumerations.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1347,6 +1347,13 @@ enum DebuggerBroadcastBit {
eBroadcastBitProgressCategory = (1 << 3),
};

/// Used for expressing severity in logs and diagnostics.
enum Severity {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would enum class prevent people from accidentally casting this to an unrelated enum with similar purpose but potentially different values?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would, but lldb-enumerations.h exclusively uses enum and I think consistency outweighs the small benefit of using enum class. FWIW this came up previously in #80167 (comment) too.

eSeverityError,
eSeverityWarning,
eSeverityInfo, // Equivalent to Remark used in clang.
};

} // namespace lldb

#endif // LLDB_LLDB_ENUMERATIONS_H
37 changes: 17 additions & 20 deletions lldb/source/Core/Debugger.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1476,19 +1476,18 @@ void Debugger::ReportProgress(uint64_t progress_id, std::string title,
}
}

static void PrivateReportDiagnostic(Debugger &debugger,
DiagnosticEventData::Type type,
static void PrivateReportDiagnostic(Debugger &debugger, Severity severity,
std::string message,
bool debugger_specific) {
uint32_t event_type = 0;
switch (type) {
case DiagnosticEventData::Type::Info:
assert(false && "DiagnosticEventData::Type::Info should not be broadcast");
switch (severity) {
case eSeverityInfo:
assert(false && "eSeverityInfo should not be broadcast");
return;
case DiagnosticEventData::Type::Warning:
case eSeverityWarning:
event_type = Debugger::eBroadcastBitWarning;
break;
case DiagnosticEventData::Type::Error:
case eSeverityError:
event_type = Debugger::eBroadcastBitError;
break;
}
Expand All @@ -1497,19 +1496,19 @@ static void PrivateReportDiagnostic(Debugger &debugger,
if (!broadcaster.EventTypeHasListeners(event_type)) {
// Diagnostics are too important to drop. If nobody is listening, print the
// diagnostic directly to the debugger's error stream.
DiagnosticEventData event_data(type, std::move(message), debugger_specific);
DiagnosticEventData event_data(severity, std::move(message),
debugger_specific);
StreamSP stream = debugger.GetAsyncErrorStream();
event_data.Dump(stream.get());
return;
}
EventSP event_sp = std::make_shared<Event>(
event_type,
new DiagnosticEventData(type, std::move(message), debugger_specific));
new DiagnosticEventData(severity, std::move(message), debugger_specific));
broadcaster.BroadcastEvent(event_sp);
}

void Debugger::ReportDiagnosticImpl(DiagnosticEventData::Type type,
std::string message,
void Debugger::ReportDiagnosticImpl(Severity severity, std::string message,
std::optional<lldb::user_id_t> debugger_id,
std::once_flag *once) {
auto ReportDiagnosticLambda = [&]() {
Expand All @@ -1519,7 +1518,7 @@ void Debugger::ReportDiagnosticImpl(DiagnosticEventData::Type type,
Diagnostics::Instance().Report(message);

// We don't broadcast info events.
if (type == DiagnosticEventData::Type::Info)
if (severity == lldb::eSeverityInfo)
return;

// Check if this diagnostic is for a specific debugger.
Expand All @@ -1528,15 +1527,16 @@ void Debugger::ReportDiagnosticImpl(DiagnosticEventData::Type type,
// still exists.
DebuggerSP debugger_sp = FindDebuggerWithID(*debugger_id);
if (debugger_sp)
PrivateReportDiagnostic(*debugger_sp, type, std::move(message), true);
PrivateReportDiagnostic(*debugger_sp, severity, std::move(message),
true);
return;
}
// The diagnostic event is not debugger specific, iterate over all debuggers
// and deliver a diagnostic event to each one.
if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
std::lock_guard<std::recursive_mutex> guard(*g_debugger_list_mutex_ptr);
for (const auto &debugger : *g_debugger_list_ptr)
PrivateReportDiagnostic(*debugger, type, message, false);
PrivateReportDiagnostic(*debugger, severity, message, false);
}
};

Expand All @@ -1549,22 +1549,19 @@ void Debugger::ReportDiagnosticImpl(DiagnosticEventData::Type type,
void Debugger::ReportWarning(std::string message,
std::optional<lldb::user_id_t> debugger_id,
std::once_flag *once) {
ReportDiagnosticImpl(DiagnosticEventData::Type::Warning, std::move(message),
debugger_id, once);
ReportDiagnosticImpl(eSeverityWarning, std::move(message), debugger_id, once);
}

void Debugger::ReportError(std::string message,
std::optional<lldb::user_id_t> debugger_id,
std::once_flag *once) {
ReportDiagnosticImpl(DiagnosticEventData::Type::Error, std::move(message),
debugger_id, once);
ReportDiagnosticImpl(eSeverityError, std::move(message), debugger_id, once);
}

void Debugger::ReportInfo(std::string message,
std::optional<lldb::user_id_t> debugger_id,
std::once_flag *once) {
ReportDiagnosticImpl(DiagnosticEventData::Type::Info, std::move(message),
debugger_id, once);
ReportDiagnosticImpl(eSeverityInfo, std::move(message), debugger_id, once);
}

void Debugger::ReportSymbolChange(const ModuleSpec &module_spec) {
Expand Down
10 changes: 5 additions & 5 deletions lldb/source/Core/DebuggerEvents.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,19 +73,19 @@ ProgressEventData::GetAsStructuredData(const Event *event_ptr) {
}

llvm::StringRef DiagnosticEventData::GetPrefix() const {
switch (m_type) {
case Type::Info:
switch (m_severity) {
case Severity::eSeverityInfo:
return "info";
case Type::Warning:
case Severity::eSeverityWarning:
return "warning";
case Type::Error:
case Severity::eSeverityError:
return "error";
}
llvm_unreachable("Fully covered switch above!");
}

void DiagnosticEventData::Dump(Stream *s) const {
llvm::HighlightColor color = m_type == Type::Warning
llvm::HighlightColor color = m_severity == lldb::eSeverityWarning
? llvm::HighlightColor::Warning
: llvm::HighlightColor::Error;
llvm::WithColor(s->AsRawOstream(), color, llvm::ColorMode::Enable)
Expand Down
16 changes: 8 additions & 8 deletions lldb/source/Expression/DiagnosticManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,17 @@ void DiagnosticManager::Dump(Log *log) {
log->PutCString(str.c_str());
}

static const char *StringForSeverity(DiagnosticSeverity severity) {
static const char *StringForSeverity(lldb::Severity severity) {
switch (severity) {
// this should be exhaustive
case lldb_private::eDiagnosticSeverityError:
case lldb::eSeverityError:
return "error: ";
case lldb_private::eDiagnosticSeverityWarning:
case lldb::eSeverityWarning:
return "warning: ";
case lldb_private::eDiagnosticSeverityRemark:
case lldb::eSeverityInfo:
return "";
}
llvm_unreachable("switch needs another case for DiagnosticSeverity enum");
llvm_unreachable("switch needs another case for lldb::Severity enum");
}

std::string DiagnosticManager::GetString(char separator) {
Expand All @@ -65,8 +65,8 @@ std::string DiagnosticManager::GetString(char separator) {
return ret;
}

size_t DiagnosticManager::Printf(DiagnosticSeverity severity,
const char *format, ...) {
size_t DiagnosticManager::Printf(lldb::Severity severity, const char *format,
...) {
StreamString ss;

va_list args;
Expand All @@ -79,7 +79,7 @@ size_t DiagnosticManager::Printf(DiagnosticSeverity severity,
return result;
}

void DiagnosticManager::PutString(DiagnosticSeverity severity,
void DiagnosticManager::PutString(lldb::Severity severity,
llvm::StringRef str) {
if (str.empty())
return;
Expand Down
25 changes: 11 additions & 14 deletions lldb/source/Expression/FunctionCaller.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,27 +67,25 @@ bool FunctionCaller::WriteFunctionWrapper(
Process *process = exe_ctx.GetProcessPtr();

if (!process) {
diagnostic_manager.Printf(eDiagnosticSeverityError, "no process.");
diagnostic_manager.Printf(lldb::eSeverityError, "no process.");
return false;
}

lldb::ProcessSP jit_process_sp(m_jit_process_wp.lock());

if (process != jit_process_sp.get()) {
diagnostic_manager.Printf(eDiagnosticSeverityError,
"process does not match the stored process.");
diagnostic_manager.Printf(lldb::eSeverityError,
"process does not match the stored process.");
return false;
}

if (process->GetState() != lldb::eStateStopped) {
diagnostic_manager.Printf(eDiagnosticSeverityError,
"process is not stopped");
diagnostic_manager.Printf(lldb::eSeverityError, "process is not stopped");
return false;
}

if (!m_compiled) {
diagnostic_manager.Printf(eDiagnosticSeverityError,
"function not compiled");
diagnostic_manager.Printf(lldb::eSeverityError, "function not compiled");
return false;
}

Expand All @@ -101,7 +99,7 @@ bool FunctionCaller::WriteFunctionWrapper(
can_interpret, eExecutionPolicyAlways));

if (!jit_error.Success()) {
diagnostic_manager.Printf(eDiagnosticSeverityError,
diagnostic_manager.Printf(lldb::eSeverityError,
"Error in PrepareForExecution: %s.",
jit_error.AsCString());
return false;
Expand Down Expand Up @@ -144,7 +142,7 @@ bool FunctionCaller::WriteFunctionArguments(
// All the information to reconstruct the struct is provided by the
// StructExtractor.
if (!m_struct_valid) {
diagnostic_manager.PutString(eDiagnosticSeverityError,
diagnostic_manager.PutString(lldb::eSeverityError,
"Argument information was not correctly "
"parsed, so the function cannot be called.");
return false;
Expand Down Expand Up @@ -192,7 +190,7 @@ bool FunctionCaller::WriteFunctionArguments(
size_t num_args = arg_values.GetSize();
if (num_args != m_arg_values.GetSize()) {
diagnostic_manager.Printf(
eDiagnosticSeverityError,
lldb::eSeverityError,
"Wrong number of arguments - was: %" PRIu64 " should be: %" PRIu64 "",
(uint64_t)num_args, (uint64_t)m_arg_values.GetSize());
return false;
Expand Down Expand Up @@ -231,11 +229,11 @@ bool FunctionCaller::InsertFunction(ExecutionContext &exe_ctx,
// the caller, we need to be stopped.
Process *process = exe_ctx.GetProcessPtr();
if (!process) {
diagnostic_manager.PutString(eDiagnosticSeverityError, "no process");
diagnostic_manager.PutString(lldb::eSeverityError, "no process");
return false;
}
if (process->GetState() != lldb::eStateStopped) {
diagnostic_manager.PutString(eDiagnosticSeverityError, "process running");
diagnostic_manager.PutString(lldb::eSeverityError, "process running");
return false;
}
if (CompileFunction(exe_ctx.GetThreadSP(), diagnostic_manager) != 0)
Expand Down Expand Up @@ -267,8 +265,7 @@ lldb::ThreadPlanSP FunctionCaller::GetThreadPlanToCallFunction(
Thread *thread = exe_ctx.GetThreadPtr();
if (thread == nullptr) {
diagnostic_manager.PutString(
eDiagnosticSeverityError,
"Can't call a function without a valid thread.");
lldb::eSeverityError, "Can't call a function without a valid thread.");
return nullptr;
}

Expand Down
Loading