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

Support finishing component types without compilation artifacts #7888

Conversation

macovedj
Copy link

@macovedj macovedj commented Feb 7, 2024

As referenced in this PR, the addition of component type reflection support breaks jco's current dependence on wasmtime-environ, and since jco doesn't leverage an Engine, I'm not immediately aware of a good way to adapt jco. So this PR just does the simple thing and reintroduces the old finish function with the new name finish_sans_reflection, right next to the one that was updated for reflection support, though if there's a way I'm missing to adapt jco, then this could be reconsidered.

@macovedj macovedj requested a review from a team as a code owner February 7, 2024 19:01
@macovedj macovedj requested review from alexcrichton and removed request for a team February 7, 2024 19:01
@guybedford
Copy link
Contributor

Can confirm this gets the tests in the JCO upgrade PR to pass on bytecodealliance/jco#368.

@alexcrichton
Copy link
Member

Thanks! Would it be possible to call finish as-is though? I think passing an empty PrimaryMap and empty iterators should preserve the same behavior for jco?

@macovedj
Copy link
Author

macovedj commented Feb 8, 2024

Thanks! Would it be possible to call finish as-is though? I think passing an empty PrimaryMap and empty iterators should preserve the same behavior for jco?

Yeah it seems you're right. Just pushed to the jco branch and have it pointing at wasmtime main with empty PrimaryMap and iterators.

@macovedj macovedj closed this Feb 8, 2024
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.

3 participants