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.

Sign up for GitHub

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

Jump to bottom

Remove component union types #6913

Merged
merged 1 commit into from
Aug 26, 2023
Merged

Remove component union types #6913

merged 1 commit into from
Aug 26, 2023

Conversation

lann
Copy link
Contributor

@lann lann commented Aug 25, 2023 •
edited
Loading

...and bump wasm-tools deps.

Fixes #6871

@lann

This comment was marked as resolved.

Sign in to view
@lann lann force-pushed the remove-union branch 2 times, most recently from c86c020 to 1b7c863 Compare August 25, 2023 15:50
@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:wasm fuzzing Issues related to our fuzzing infrastructure wasmtime:api Related to the API of the `wasmtime` crate itself labels Aug 25, 2023
@github-actions
Copy link

Subscribe to Label Action

cc @fitzgen, @peterhuene

This issue or pull request has been labeled: "cranelift", "cranelift:wasm", "fuzzing", "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

  • fitzgen: fuzzing
  • peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

@lann lann force-pushed the remove-union branch 2 times, most recently from 6be3b47 to a3d11f3 Compare August 25, 2023 19:45
lann
lann commented Aug 25, 2023
View reviewed changes
cranelift/wasm/src/sections_translator.rs
Comment on lines +52 to +54
for ty in types.into_iter_err_on_gc_types() {
let ty = environ.convert_func_type(&ty?);
environ.declare_type_func(ty)?;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexcrichton Updated this

lann
lann commented Aug 25, 2023
View reviewed changes
crates/environ/src/module_environ.rs
Comment on lines +240 to +242
for ty in types.into_iter_err_on_gc_types() {
let ty = self.convert_func_type(&ty?);
self.declare_type_func(ty)?;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

...and this

@lann lann marked this pull request as ready for review August 25, 2023 20:54
@lann lann requested review from a team as code owners August 25, 2023 20:54
@lann lann requested review from cfallin and pchickey and removed request for a team August 25, 2023 20:54
@lann
Copy link
Contributor Author

lann commented Aug 25, 2023 •
edited
Loading

I just pushed an update here anticipating a wit-bindgen 0.11.0 release. Obviously CI failed, but someone with access can hopefully just retrigger the CI jobs after that release and this should then be mergeable.

The previous CI run against wit-bindgen main passed: https://github.com/bytecodealliance/wasmtime/actions/runs/5979833964

@alexcrichton
Copy link
Member

At this rate I think I'll post a bump for 0.11.0 on Monday, but it'll definitely include this

@alexcrichton
Copy link
Member

Oh oops wrong project, I see now! I think it's ok to avoid a wit-bindgen update here as that's mainly used for tests. The dependency between wit-bindgen and wasmtime mainly goes the other direction where wit-bindgen relies on wasmtime for tests, but it's ok in this regard for them to be slightly out of sync.

@lann
Copy link
Contributor Author

lann commented Aug 25, 2023

Ah yeah I didn't really think that through. Well I'll fix it up when I get a chance if someone else doesnt get to it.

@lann lann force-pushed the remove-union branch 2 times, most recently from 9767ff1 to fa76ff9 Compare August 26, 2023 14:05
@lann
Remove component union types
5ea30c7 
- Bump wasm-tools deps
- Use new TypeSectionReader::into_iter_err_on_gc_types method
@lann
Copy link
Contributor Author

lann commented Aug 26, 2023

This should be ready now.

@alexcrichton alexcrichton added this pull request to the merge queue Aug 26, 2023
Merged via the queue into bytecodealliance:main with commit 3590236 Aug 26, 2023
@lann lann deleted the remove-union branch August 27, 2023 00:26
eduardomourar pushed a commit to eduardomourar/wasmtime that referenced this pull request Sep 6, 2023
@lann @eduardomourar
Remove component union types (bytecodealliance#6913)
0e662a2 
- Bump wasm-tools deps
- Use new TypeSectionReader::into_iter_err_on_gc_types method
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexcrichton alexcrichton alexcrichton approved these changes

@cfallin cfallin Awaiting requested review from cfallin cfallin is a code owner automatically assigned from bytecodealliance/wasmtime-compiler-reviewers

@pchickey pchickey Awaiting requested review from pchickey pchickey is a code owner automatically assigned from bytecodealliance/wasmtime-core-reviewers

Assignees
No one assigned
Labels
cranelift:wasm cranelift Issues related to the Cranelift code generator fuzzing Issues related to our fuzzing infrastructure wasmtime:api Related to the API of the `wasmtime` crate itself
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove component union types
2 participants
@lann @alexcrichton