[PATCH] D54652: [WebAssembly] replaced .param/.result by .functype

[Wouter van Oortmerssen via Phabricator via llvm-commits]

aardappel marked 3 inline comments as done.
aardappel added inline comments.


================
Comment at: lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:189
+  Optional<wasm::ValType> ParseType(const StringRef &Type) {
+    // FIXME: can't use StringSwitch because wasm::ValType doesn't have a
+    // "invalid" value.
----------------
dschuff wrote:
> at least the `operator ==` overload on `StringRef` makes this code not as bad as it could be?
Yup, it's ok :)


================
Comment at: lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:377
+                    TOut.getStreamer().getContext().getOrCreateSymbol(SymName));
+      WasmSym->setType(wasm::WASM_SYMBOL_TYPE_GLOBAL);
+      auto Signature = make_unique<wasm::WasmSignature>();
----------------
dschuff wrote:
> why set and then reset the symbol type? all of the intervening returns are error conditions, right?
That looks like a refactoring slip-up, will remove.


Repository:
  rL LLVM

https://reviews.llvm.org/D54652