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

wasmparser: Implement missing WasmFeatures configs #482

Merged

Conversation

Robbepop
Copy link
Contributor

Closes: #481

The following Wasm proposals are now covered by the WasmFeatures type:

  • saturating_float_to_int
  • sign_extension
  • mutable_global

All those features are enabled by default to not break existing code that uses WasmFeatures::deafult.
The Wasm spec has verbose names for sign_extension and especially saturating_float_to_int with sign-extension-ops and nontrapping-float-to-int-conversions so I thought it is nicer to have shorter names in the WasmFeatures struct. If the maintainers of the wasmparser crate don't like this I'd love to accept name proposals.

Since there were no negative feature tests for all the other features (from what I saw) I didn't know if those tests are actually wanted. If those tests are wanted I can implement them as well for the new features.

The following Wasm proposals are now covered by the WasmFeatures type:

- saturating_float_to_int
- sign_extension
- mutable_global

All enabled by default.
@Robbepop Robbepop changed the title Implement missing WasmFeatures configs wasmparser: Implement missing WasmFeatures configs Feb 15, 2022
Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Thanks! Could you add a smoke test or two to the tests/local/missing-features directory?

crates/wasmparser/src/validator.rs Outdated Show resolved Hide resolved
@Robbepop
Copy link
Contributor Author

@alexcrichton Thanks for your review. I applied all your review suggestions.

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.

wasmparser: Missing options in WasmFeatures
2 participants