binaryen icon indicating copy to clipboard operation
binaryen copied to clipboard

Missing error when `struct.new` has too many arguments in assembler

Open hgruniaux opened this issue 1 year ago • 2 comments

When assembling the following invalid WAT code:

(module
    (type $t (struct))
    (func (param i32)
        (struct.new $t (local.get 0))))

the assembler rejects it with the following error message:

unexpected false: non-final block elements returning a value must be drop()ed
unexpected true: if block is not returning a value, final element should not flow out a value

I think, it will be better that it generates an error about the fact that struct.new has too many arguments.

hgruniaux avatar Jul 12 '24 12:07 hgruniaux

Unfortunately the wat text format considers code like this equal:

(struct.new $t (local.get 0))

(local.get 0)
(struct.new $t)

So the parser has to accept it.

Though if an error happens later, as in this testcase, then we could perhaps look back at the wat to see if we can print a better error message. But connecting the validator to the wat text that was already fully processed at that point might not be easy. @tlively what do you think?

kripken avatar Jul 12 '24 20:07 kripken

In particular, when the parser parses the struct.new $t, as far as it knows, that local.get 0 could be consumed by a later instruction. Even if we did more error checking in the parser, it wouldn't be clear that an error occurred until the end of the block, but then it would be unclear which instruction was intended to consume it, so we still couldn't print a useful error message. In principle, we could use the parentheses to inform heuristics to improve the error message, but that would break a very important abstraction boundary in the implementation and make the code too complicated.

tlively avatar Jul 14 '24 00:07 tlively