[PATCH] D149610: [llvm-objdump][COFF] Skip empty entries when dumping the export table

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/Object/COFFObjectFile.cpp:1568-1570
+  Result = (bool)E;
+  consumeError(std::move(E));
+  return Error::success();
----------------
Drive-by comments as I noticed these when my herald rule triggered:

I believe static_cast is generally preferred to C-style casts.

Why `consumeError` rather than not return `E`? I guess it's because the interesting bit is whether or not `getRvaPtr` fails, but is there a risk it will fail for some other reason (I haven't looked at the underlying code at all, so there might not be)? Also, would it be better for this method to return `Expected<bool>` rather than pass in an input parameter for the result?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149610