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

[Davide Italiano via llvm-commits]

davide accepted this revision.
davide added a reviewer: davide.
davide added a comment.
This revision is now accepted and ready to land.

This LGTM. Michael might have some extra comment.



================
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(); }
+
----------------
sbc100 wrote:
> davide wrote:
> > sbc100 wrote:
> > > 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
> > This doesn't necessarily mean they're right.
> Hmm, seems like the codebase is quite divided on this one:
> 
> $ git grep classof ./include | grep inline | wc -l
> 230
> $ git grep classof ./include | grep -v inline | wc -l
> 257
> 
> Perhaps I should propose a cleanup  CL? 
> 
> In the mean time I'll drop inline here I guess?
That's a great idea if you feel like it =)
I'm not sure if it's possible in the general case but it would be nice to think if it can be automated using a `clang-tidy` check to be run on the whole codebase (instead of you changing every definition by hand)


https://reviews.llvm.org/D26172