bytecodealliance/wat

Duplicate identifiers should cause errors

eqrion opened this issue · 2 comments

The following module parses and encodes.

(module (func $foo) (func $foo) (export "foo" (func $foo)))

The spec says that identifier spaces must be well-formed [1], which means no duplicates. If I understand the code correctly, we should be failing somewhere in resolve [2].

[1] https://webassembly.github.io/spec/core/text/conventions.html#contexts
[2]

self.names.insert(name, self.count);

Oh interesting! I remember being curious about this when I first implemented this but discovered that the test suite has a few cases where duplicate identifiers are expected to be valid:

I was otherwise pretty surprised that there wasn't a test that two same-named functions failed, so I sort of assumed that there was a test asserting that they were correct!

I read the spec the same way you're pointing out, though, in that duplicate names should definitely be errors. I'll file an issue with the upstream spec to see what's up with those two test cases.

Oh interesting! I remember being curious about this when I first implemented this but discovered that the test suite has a few cases where duplicate identifiers are expected to be valid:

Huh, that's interesting! Hopefully it's a bug, I'm not sure what use duplicate identifiers are.

This came up while migrating away from the existing SpiderMonkey wat2wasm code on a couple tests for this behavior. Although, our implementation is very non-standard so there's not much weight there for expected behavior.