[Lldb-commits] [PATCH] D128480: [lldb] Replace Host::SystemLog with Debugger::Report{Error, Warning}

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.


================
Comment at: lldb/source/Core/Module.cpp:1187
+
+    Debugger::ReportError(std::string(strm.GetString()));
+  }
----------------
mib wrote:
> mib wrote:
> > It's unrelated to this patch, but it looks like `Debugger::HandleDiagnosticEvent` dumps everything to the error stream without checking if the event is a warning or an error. I'm mentioning that here because if we did that distinction at the event handling level, we could get rid of `strm.PutCString("error: ")`.
> > 
> > Note however, the reason I'm bringing this up, is that the error message prefix is not consistent with `ReportErrorIfModifyDetected`, since it's not prepended by `error: `.
> > 
> > We should at least fix that for this patch before before making it consistent at the event handler level.
> I hadn't looked at the rest of the patch and this looks very inconsistent throughout the other files as well ... may be we should just leave that for a follow-up patch
Yeah this is an oversight. The prefix will get printed when the event gets handled (or by the IDE) so it shouldn't be part of the message. I'll fix that here. 


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

https://reviews.llvm.org/D128480