Drop support for old-style *.witx files
#195
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I today started looking at updating this project to the new draft of the canonical ABI specified at WebAssembly/component-model#23 and while there's quite a few changes that need to happen the first thing that struck me was that maintaining all of the support for old-style witx files is pretty onerous. Especially the ABI-calculating code has lots of special cases for various witx constructs and the Preview1 ABI, and I wasn't really sure how to update this in many situations. Overall the original purpose of the witx support was to prove out that it's possible to support both the old witx abi and the new canonical ABI in the same generator. The canonical ABI has changed significantly in the meantime however and this doesn't necessarily make sense to do any more. I think it would be best now to reevaluate at the point when WASI is ready to switch to the component model what to do with the old witx support. I no longer think that "build it in here" is the obvious option. As this diff shows there's quite a bit of weight to carry the old witx abis as this commit clocks in at nearly 7000 lines removed. The specifics being dropped here are: * Parsing support for `*.witx` * Support for `Pointer` and `ConstPointer` * Support for `WitxInstruction` * Support for push/pull buffers The push/pull buffer feature was never actually fully implemented, even for Rust hosts they only kind-of worked. Most other generators never even implemented support for them. Additionally support for other `*.witx` constructs was somewhat spotty at best with very few tests. My hope is that there are no existing users of this support. If there are then I think it's best to re-evaluate how best to solve the scenario on-hand.
peterhuene
approved these changes
Apr 19, 2022
alexcrichton
added a commit
to alexcrichton/witx-bindgen
that referenced
this pull request
Apr 19, 2022
Further iteration from bytecodealliance#195 and dropping more features that are only needed for `*.witx`, the `usize` and `char8` types were only added for backcompat with witx itself.
alexcrichton
added a commit
to alexcrichton/witx-bindgen
that referenced
this pull request
Apr 19, 2022
Some more vestigial remains from bytecodealliance#195
This was referenced Apr 19, 2022
alexcrichton
added a commit
that referenced
this pull request
Apr 19, 2022
alexcrichton
added a commit
to alexcrichton/witx-bindgen
that referenced
this pull request
Apr 19, 2022
Further iteration from bytecodealliance#195 and dropping more features that are only needed for `*.witx`, the `usize` and `char8` types were only added for backcompat with witx itself.
alexcrichton
added a commit
that referenced
this pull request
Apr 20, 2022
alexcrichton
added a commit
to alexcrichton/witx-bindgen
that referenced
this pull request
Apr 20, 2022
Not actually needed for anything after bytecodealliance#195
alexcrichton
added a commit
that referenced
this pull request
Apr 20, 2022
21 tasks
willemneal
pushed a commit
to AhaLabs/wit-bindgen
that referenced
this pull request
May 31, 2022
I today started looking at updating this project to the new draft of the canonical ABI specified at WebAssembly/component-model#23 and while there's quite a few changes that need to happen the first thing that struck me was that maintaining all of the support for old-style witx files is pretty onerous. Especially the ABI-calculating code has lots of special cases for various witx constructs and the Preview1 ABI, and I wasn't really sure how to update this in many situations. Overall the original purpose of the witx support was to prove out that it's possible to support both the old witx abi and the new canonical ABI in the same generator. The canonical ABI has changed significantly in the meantime however and this doesn't necessarily make sense to do any more. I think it would be best now to reevaluate at the point when WASI is ready to switch to the component model what to do with the old witx support. I no longer think that "build it in here" is the obvious option. As this diff shows there's quite a bit of weight to carry the old witx abis as this commit clocks in at nearly 7000 lines removed. The specifics being dropped here are: * Parsing support for `*.witx` * Support for `Pointer` and `ConstPointer` * Support for `WitxInstruction` * Support for push/pull buffers The push/pull buffer feature was never actually fully implemented, even for Rust hosts they only kind-of worked. Most other generators never even implemented support for them. Additionally support for other `*.witx` constructs was somewhat spotty at best with very few tests. My hope is that there are no existing users of this support. If there are then I think it's best to re-evaluate how best to solve the scenario on-hand.
willemneal
pushed a commit
to AhaLabs/wit-bindgen
that referenced
this pull request
May 31, 2022
Some more vestigial remains from bytecodealliance#195
willemneal
pushed a commit
to AhaLabs/wit-bindgen
that referenced
this pull request
May 31, 2022
Further iteration from bytecodealliance#195 and dropping more features that are only needed for `*.witx`, the `usize` and `char8` types were only added for backcompat with witx itself.
willemneal
pushed a commit
to AhaLabs/wit-bindgen
that referenced
this pull request
May 31, 2022
Not actually needed for anything after bytecodealliance#195
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I today started looking at updating this project to the new draft of the
canonical ABI specified at WebAssembly/component-model#23 and while
there's quite a few changes that need to happen the first thing that
struck me was that maintaining all of the support for old-style witx
files is pretty onerous. Especially the ABI-calculating code has lots of
special cases for various witx constructs and the Preview1 ABI, and I
wasn't really sure how to update this in many situations.
Overall the original purpose of the witx support was to prove out that
it's possible to support both the old witx abi and the new canonical ABI
in the same generator. The canonical ABI has changed significantly in
the meantime however and this doesn't necessarily make sense to do any
more. I think it would be best now to reevaluate at the point when WASI
is ready to switch to the component model what to do with the old witx
support. I no longer think that "build it in here" is the obvious
option. As this diff shows there's quite a bit of weight to carry the
old witx abis as this commit clocks in at nearly 7000 lines removed.
The specifics being dropped here are:
*.witxPointerandConstPointerWitxInstructionThe push/pull buffer feature was never actually fully implemented, even
for Rust hosts they only kind-of worked. Most other generators never
even implemented support for them. Additionally support for other
*.witxconstructs was somewhat spotty at best with very few tests.My hope is that there are no existing users of this support. If there
are then I think it's best to re-evaluate how best to solve the scenario
on-hand.