[Lldb-commits] [PATCH] D121511: [lldb] Report debugger diagnostics as events

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Heh, I didn't realize that "debugger-less" events are implemented by iterating over all debugger instances, but since we're doing that for progress events anyway, we might as well copy the pattern.

Any chance of a test case for some of this stuff? Maybe a unittest which creates a Debugger object and uses it to send/receive some events?



================
Comment at: lldb/include/lldb/Core/DebuggerEvents.h:73
+
+class WarningEventData : public DiagnosticEventData {
+public:
----------------
Do we really need separate types for this? The handling of the two events is completely identical (except for the "error"/"warning" prefix, so it seems that an enum would suffice. The classes won't make it to the SB api (I think), so it's not like we're locking ourselves into a particular design.


================
Comment at: lldb/source/Core/Debugger.cpp:1665-1671
   listener_sp->StartListeningForEvents(&m_broadcaster,
                                        Debugger::eBroadcastBitProgress);
+  listener_sp->StartListeningForEvents(&m_broadcaster,
+                                       Debugger::eBroadcastBitWarning);
+  listener_sp->StartListeningForEvents(&m_broadcaster,
+                                       Debugger::eBroadcastBitError);
 
----------------
StartListeningForEvents(eBroadcastBitProgress | eBroadcastBitWarning | eBroadcastBitError) ?


================
Comment at: lldb/source/Core/Debugger.cpp:1866
+
+  Stream &stream = *GetAsyncErrorStream();
+  stream << "warning: " << data->GetMessage() << '\n';
----------------
GetAsyncErrorStream returns a (shared -- it should probably be a unique_ptr instead) pointer to a new object. You can't just throw it away here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121511/new/

https://reviews.llvm.org/D121511