Report warnings and errors through events instead of printing directly the to the debugger's error stream. Diagnostic events are handled by the default event loop. This allows IDEs such as Xcode to report these issues in the UI instead of having them show up in the debugger console.
Details
Diff Detail
Event Timeline
It seems to me that it would be better to decentralize the warning tracking. Also due to a central list of all possible warnings not being modular, but mainly because I can imagine that some warnings may want to be reported with different "scopes" -- once per target, thread, module (this is the case of the "dwp missing" warning) or whatever. That won't work if the list is managed centrally in the debugger object. We could define some token warn_once type (maybe just a typedef to once_flag or atomic_flag) that would have to be passed to the warning emitting function. The function would handle the actual "once" logic, and the scope of the token variable would determine the frequency of the warning.
My second remark is that this does not help with the warnings that are not tied to a specific debugger instance -- such as those coming from the module parsing code. I don't know if you have a need to display those (I know I don't), but it may be nice to set things up so that we're able to transmit those as well -- I believe we are able to do that for progress events.
lldb/include/lldb/Core/DebuggerEvents.h | ||
---|---|---|
91 |
Address code review:
- Decentralize tracking whether a warning/error has been emitted.
- Support diagnostics that are not tied to a single debugger.
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?
lldb/include/lldb/Core/DebuggerEvents.h | ||
---|---|---|
73 | 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. | |
lldb/source/Core/Debugger.cpp | ||
1665–1671 | StartListeningForEvents(eBroadcastBitProgress | eBroadcastBitWarning | eBroadcastBitError) ? | |
1866 | 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. |
Print diagnostics directly to the debugger's error stream if nobody is listening for them.
cool
lldb/unittests/Core/DiagnosticEventTest.cpp | ||
---|---|---|
53–54 ↗ | (On Diff #415699) | I guess you could move this into the SetUp function as well. |
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.