[PATCH] D27355: [WebAssembly] Add wasm support for llvm-readobj
Sam Clegg via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 30 14:15:33 PST 2017
sbc100 marked an inline comment as done.
sbc100 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;
> 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?
More information about the llvm-commits