[PATCH] D26172: [WebAssembly] Add llvm-objdump support for wasm file format

Sam Clegg via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 21 17:25:40 PST 2016


sbc100 added inline comments.


================
Comment at: include/llvm/Object/Wasm.h:31
+  const wasm::WasmSection *getWasmSection(const SectionRef &Section) const;
+  static inline bool classof(const Binary *v) { return v->isWasm(); }
+
----------------
davide wrote:
> I think `inline` inside class definition is not necessary (check elsewhere).
All the other headers in include/llvm/Object do it this way


================
Comment at: lib/Object/WasmObjectFile.cpp:71
+  if (Header.Magic != StringRef("\0asm", 4)) {
+    Err = make_error<GenericBinaryError>("Bad magic number",
+                                         object_error::parse_failed);
----------------
davide wrote:
> Can't this just be a `StringError` instead of a `GenericBinaryError`? 
GenericBinaryError seems to make sense since this is an problem with binary file.  Its a subclass of BinaryError which I think is that kind of error we want here.

From the docs from GenericBinaryError:

/// For errors that don't require their own specific sub-error (most errors)
/// this class can be used to describe the error via a string message.

MachOObjectFile.cpp uses it too.


https://reviews.llvm.org/D26172





More information about the llvm-commits mailing list