[lldb] Create a single Severity enum in lldb-enumerations #90917
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We have 3 different enums all expressing severity (info, warning, error). Remove all uses with a new Severity enum in lldb-enumerations.h.
|
@llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) ChangesWe have 3 different enums all expressing severity (info, warning, error). Remove all uses with a new Severity enum in lldb-enumerations.h. Patch is 52.97 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/90917.diff 20 Files Affected:
diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index 49ff0737acef82..c0f7c732ad2d46 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -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);
diff --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h
index 74bb05e6e6bf88..49a4ecf8e537e3 100644
--- a/lldb/include/lldb/Core/DebuggerEvents.h
+++ b/lldb/include/lldb/Core/DebuggerEvents.h
@@ -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;
@@ -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;
diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h b/lldb/include/lldb/Expression/DiagnosticManager.h
index 06bf1d115f1541..d49b7c99b114fb 100644
--- a/lldb/include/lldb/Expression/DiagnosticManager.h
+++ b/lldb/include/lldb/Expression/DiagnosticManager.h
@@ -28,12 +28,6 @@ enum DiagnosticOrigin {
eDiagnosticOriginLLVM
};
-enum DiagnosticSeverity {
- eDiagnosticSeverityError,
- eDiagnosticSeverityWarning,
- eDiagnosticSeverityRemark
-};
-
const uint32_t LLDB_INVALID_COMPILER_ID = UINT32_MAX;
class Diagnostic {
@@ -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) {}
@@ -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; }
@@ -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
};
@@ -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(
@@ -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())
diff --git a/lldb/include/lldb/Host/Host.h b/lldb/include/lldb/Host/Host.h
index b0cb477d82fa14..9d0994978402f7 100644
--- a/lldb/include/lldb/Host/Host.h
+++ b/lldb/include/lldb/Host/Host.h
@@ -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.
///
diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h
index 15e45857186091..22dc52b5e32264 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -1347,6 +1347,12 @@ enum DebuggerBroadcastBit {
eBroadcastBitProgressCategory = (1 << 3),
};
+enum Severity {
+ eSeverityError,
+ eSeverityWarning,
+ eSeverityInfo,
+};
+
} // namespace lldb
#endif // LLDB_LLDB_ENUMERATIONS_H
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index cac4642873b772..065f70c3880aaa 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -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;
}
@@ -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 = [&]() {
@@ -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.
@@ -1528,7 +1527,8 @@ 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
@@ -1536,7 +1536,7 @@ void Debugger::ReportDiagnosticImpl(DiagnosticEventData::Type type,
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);
}
};
@@ -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) {
diff --git a/lldb/source/Core/DebuggerEvents.cpp b/lldb/source/Core/DebuggerEvents.cpp
index 65aed0eba9c41a..2fa6efd155af7c 100644
--- a/lldb/source/Core/DebuggerEvents.cpp
+++ b/lldb/source/Core/DebuggerEvents.cpp
@@ -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)
diff --git a/lldb/source/Expression/DiagnosticManager.cpp b/lldb/source/Expression/DiagnosticManager.cpp
index 9a1100df78db2b..a8330138f3d53b 100644
--- a/lldb/source/Expression/DiagnosticManager.cpp
+++ b/lldb/source/Expression/DiagnosticManager.cpp
@@ -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) {
@@ -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;
@@ -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;
diff --git a/lldb/source/Expression/FunctionCaller.cpp b/lldb/source/Expression/FunctionCaller.cpp
index ffadbf9b32ec53..5ac2b0681ebbec 100644
--- a/lldb/source/Expression/FunctionCaller.cpp
+++ b/lldb/source/Expression/FunctionCaller.cpp
@@ -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;
}
@@ -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;
@@ -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;
@@ -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;
@@ -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)
@@ -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;
}
diff --git a/lldb/source/Expression/LLVMUserExpression.cpp b/lldb/source/Expression/LLVMUserExpression.cpp
index 1434011c80ad81..b4fdfc4d1fa8ba 100644
--- a/lldb/source/Expression/LLVMUserExpression.cpp
+++ b/lldb/source/Expression/LLVMUserExpression.cpp
@@ -73,7 +73,7 @@ LLVMUserExpression::DoExecute(DiagnosticManager &diagnostic_manager,
if (m_jit_start_addr == LLDB_INVALID_ADDRESS && !m_can_interpret) {
diagnostic_manager.PutString(
- eDiagnosticSeverityError,
+ lldb::eSeverityError,
"Expression can't be run, because there is no JIT compiled function");
return lldb::eExpressionSetupError;
}
@@ -83,7 +83,7 @@ LLVMUserExpression::DoExecute(DiagnosticManager &diagnostic_manager,
if (!PrepareToExecuteJITExpression(diagnostic_manager, exe_ctx,
struct_address)) {
diagnostic_manager.Printf(
- eDiagnosticSeverityError,
+ lldb::eSeverityError,
"errored out in %s, couldn't PrepareToExecuteJITExpression",
__FUNCTION__);
return lldb::eExpressionSetupError;
@@ -98,8 +98,7 @@ LLVMUserExpression::DoExecute(DiagnosticManager &diagnostic_manager,
if (!module || !function) {
diagnostic_manager.PutString(
- eDiagnosticSeverityError,
- "supposed to interpret, but nothing is there");
+ lldb::eSeverityError, "supposed to interpret, but nothing is there");
return lldb::eExpressionSetupError;
}
@@ -108,7 +107,7 @@ LLVMUserExpression::DoExecute(DiagnosticManager &diagnostic_manager,
std::vector<lldb::addr_t> args;
if (!AddArguments(exe_ctx, args, struct_address, diagnostic_manager)) {
- diagnostic_manager.Printf(eDiagnosticSeverityError,
+ diagnostic_manager.Printf(lldb::eSeverityError,
"errored out in %s, couldn't AddArguments",
__FUNCTION__);
return lldb::eExpressionSetupError;
@@ -122,14 +121,14 @@ LLVMUserExpression::DoExecute(DiagnosticManager &diagnostic_manager,
function_stack_top, exe_ctx, options.GetTimeout());
if (!interpreter_error.Success()) {
- diagnostic_manager.Printf(eDiagnosticSeverityError,
+ diagnostic_manager.Printf(lldb::eSeverityError,
"supposed to interpret, but failed: %s",
interpreter_error.AsCString());
return lldb::eExpressionDiscarded;
}
} else {
if (!exe_ctx.HasThreadScope()) {
- diagnostic_manager.Printf(eDiagnosticSeverityError,
+ diagnostic_manager.Printf(lldb::eSeverityError,
"%s called with no thread selected",
__FUNCTION__);
return lldb::eExpressionSetupError;
@@ -144,7 +143,7 @@ LLVMUserExpression::DoExecute(DiagnosticManager &diagnostic_manager,
std::vector<lldb::addr_t> args;
if (!AddArguments(exe_ctx, args, struct_address, diagnostic_manager)) {
- diagnostic_manager.Printf(eDiagnosticSeverityError,
+ diagnostic_manager.Printf(lldb::eSeverityError,
"errored out in %s, couldn'...
[truncated]
|
| enum Severity { | ||
| eSeverityError, | ||
| eSeverityWarning, | ||
| eSeverityInfo, |
There was a problem hiding this comment.
Can you comment that "Clang also calls these Remarks, it's conceptually the same level"
There was a problem hiding this comment.
Or even eSeverityRemark = eSeverityInfo
There was a problem hiding this comment.
I considered the latter, but we use eSeverityRemark so infrequently and not even always for DiagnosticsEngine::Level::Remark that I decided against it. The comment however seems like a good compromise.
| @@ -1347,6 +1347,12 @@ enum DebuggerBroadcastBit { | |||
| eBroadcastBitProgressCategory = (1 << 3), | |||
| }; | |||
|
|
|||
| enum Severity { | |||
There was a problem hiding this comment.
Would enum class prevent people from accidentally casting this to an unrelated enum with similar purpose but potentially different values?
There was a problem hiding this comment.
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.
adrian-prantl
approved these changes
May 3, 2024
JDevlieghere
added a commit
to swiftlang/llvm-project
that referenced
this pull request
May 3, 2024
We have 3 different enums all expressing severity (info, warning, error). Remove all uses with a new Severity enum in lldb-enumerations.h. (cherry picked from commit 528f5ba)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have 3 different enums all expressing severity (info, warning, error). Remove all uses with a new Severity enum in lldb-enumerations.h.