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

Document new implementation-defined limits #360

Merged
merged 3 commits into from Mar 23, 2023
Merged

Conversation

tlively
Copy link
Member

@tlively tlively commented Mar 22, 2023

Based on discussion in #335 and subgroup meetings.

Closes #335.

@tlively
Copy link
Member Author

tlively commented Mar 22, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@tlively
Copy link
Member Author

tlively commented Mar 22, 2023

The one limit we discussed that is not in this PR is the byte limit on the size of global initializers. I decided not to include it because it is under discussion separately in the extended-const proposal: WebAssembly/extended-const#15.

@@ -779,6 +779,14 @@ The opcode for heap types is encoded as an `s33`.
| 0xfb70 | `extern.internalize` | |
| 0xfb71 | `extern.externalize` | |

## Implementation-defined Limits
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since these are constraints of the JS API spec, it should make more sense to move this section down below and turn it into a subsection of that one.

@tlively
Copy link
Member Author

tlively commented Mar 23, 2023

I've now fixed the link in MVP.md to point to the actual bikeshed document containing the JS spec (rather than the outdated MVP-JS.md) and I've put the new limits directly into the relevant section in the bikeshed doc.

@rossberg
Copy link
Member

Hm, I think we should update the MVP-JS doc to match the status quo. Otherwise, we're losing the documentation of that part of the proposal and its design. Looking at the final spec provides no useful information about what the proposal was about. We are intentionally keeping around design docs in the proposals directory, after all.

@tlively tlively changed the base branch from main to update-mvp-js March 23, 2023 17:11
@tlively tlively mentioned this pull request Mar 23, 2023
@tlively tlively changed the title Document new implementation-defined limits in MVP.md Document new implementation-defined limits Mar 23, 2023
@tlively
Copy link
Member Author

tlively commented Mar 23, 2023

Ok, this now documents the limits in a revamped MVP-JS.md doc as well as in the actual spec doc.

@tlively tlively merged commit db976cc into update-mvp-js Mar 23, 2023
tlively added a commit that referenced this pull request Nov 6, 2023
The new limits were previously merged in #360, but apparently that PR was to
another branch that itself was never merged. Reland that change with two
additional fixes:

 - Add digit separators in the numbers to improve readability and match the
   formatting of other limits.

 - Add an explicit limit of 1,000,000 types per recursion group. This is not
   intended to be a functional change, but this limit was previously left
   implicit.
tlively added a commit that referenced this pull request Nov 7, 2023
The new limits were previously merged in #360, but apparently that PR was to
another branch that itself was never merged. Reland that change with two
additional fixes:

 - Add digit separators in the numbers to improve readability and match the
   formatting of other limits.

 - Add an explicit limit of 1,000,000 types per recursion group. This is not
   intended to be a functional change, but this limit was previously left
   implicit.
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 this pull request may close these issues.

Implementation-defined limits
2 participants