[spec+interpreter] Accepts "i32.const0" (missing space)

[sunfishcode]

The interpreter currently accepts the following:

(module (func (export "foo") (result i32) (i32.const0)))

(assert_return (invoke "foo") (i32.const0))

Note that there's no space between the i32.const and the 0. This doesn't create any actual ambiguity, but it doesn't seem to add meaningful flexibility either, and it's surprising.

For reference, wabt and binaryen both issue errors on this code.

[rossberg]

Hm, interesting. That is a normal result of the usual longest match rule for lexing. But it seems that for an S-expression format that is not actually a desirable logic to use.

[binji]

I don't remember doing it, but it looks like I added a rule a while back that prevented this in the wabt lexer: https://github.com/WebAssembly/wabt/blob/master/src/wast-lexer.cc#L464

Removing that rule makes wabt work the same, allowing you to parse modules like: (module(func(resulti32)i32.const0)) :-)

[rossberg]

@binji, yes, I just implemented the same rule. It's a bit more interesting how to specify it, though. Probably gonna do a similar trick in the spec itself, defining a general syntax for atoms, wth all other ones being "reserved".

[rossberg]

Okay, I refined PR #471 to rule out such cases and adjust the interpreter accordingly.

[AndrewScheidecker]

@rossberg-chromium I don't think your change rejects a keyword following a number:
offset=1align=1

[rossberg]

@AndrewScheidecker, works for me. Note that the sequence offset=1align=1 forms a single "keyword" under the given grammar. There is no number.

[rossberg]

Ah, but you are right nevertheless! (module (func br 0nop)) is still accepted. Hm, darn, not sure how to solve this.

[AndrewScheidecker]

I'm thinking about fixing this in WAVM by only matching a keyword or number if it's followed by a token separating character (without consuming that character).

[rossberg]

Actually, it seems easy to fix by just removing the bit that a bogus "keyword" starts with a letter. See latest patch, which also adjusts the spec text.

[rossberg]

This has been resolved as part of PR #471.