Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moving WebAssembly.Tag.type to type reflection? #172

Open
thibaudmichaud opened this issue Jul 13, 2021 · 6 comments · Fixed by #174
Open

Moving WebAssembly.Tag.type to type reflection? #172

thibaudmichaud opened this issue Jul 13, 2021 · 6 comments · Fixed by #174

Comments

@thibaudmichaud
Copy link
Collaborator

Are there known use cases that require WebAssembly.Tag.type to be part of the exception handling proposal? If not I would be in favor of moving it to the type reflection proposal.

The argument is that exception handling has more support and is advancing faster, so we should not make it depend on a less mature proposal. In particular if changes are made to the type reflection API later, it might be too late to reflect these changes in WebAssembly.Tag.type without breaking backwards compatibility, and would introduce inconcistencies.
The two proposals are technically both at phase 2, but AFAIK type reflection is much less active, and exception handling is close to reaching phase 3 and 4.

cc @gahaas who pointed this out to me.

@Ms2ger
Copy link
Contributor

Ms2ger commented Jul 13, 2021

WebAssembly.Tag.type() should only be implemented if the engine implements both exception handling and type reflection. I don't really care what document it lives in; I think type reflection should be ready for phase 3 already and probably phase 4 soon as well.

@thibaudmichaud
Copy link
Collaborator Author

I agree that this should only be enabled if both are supported. I am not sure how proposal intersections are usually handled, but I read the current explainer as though this was an unconditional part of exception handling. If we just agree informally here that this is not the case, that's fine with me and I think we can close this.

@ajklein
Copy link

ajklein commented Oct 21, 2022

Unfortunately it looks like the inclusion of this method in this spec, and the WPT tests for it, has caused confusion among implementers. According to wpt.fyi, both Firefox and Safari are currently shipping the type accessor. Not yet sure what I think the right resolution is but something to keep in mind as we think about how to handle the type reflection proposal.

@eqrion @justinmichaud @codehag @SPY

@ajklein ajklein reopened this Oct 21, 2022
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 21, 2022
For most tests, expectations were deleted since the tests now pass.

The getArg test was corrected to match the spec:
https://webassembly.github.io/exception-handling/js-api/index.html#dom-exception-getarg

The tag type test had its expectations updated, but is still
(incorrectly) failing because the test also depends on the not-yet-shipped
type reflection proposal. Updated the spec bug to bring this conversation
back to the spec (WebAssembly/exception-handling#172).

Bug: v8:11992
Change-Id: I979a9eb00219e0b9515d43bbeff6f80ac57df7c9
@Ms2ger
Copy link
Contributor

Ms2ger commented Oct 24, 2022

Are you sure you're reading that correctly? Those results seem to be for nightly versions, which also ship the rest of the type reflection API. In any case, thanks for reminding me that I should remove it before we ask for phase advancement.

@ajklein
Copy link

ajklein commented Oct 24, 2022

Thanks for pointing out the version skew. Updated wpt.fyi results show stable Firefox correctly avoiding exposing type, but with stable Safari shipping it. I don't have Safari in front of me right now so can't comment on whether they also ship the rest of Type Reflection, hopefully @justinmichaud can shed some light.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 24, 2022
For most tests, expectations were deleted since the tests now pass.

The getArg test was corrected to match the spec:
https://webassembly.github.io/exception-handling/js-api/index.html#dom-exception-getarg

The tag type test had its expectations updated, but is still
(incorrectly) failing because the test also depends on the not-yet-shipped
type reflection proposal. Updated the spec bug to bring this conversation
back to the spec (WebAssembly/exception-handling#172).

Bug: v8:11992
Change-Id: I979a9eb00219e0b9515d43bbeff6f80ac57df7c9
@Ms2ger
Copy link
Contributor

Ms2ger commented Oct 25, 2022

Based on https://wpt.fyi/results/wasm/jsapi/global/type.tentative.any.html?label=master&label=stable&aligned&view=subtest , it seems like stable Safari ships everything, yeah.

aarongable pushed a commit to chromium/chromium that referenced this issue Oct 25, 2022
For most tests, expectations were deleted since the tests now pass.

The getArg test was corrected to match the spec:
https://webassembly.github.io/exception-handling/js-api/index.html#dom-exception-getarg

The tag type test had its expectations updated, but is still
(incorrectly) failing because the test also depends on the not-yet-shipped
type reflection proposal. Updated the spec bug to bring this conversation
back to the spec (WebAssembly/exception-handling#172).

Bug: v8:11992
Change-Id: I979a9eb00219e0b9515d43bbeff6f80ac57df7c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3972061
Reviewed-by: Andreas Haas <[email protected]>
Commit-Queue: Adam Klein <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1063368}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 25, 2022
For most tests, expectations were deleted since the tests now pass.

The getArg test was corrected to match the spec:
https://webassembly.github.io/exception-handling/js-api/index.html#dom-exception-getarg

The tag type test had its expectations updated, but is still
(incorrectly) failing because the test also depends on the not-yet-shipped
type reflection proposal. Updated the spec bug to bring this conversation
back to the spec (WebAssembly/exception-handling#172).

Bug: v8:11992
Change-Id: I979a9eb00219e0b9515d43bbeff6f80ac57df7c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3972061
Reviewed-by: Andreas Haas <[email protected]>
Commit-Queue: Adam Klein <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1063368}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 25, 2022
For most tests, expectations were deleted since the tests now pass.

The getArg test was corrected to match the spec:
https://webassembly.github.io/exception-handling/js-api/index.html#dom-exception-getarg

The tag type test had its expectations updated, but is still
(incorrectly) failing because the test also depends on the not-yet-shipped
type reflection proposal. Updated the spec bug to bring this conversation
back to the spec (WebAssembly/exception-handling#172).

Bug: v8:11992
Change-Id: I979a9eb00219e0b9515d43bbeff6f80ac57df7c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3972061
Reviewed-by: Andreas Haas <[email protected]>
Commit-Queue: Adam Klein <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1063368}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Nov 12, 2022
…Ts, a=testonly

Automatic update from web-platform-tests
[wasm] Enable and fix wasm exceptions WPTs

For most tests, expectations were deleted since the tests now pass.

The getArg test was corrected to match the spec:
https://webassembly.github.io/exception-handling/js-api/index.html#dom-exception-getarg

The tag type test had its expectations updated, but is still
(incorrectly) failing because the test also depends on the not-yet-shipped
type reflection proposal. Updated the spec bug to bring this conversation
back to the spec (WebAssembly/exception-handling#172).

Bug: v8:11992
Change-Id: I979a9eb00219e0b9515d43bbeff6f80ac57df7c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3972061
Reviewed-by: Andreas Haas <[email protected]>
Commit-Queue: Adam Klein <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1063368}

--

wpt-commits: b8516ab3ff992263cb65e9467a82f4760fd8e89e
wpt-pr: 36607
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Nov 14, 2022
…Ts, a=testonly

Automatic update from web-platform-tests
[wasm] Enable and fix wasm exceptions WPTs

For most tests, expectations were deleted since the tests now pass.

The getArg test was corrected to match the spec:
https://webassembly.github.io/exception-handling/js-api/index.html#dom-exception-getarg

The tag type test had its expectations updated, but is still
(incorrectly) failing because the test also depends on the not-yet-shipped
type reflection proposal. Updated the spec bug to bring this conversation
back to the spec (WebAssembly/exception-handling#172).

Bug: v8:11992
Change-Id: I979a9eb00219e0b9515d43bbeff6f80ac57df7c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3972061
Reviewed-by: Andreas Haas <[email protected]>
Commit-Queue: Adam Klein <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1063368}

--

wpt-commits: b8516ab3ff992263cb65e9467a82f4760fd8e89e
wpt-pr: 36607
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants