Wasmtime GitHub notifications bot (Apr 16 2025 at 18:37): Wasmtime GitHub notifications bot (Apr 16 2025 at 18:37):pchickey opened PR #10597 from bytecodealliance:pch/component_instance_export to bytecodealliance:main:
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Apr 16 2025 at 18:37):pchickey requested alexcrichton for a review on PR #10597.
Wasmtime GitHub notifications bot (Apr 16 2025 at 18:37):pchickey requested wasmtime-core-reviewers for a review on PR #10597.
Wasmtime GitHub notifications bot (Apr 16 2025 at 18:40):pchickey edited PR #10597:
Prior to this PR, there was
Component::export_indexwhich looked up(ComponentItem, ComponentExportIndex)andInstance::get_exportwhich looked upComponentExportIndex.Now each of these structures has a
get_exportwhich looks up the(ComponentItem, ComponentExportIndex)andget_export_indexwhich looks up just theComponentExportIndex. The docs point to the sister methods, and mention that creating the ComponentItem is comparatively expensive so use the cheaperget_export_indexif you don't need it.<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Apr 16 2025 at 18:40):pchickey edited PR #10597:
Prior to this PR, there was
Component::export_indexwhich looked up(ComponentItem, ComponentExportIndex)andInstance::get_exportwhich looked upComponentExportIndex.Now each of these structures has a
get_exportwhich looks up the(ComponentItem, ComponentExportIndex)andget_export_indexwhich looks up just theComponentExportIndex. The docs point to the sister methods, and mention that creating the ComponentItem is comparatively expensive so use the cheaperget_export_indexif you don't need it.This will be a breaking change to users, but the hope is that most users are only using these by way of wasmtime-wit-bindgen, so hopefully its not a big deal.
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Apr 16 2025 at 19:18):pchickey updated PR #10597.
Wasmtime GitHub notifications bot (Apr 16 2025 at 19:29):pchickey updated PR #10597.
Wasmtime GitHub notifications bot (Apr 16 2025 at 19:37):pchickey updated PR #10597.
Wasmtime GitHub notifications bot (Apr 16 2025 at 20:27):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Apr 16 2025 at 20:27):alexcrichton created PR review comment:
Possible bikeshed: instead of
get_export{,_index}, maybeget_export{,_and_type}? (e.g. favoring the just-the-index version)
Wasmtime GitHub notifications bot (Apr 16 2025 at 20:27):alexcrichton created PR review comment:
Since this is a copy of what's in
component.rs, could there be a crate-private helper which takes theInstanceTypeas an argument and both this and the public component version call into that?
Wasmtime GitHub notifications bot (Apr 16 2025 at 20:27):alexcrichton created PR review comment:
This looks like it can change as well to the index-only version?
Wasmtime GitHub notifications bot (Apr 16 2025 at 21:07):pchickey submitted PR review.
Wasmtime GitHub notifications bot (Apr 16 2025 at 21:07):pchickey created PR review comment:
Eh, maybe, but get_export_and_type feels too smalltalky and 98% of users just use bindgen which hides all that so I'll respectfully decline
Wasmtime GitHub notifications bot (Apr 16 2025 at 21:11):pchickey submitted PR review.
Wasmtime GitHub notifications bot (Apr 16 2025 at 21:11):pchickey created PR review comment:
Good call, I'll make a
pub(crate) ComponentItem::from_exportthat takes a wasmtime-environ::Export which should reduce the duplication to just a line or two.
Wasmtime GitHub notifications bot (Apr 16 2025 at 21:11):pchickey updated PR #10597.
Wasmtime GitHub notifications bot (Apr 16 2025 at 21:12):pchickey submitted PR review.
Wasmtime GitHub notifications bot (Apr 16 2025 at 21:12):pchickey created PR review comment:
Fixed 947b90df
Wasmtime GitHub notifications bot (Apr 16 2025 at 21:24):pchickey updated PR #10597.
Wasmtime GitHub notifications bot (Apr 16 2025 at 21:25):pchickey submitted PR review.
Wasmtime GitHub notifications bot (Apr 16 2025 at 21:25):pchickey created PR review comment:
Wasmtime GitHub notifications bot (Apr 16 2025 at 21:25):pchickey requested alexcrichton for a review on PR #10597.
Wasmtime GitHub notifications bot (Apr 16 2025 at 21:29):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Apr 16 2025 at 21:31):pchickey has enabled auto merge for PR #10597.
Wasmtime GitHub notifications bot (Apr 16 2025 at 22:01):pchickey merged PR #10597.
Last updated: Dec 06 2025 at 06:05 UTC