[PATCH] D54138: [WebAssembly] Read prefixed opcodes as ULEB128s

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added inline comments.


================
Comment at: lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp:135
+    if (Error || Opc < 0 || Opc >= WebAssemblyInstructionTableSize)
       return MCDisassembler::Fail;
+    Size += N;
----------------
I'm not too familiar with this disassembler part of the code, but the types with which `Opc` seem a bit confusing. So `Opc` is first declared above in 
```
auto Opc = nextByte(Bytes, Size);
```
It is `auto`, but I guess because `nextByte` reutrns int, that's gonna be an int type.

And `nextByte` takes `uint8_t` array but returns an `int`. Now the same variable `Opc` is used to store the return type of `decodeULEB128` whose return type is `uint64_t`. I know I'm talking about both something pre-existing and things added in this CL, but do you think it would make more sense to change the return type of `nextByte` to `uint8_t` too and split the two `Opc`s, and maybe not make them `auto`? 


Repository:
  rL LLVM

https://reviews.llvm.org/D54138