Dan Gohman [:sunfish] Assignee
	Description • 8 years ago

The WebAssembly s-expression syntax changed such that syntactic arity is no longer required to follow actual operator arities, so surprising things like this are valid now:

  (i32.const 0 (i32.const 1))
  (i32.div_s (i32.const 2))
  (i32.sub (i32.const 3))
  (i32.add)

because values are always just pushed on the stack and checked when popped when needed by an operator.

Attached is a patch which implements this. It roughly works, though it currently exposes some test failures due to unrelated issues.

	Hannes Verschore [:h4writer]
	Comment 1 • 8 years ago

Is there an update on this bug? It hasn't received attention in the last 19 days?
I'm asking since P1 means this should get into the current release. There are only 14 days left before the merge.

Is this still a P1? If not should we demote to P2 (next release) or P3 (any release)?
If this is still P1 it is starting to become urgent to see some progress.

	Benjamin Bouvier [:bbouvier] (inactive)
	Comment 2 • 8 years ago

If that's okay, I'll complete this patch and make sure we pass other tests. Leaving it p1.

	Benjamin Bouvier [:bbouvier] (inactive)
	Comment 3 • 8 years ago

Rebased and updated to fix issues:
- initializers expressions want their parenthesis
- a test in typecheck.wast (removed since then) is not compatible with arity insensitive parsing

Selecting luke as a reviewer, to avoid the conflict of interest author/reviewer.

	Luke Wagner [:luke]
	Comment 4 • 8 years ago

Comment on attachment 8804240 [details] [diff] [review]
arity-insensitive.patch

Review of attachment 8804240 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

::: js/src/jit-test/tests/wasm/spec/typecheck.wast
@@ +159,5 @@
>      (i32.eqz) (drop)
>    ))
>    "type mismatch"
>  )
> +;; Test incompatible with arity insensitive parsing.

If I read trunk spec/typecheck.wast correctly, this test was simply removed.  Is your intention to just keep this here temporarily and update typecheck.wast as a whole or can you just delete this now?

	Benjamin Bouvier [:bbouvier] (inactive)
	Comment 5 • 8 years ago

(In reply to Luke Wagner [:luke] from comment #4)
> If I read trunk spec/typecheck.wast correctly, this test was simply removed.
> Is your intention to just keep this here temporarily and update
> typecheck.wast as a whole or can you just delete this now?

That's right, it is temporary. Thanks for the quick review!

	Pulsebot
	Comment 6 • 8 years ago

Pushed by bbouvier@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9ba1705b5af6
BaldrMonkey: support arity-insensitive syntax; r=luke

	Carsten Book [:Tomcat]
	Comment 7 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/9ba1705b5af6