Return nop arity

[Cellule]

For the following function
(func $return-nop (return (nop))) (https://github.com/WebAssembly/testsuite/blob/master/functions.wast)

There is a conflict between the function type and return arity.
The type of the function is

; type 0
000000f: 40                                         ; function form
0000010: 00                                         ; num params
0000011: 00                                         ; num results

And the function body is the following

; function body 6
00001a0: 00                                         ; func body size (guess)
00001a1: 00                                         ; local decl count
00001a2: 00                                         ; OPCODE_NOP
00001a3: 09                                         ; OPCODE_RETURN
00001a4: 01                                         ; return arity
00001a0: 04                                         ; FIXUP func body size

Since nop is: an empty operator that does not yield a value
then the arity of the return statement should be 0 thus having a valid type.

I found this by running the tests, I am surprised this didn't come up in the CI

[binji]

Are you suggesting that the arity is 0 but still include the nop? That seems strangely inconsistent; I thought the arity represented the number of expected expressions, not the number of returned values. (the benefit being for naive consumers, IIRC -- they can read the code section without having to know function signatures, or track block types, etc.)

If you mean arity 0 without the nop, that seems strange to me too -- AIUI the wast format is meant to be a representation of the AST format of the file, so it doesn't make sense that you'd have to remove the nop to convert it to binary.

[Cellule]

Well right now, in the baseline for this module there's

WasmModule::Instantiate(): Compiling WASM function #6:<?> failed:Result = arity mismatch in return @+2
  var module = Wasm.instantiateModule(u8a, ffi);
                    ^

Which is wrong because this is a spec test, meaning the sexpr is valid, but d8 thinks it's wrong.
(I think check in a failure as a baseline is a mistake to start with since it won't match with other hosts).

From what I see, the function signature returns void, therefore, the return opcode should have arity 0.

After further investigation, I realized that this is not a problem with nop in particular, but with the fact that sexpr-wasm determines the arity of the return opcode if there's an expression next to it.
This is also a problem with the case (func $return-drop (return (i32.const 1)))
which is also valid. This is a function that returns void thus the i32 const should be dropped (still emitted by sexpr-wasm since it could have side-effects).

By reading the spec for the return opcode, looking at the v8's and chakra's implementation of the return opcode. The arity means, how many values to return. If the return type is void, the arity should be 0 regardless of what the sexpr "looks" like.

[binji]

I'm not sure that's right, see the PR that added explicit arities here: WebAssembly/design#595

The arity is simply the number of operands that syntactically follow the operator, and have to be consumed by the decoder. The point of introducing these in the binary format is to decouple decoding from typing and validation, and make the binary grammar "context-free".

[Cellule]

Well from what you've quoted it sounds like we need the arity and the functions type to match so that tools don't need to know about the function type.
Either the spec is wrong or sexpr-wasm is wrong or v8 implementation is wrong.

I feel that it is valid to give a validation error if the arity doesn't match the type like v8
Then either this is an invalid sexpr function (func $return-drop (return (i32.const 1))) in which case I feel sexpr-wasm should return an error and we should update the spec suite.
Or what sexpr-wasm is not producing the right binary.

@rossberg-chromium What do you think ?

[Cellule]

This is no longer needed as we are removing arities from return op. WebAssembly/design#741