[PATCH] D27355: [WebAssembly] Add wasm support for llvm-readobj
Davide Italiano via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 24 07:41:07 PST 2017
davide added inline comments.
================
Comment at: test/tools/llvm-readobj/file-headers.test:29-31
+RUN: llvm-readobj -h %p/Inputs/trivial.obj.wasm \
+RUN: | FileCheck %s -check-prefix WASM
----------------
Please add a comment exaplining how you created the source file. That is:
-> compiler/linker/any tool used invocation & source code. Thanks!
This is useful in case we need to regenerate the input object file.
================
Comment at: tools/llvm-readobj/WasmDumper.cpp:1
+//===-- WasmDumper.cpp - Object file dumping utility for llvm -------------===//
+//
----------------
You may want to mention `WASM Object file dumping utility` ?
================
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).
https://reviews.llvm.org/D27355
More information about the llvm-commits
mailing list