[PATCH] D44562: [ELF] Rework debug line parsing to use llvm::Error and callbacks (LLD-side)

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: Common/ErrorHandler.cpp:86-91
+void ErrorHandler::remark(const Twine &Msg) {
+  std::lock_guard<std::mutex> Lock(Mu);
+  newline(ErrorOS, Msg);
+  print("remark: ", raw_ostream::BLUE);
+  *ErrorOS << Msg << "\n";
+}
----------------
ruiu wrote:
> Honestly I don't think we want to add a new level of error messages, as I think error() for errors, warning() for warnings and message() for non-error messages is enough. If something needs to be fixed, it should be an error or a warning. If something doesn't have to be fixed, lld shouldn't print out anything. A situation like "verbose messages are printed out but you can ignore them" isn't actionable and thus not desirable.
> 
> So, can you choose either (1) show it as a warning or (2) don't show anything?
Okay, I have no problem making it a warning. I think this might be a safer course than not printing anything.  If undefined symbol messages are incomplete, due to a problem in the input, it might be possible for users to fix it, depending on the nature of the problem (e.g. bogus hand-written debug_line assembly), and therefore allowing them to see the useful error information, allowing them to more quickly identify the problem. I could also see us potentially using this function in the future to produce other informational messages, or implement other features, without always emitting an error, so a warning would be appropriate here too.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D44562