[PATCH] D27355: [WebAssembly] Add wasm support for llvm-readobj
Davide Italiano via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 30 14:17:27 PST 2017
davide added inline comments.
Comment at: tools/llvm-readobj/WasmDumper.cpp:85-88
+ const WasmObjectFile *WasmObj = dyn_cast<WasmObjectFile>(Obj);
+ if (!WasmObj)
+ return readobj_error::unsupported_obj_file_format;
> davide wrote:
> > Can this always happen? You enter here under `Obj->isWasm()`.
> > If there's a legitimate where this can happen, I would add a test (if you don't mind).
> > Also, not entirely sure if `unsupported_obj_file_format` is the best diagnostic we can emit. Maybe `broken_file`? (I suspect this is consistent with what other dumpers are doing, if so, leave it as is but please add a test to exercise this code).
> This is exactly what the MachO and COFF dumpers do yes, and basically the same as the ELF dumper.
> You are correct that in practice there is no way this can happen, at least not by running the tool itself. In light of this I don't see how I can write a test to tickle this, and I think perhaps an assert would be more appropriate here?
Yes, that's what I would do.
Probably better not cargo culting dubious code from other drivers.
More information about the llvm-commits