This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objdump] - Remove one of `report_error` functions and improve the error reporting.
ClosedPublic

Authored by grimar on Aug 20 2019, 4:28 AM.

Details

Summary

One of the report_error functions was taking object::Archive::Child as an
argument. It feels excessive, this patch removes it and introduce a helper
function instead. Also I fixed a "TODO" in this patch what improved the message printed.

Diff Detail

Event Timeline

grimar created this revision.Aug 20 2019, 4:28 AM
jhenderson added inline comments.Aug 20 2019, 4:33 AM
tools/llvm-objdump/MachODump.cpp
2216–2217

Ndx -> ChildIndex

tools/llvm-objdump/llvm-objdump.cpp
372

Comment to explain why we're consuming the error?

443–444

Did you mean to delete this function?

grimar updated this revision to Diff 216100.Aug 20 2019, 4:47 AM
grimar marked 3 inline comments as done.
  • Addressed review comments.
tools/llvm-objdump/llvm-objdump.cpp
443–444

Yes..

grimar planned changes to this revision.Aug 20 2019, 5:08 AM
grimar updated this revision to Diff 216105.Aug 20 2019, 5:12 AM
  • Ndx->ChildIndex in 2 more missed places.
MaskRay added inline comments.Aug 20 2019, 5:16 AM
tools/llvm-objdump/MachODump.cpp
2486

Missing ++I ?

jhenderson added inline comments.Aug 20 2019, 5:26 AM
tools/llvm-objdump/llvm-objdump.cpp
372

a error -> an error

print the index of which archive member this is -> print the index of the archive member

373

we already -> we are already

grimar updated this revision to Diff 216112.Aug 20 2019, 5:47 AM
grimar marked 3 inline comments as done.
  • Addressed review comments.
tools/llvm-objdump/MachODump.cpp
2486

Yes :/ thanks.

jhenderson accepted this revision.Aug 20 2019, 6:03 AM

LGTM, with one nit.

tools/llvm-objdump/llvm-objdump.cpp
373

a error -> an error

This revision is now accepted and ready to land.Aug 20 2019, 6:03 AM
This revision was automatically updated to reflect the committed changes.
grimar marked an inline comment as done.
Herald added a project: Restricted Project. · View Herald TranscriptAug 20 2019, 6:18 AM