[PATCH] D56223: [WebAssembly] Made InstPrinter more robust

Wouter van Oortmerssen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 3 14:12:26 PST 2019


aardappel marked an inline comment as done.
aardappel added inline comments.


================
Comment at: lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp:304
+  default:
+    return "invalid_type";
   }
----------------
aheejin wrote:
> aardappel wrote:
> > aheejin wrote:
> > > So does this print "invalid_type" even when we have an invalid instruction within itself?
> > I'm not sure what you mean. It currently prints this only when an instruction explicitly tries to print an operand that is a block_type or other signature, and it happens to be an unknown value.
> Oh what I meant was.. I was not sure if printing instructions like
> ```
> block invalid_type
> ```
> was ok. 
> ```
> br 32 # invalid depth argument!
> ```
> seemed ok because even if it's out of context, `br 32` itself is a valid instruction, and we are giving hints that this instruction does not make sense within the current context in the comment anyway. But `block invalid_type` is not an instruction and cannot be parsed. So I think in this case it deserves an assertion, but I don't feel very strongly about it. What do you think?
Well, there's no way to print that `block` instruction with a valid type. I could print `block void # Invalid type!` instead but that would be highly confusing. I don't think there should be an assert here, no, as per my original motivation for this commit.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56223





More information about the llvm-commits mailing list