[PATCH] D53842: [WebAssembly] Parsing missing directives to produce valid .o

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
Comment at: include/llvm/BinaryFormat/Wasm.h:245
   WASM_SYMBOL_TYPE_SECTION = 0x3,
+  WASM_SYMBOL_TYPE_INVALID = 0x4,
 };
----------------
I'd rather avoid adding this if possible.  We don't want to allow symbols to have an invalid type, or have to add this invalid case to our switch statements over symbol type.


================
Comment at: lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:351
       // "label, at function"
-      if (!(IsNext(AsmToken::Identifier) && IsNext(AsmToken::Comma) &&
-            IsNext(AsmToken::At) && Lexer.is(AsmToken::Identifier)))
+      if (!Lexer.is(AsmToken::Identifier))
+        return Error("Expected label after .type directive, got: ",
----------------
dschuff wrote:
> It just occurred to me: do we need to make this stateful like this? Should a `.type` directive create a symbol/type association regardless of where it is with respect to the actual label? It looks like the ELF Asm parser (`ParseDirectiveType`) just looks up the symbol (creating if necessary) and emits a symbol attribute to the streamer (which is what actually manipulates the symbol). Maybe we could use this pattern in wasm too, to make asm vs object more uniform?
This sounds good to me.  Than we could avoid LastTypeOfSymbol and GlobalTypeFixups completely?


Repository:
  rL LLVM

https://reviews.llvm.org/D53842