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

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.


================
Comment at: lldb/source/Core/Module.cpp:1187
+
+    Debugger::ReportError(std::string(strm.GetString()));
+  }
----------------
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


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

https://reviews.llvm.org/D128480