Skip to content

Remove component union types#6913

Merged
alexcrichton merged 1 commit into
bytecodealliance:mainfrom
lann:remove-union
Aug 26, 2023
Merged

Remove component union types#6913
alexcrichton merged 1 commit into
bytecodealliance:mainfrom
lann:remove-union

Conversation

@lann
Copy link
Copy Markdown
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
Copy Markdown

github-actions Bot commented Aug 25, 2023

Subscribe to Label Action

cc @fitzgen, @peterhuene

Details 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
Comment thread 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
Copy Markdown
Contributor Author

@lann lann Aug 25, 2023

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
Comment thread 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
Copy Markdown
Contributor Author

@lann lann Aug 25, 2023

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
Copy Markdown
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
Copy Markdown
Member

alexcrichton commented Aug 25, 2023

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
Copy Markdown
Member

alexcrichton commented Aug 25, 2023

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
Copy Markdown
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
Copy Markdown
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