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

[Wouter van Oortmerssen via Phabricator via llvm-commits]

aardappel added inline comments.


================
Comment at: include/llvm/ADT/StringSwitch.h:214
+    return std::move(Result);
+  }
+
----------------
Since this is a core LLVM class, should I get someone in particular to review? Or am I better off writing the calling code differently?


================
Comment at: include/llvm/MC/MCSymbolWasm.h:73
+
+  // TODO(wvo): not great to have this code in a header, but currently not a
+  // a lot of other places shared by Codegen and Assembler.
----------------
Suggestions welcome where to move this code. The only library/.cpp that Codegen and Assembler share is the `WebAssemblyInfo` lib, which seemed very unrelated.


================
Comment at: lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:282
+                                            Triple::wasm64);
+          // TODO: do we ideally want more generic handling like this code
+          // below instead of checkKnownSymbol?
----------------
I could remove the code below, left in for code review purposes.


Repository:
  rL LLVM

https://reviews.llvm.org/D53842