[PATCH] D101835: [llvm] llvm-tapi-diff
Cyndy Ishida via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri May 7 16:36:02 PDT 2021
cishida added inline comments.
================
Comment at: llvm/tools/llvm-tapi-diff/llvm-tapi-diff.cpp:1
+//===-- llvm-tapi-diff.cpp - symbol table comparator command-line driver ---*-
+// C++
----------------
we should also replace `symbol table comparator` with something like `(tapi|tbd) file comparator`
================
Comment at: llvm/tools/llvm-tapi-diff/llvm-tapi-diff.cpp:11
+//
+// This file defines the command-line driver for the llvm-nm difference engine.
+//
----------------
s/llvm-nm/llvm-tapi-diff
================
Comment at: llvm/tools/llvm-tapi-diff/llvm-tapi-diff.cpp:79
+ auto BinRHS = convertFileToBinary(InputFileNameRHS);
+ if (!BinLHS || !BinRHS)
+ return EXIT_FAILURE;
----------------
Please add a testcase that goes through an error path.
When I ran the tool with a misspelled input it crashes.
If I understood @JDevlieghere comment correctly, we should be handling the error at this call site instead of inside `convertFileToBinary` and currently we don't capture the possible error from the wrapped `Expected<T>` object.
```bin/llvm-tapi-diff: error: src/llvm-project/llvm/test/tools/llvm-tapi-diff/Inputs/vC.tbd: No such file or directory
Expected<T> must be checked before access or destruction.
Unchecked Expected<T> contained error:
No such file or directoryPLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0. Program arguments: bin/llvm-tapi-diff src/llvm-project/llvm/test/tools/llvm-tapi-diff/Inputs/v4B.tbd src/llvm-project/llvm/test/tools/llvm-tapi-diff/Inputs/vC.tbd
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
0 llvm-tapi-diff 0x00000001084d4dd7 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 39
1 llvm-tapi-diff 0x00000001084d3ff8 llvm::sys::RunSignalHandlers() + 248
<omitted>
[1] 85533 abort bin/llvm-tapi-diff
```
See https://llvm.org/docs/ProgrammersManual.html#error-handling for more details.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101835/new/
https://reviews.llvm.org/D101835
More information about the llvm-commits
mailing list