[PATCH] D66517: [llvm-objdump] - Remove one overload of reportError.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a subscriber: enderby.
MaskRay added inline comments.


================
Comment at: tools/llvm-objdump/MachODump.cpp:2409
                          ObjOrErr.takeError())) {
-            reportError(std::move(E), Filename, StringRef(), ArchitectureName);
+            reportError(std::move(E), "", Filename, ArchitectureName);
             continue;
----------------
grimar wrote:
> jhenderson wrote:
> > Should this be:
> > 
> > `reportError(std::move(E), Filename, "", ArchitectureName);`
> I am not familar with MachO, but I think - no. Seems MachOUniversalBinary can contain
> multiple objects and also `Filename` is reported as archive name around this place.
> 
> Note that my change did not change the original logic, and also that this error is untested.
I think George's change here is fine. At least it doesn't introduce new problems.

Maybe @seiya can help check whether there is something that should be improved? (@enderby seems to be inactive for a while)


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

https://reviews.llvm.org/D66517