[PATCH] D148032: [symbolizer] Change error message if module not found

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 12 18:31:06 PDT 2023


MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for making the change. I was confused by the error messages as well.

I only got a question about a variable name and its class name. Perhaps other reviewers can make suggestions.



================
Comment at: llvm/include/llvm/DebugInfo/Symbolize/DIPrinter.h:73
   raw_ostream &OS;
-  raw_ostream &ES;
+  MessagePrinter MsgPrinter;
   PrinterConfig Config;
----------------
This is used to print error messages to `errs()`, not other types of messages. Shall we name this `ErrorPrinter`?


================
Comment at: llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp:100
+        PrintEmpty =
+            Printer.printError(Request, EI, "llvm-symbolizer: error: ");
+      });
----------------
jhenderson wrote:
> Other tools use the full tool path, as per the example I gave earlier. You can how this is done here: https://github.com/llvm/llvm-project/blob/43575719d0c6d8cf5afedf39f6d89f69231aedc4/llvm/tools/llvm-nm/llvm-nm.cpp#L155
This comment about the full tool path has been resolved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148032



More information about the llvm-commits mailing list