[PATCH] D147652: [symbolizer] Check existence of input file in GNU mode

Serge Pavlov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 20 11:33:20 PDT 2023


sepavloff marked an inline comment as done.
sepavloff added inline comments.


================
Comment at: llvm/lib/DebugInfo/Symbolize/Symbolize.cpp:718
+/// \returns Error::success() if the file was found, error object otherwise.
+Error LLVMSymbolizer::findBinaryFile(StringRef BinName) {
+  auto [BinaryName, ArchName] = splitBinaryFileName(BinName);
----------------
jhenderson wrote:
> Perhaps `checkFileExists` or `checkIsFile` would be a better name? If you use one of those names, you can delete the comment entirely, as it doesn't do anything useful then.
Indeed, thank you!


================
Comment at: llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp:486
+      handleAllErrors(std::move(Status), [&](const ErrorInfoBase &EI) {
+        printError(EI, InputFile);
+      });
----------------
jhenderson wrote:
> It doesn't need to be part of this change, but I don't think it would be unreasonable to add a `printError` overload that takes `Error` instead of `ErrorInfoBase` and does the `handleAllErrors` internally. It looks like there are at least 2 separate places now that would benefit from that.
Yes, it makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147652



More information about the llvm-commits mailing list