[PATCH] D55797: [WebAssembly] Make assembler check for proper nesting of control flow.

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added a comment.

- It looks like even if an error occurs, the parsing continues. Is this an expected behavior?

- If it is, is a mismatched instruction supposed to pop the stack? For example,

  try
  loop
  block
  end_loop    // stray incorrect instruction
  end_block
  end_loop
  end_try

In this case, there is a stray `end_loop` instruction in the middle of an otherwise correct sequence of instructions. When we parse `end_loop`, we pop `block` and compare that with `end_loop` and emit an error, which is fine. But next we parse `end_block`, but `block` was incorrectly popped when we parse `loop`, it is going to fail. The next instruction as well, and this ends up failing all the instructions. But if we don't pop the stack when an incorrect match occurs, there is gonna be only one error, other than four. I'm not sure what the right behavior is, so I'm asking.



================
Comment at: test/MC/WebAssembly/basic-assembly-errors.s:1
+# RUN: (llvm-mc -triple=wasm32-unknown-unknown -mattr=+simd128,+nontrapping-fptoint,+exception-handling < %s 2>&1 || true) | FileCheck %s
+
----------------
This is ok as is, but it looks like usually people append `not` in front of the command (in which way you don't need `|| true`)


================
Comment at: test/MC/WebAssembly/basic-assembly-errors.s:13
+    try         # missing end: unmatched
+    if          # missing end: mismatched
+    end_function
----------------
By the way, can we parse `if`? You commented these out in the other test case saying they are not supported in tablegen.


================
Comment at: test/MC/WebAssembly/basic-assembly-errors.s:16
+.Lfunc_end0:
+	.size       test0, .Lfunc_end0-test0
+
----------------
Looks like this line is indented with a tab, while others with spaces. 


================
Comment at: test/MC/WebAssembly/basic-assembly-errors.s:22
+# CHECK: Block construct type mismatch, expected: end_if, instead got: end_function
+# CHECK: error: Unmatched block construct(s) at function end: try
----------------
How about interleaving these lines within the test case, so we can easily see which lines match which instruction? In that way we may not need inline comments for those instructions as well


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55797