[PATCH] D52622: [WebAssembly] Refactor use of signatures

Sam Clegg via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 27 12:54:42 PDT 2018


sbc100 accepted this revision.
sbc100 added a comment.
This revision is now accepted and ready to land.

LGTM with nits



================
Comment at: wasm/WriterUtils.cpp:78
+  auto T = static_cast<uint8_t>(Type);
+  writeU8(OS, T, Msg + "[type: " + valueTypeToString(T) + "]");
 }
----------------
Why not instead change the param type of valueTypeToString?


================
Comment at: wasm/WriterUtils.cpp:84
+  writeUleb128(OS, Sig.Params.size(), "param Count");
+  for (auto ParamType : Sig.Params) {
     writeValueType(OS, ParamType, "param type");
----------------
I think the style guide doesn't like auto unless the type is completely obvious (normally if its written on the RHS on the same line in a cast or similar).    


================
Comment at: wasm/WriterUtils.cpp:119
 void wasm::writeGlobalType(raw_ostream &OS, const WasmGlobalType &Type) {
-  writeValueType(OS, Type.Type, "global type");
+  writeValueType(OS, ValType(Type.Type), "global type");
   writeU8(OS, Type.Mutable, "global mutable");
----------------
Probably worth updating the type of WasmGlobalType.Type .. maybe add a TODO here saying this cast should probably not be needed in the future?


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D52622





More information about the llvm-commits mailing list