[PATCH] D55149: [WebAssembly] Enforce assembler emits to streamer in order.

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added a comment.

Style nits



================
Comment at: lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:144
+  // regular backend (the streamer does not enforce this).
+  // This "state machine" enum helps guarantee that correct order.
+  enum ParserState {
----------------
Nit: Could you wrap comments to 80 cols? (Currently some lines end prematurely)


================
Comment at: lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:150
+      FunctionLocals,
+      Instructions,
+  } CurrentState;
----------------
clang-format this?


================
Comment at: lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:161
       : MCTargetAsmParser(Options, STI, MII), Parser(Parser),
-        Lexer(Parser.getLexer()) {
+        Lexer(Parser.getLexer()), CurrentState(FileStart), LastLabel(nullptr) {
     setAvailableFeatures(ComputeAvailableFeatures(STI.getFeatureBits()));
----------------
Nit: Can we use C++11-style in-class initialization for these two variables?


================
Comment at: lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:400
+      // assembler and backend really don't share any common code, and
+      // this code parses the locals seperately.
       auto SymName = ExpectIdent();
----------------
Nit: Comment 80-col wrapping too, and maybe other parts as well


================
Comment at: lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:450
+        auto &TOut = reinterpret_cast<WebAssemblyTargetStreamer &>(
+                *Out.getTargetStreamer());
+        TOut.emitLocal(SmallVector<wasm::ValType, 0>());
----------------
Is this clang-formatted?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55149/new/

https://reviews.llvm.org/D55149