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

Option to Generate Unused Types with bindgen!() Macro #10090

Closed
AmmarAbouZor opened this issue Jan 23, 2025 · 2 comments · Fixed by #10311
Closed

Option to Generate Unused Types with bindgen!() Macro #10090

AmmarAbouZor opened this issue Jan 23, 2025 · 2 comments · Fixed by #10311
Assignees

Comments

@AmmarAbouZor
Copy link

Hi, first of all, I want to thank you all for the great work and effort you’ve put into this project.

Feature Request

It would be incredibly helpful to have the option to generate unused types when using the wasmtime::component::bindgen!() macro, similar to the functionality provided by the wit_bindgen::generate!() macro.
I’ve included a code example below to illustrate the desired functionality:

WIT File:

package example:example;

interface types {
  record referenced-type {
    field: string
  }

  record no-ref-type {
    filed: string
  }

  fn: func(t: referenced-type);
}

world example {
  import types;
}

Code in Rust:

mod wsm {
    wasmtime::component::bindgen!({
        path: "wit",
        // NOTE: no equivalent for this option provided
        // generate_unused_types: true,
    });

    use self::example::example::types::{
        ReferencedType,
        // NoRefType: This type won't get generated
    };
}

mod wit {
    wit_bindgen::generate!({
        path: "wit",
        generate_unused_types: true,
    });

    use self::example::example::types::{NoRefType, ReferencedType};
}

Benefit

Generating unused types can be really useful when we aim to group shared types into a single package and generate them only once for use across multiple other packages.

Alternatives

The current workaround involves adding unused worlds, interfaces, or functions that reference the desired types, ensuring they are generated.

pchickey added a commit that referenced this issue Feb 28, 2025
The calculation of TypeInfo only reaches types which are passed to or
from a function. For types which are not reachable, default to the
defining them according to the ownership setting given to bindgen.

I have my doubts that `with`-reuse of bindgen types actually works
properly when bindgen is set to Ownership::Borrowing but thats out
of scope for this PR, which is to fix #10090
@pchickey
Copy link
Contributor

Thank you for reporting this bug. I apologize that this sat for a long time without a response. We noticed it in our triage yesterday and I have a fix in #10311.

github-merge-queue bot pushed a commit that referenced this issue Feb 28, 2025
…ace (#10311)

* wasmtime-wit-bindgen: emit a definition for all types in a wit

The calculation of TypeInfo only reaches types which are passed to or
from a function. For types which are not reachable, default to the
defining them according to the ownership setting given to bindgen.

I have my doubts that `with`-reuse of bindgen types actually works
properly when bindgen is set to Ownership::Borrowing but thats out
of scope for this PR, which is to fix #10090

* component-macro: bless bindgen test output
fitzgen pushed a commit to fitzgen/wasmtime that referenced this issue Feb 28, 2025
…ace (bytecodealliance#10311)

* wasmtime-wit-bindgen: emit a definition for all types in a wit

The calculation of TypeInfo only reaches types which are passed to or
from a function. For types which are not reachable, default to the
defining them according to the ownership setting given to bindgen.

I have my doubts that `with`-reuse of bindgen types actually works
properly when bindgen is set to Ownership::Borrowing but thats out
of scope for this PR, which is to fix bytecodealliance#10090

* component-macro: bless bindgen test output
@AmmarAbouZor
Copy link
Author

@pchickey Thanks a lot for resolving this issue—no need to apologize.
I really appreciate the monster work you've been doing on this project, and naturally, some issues take time to resolve 👍

dicej added a commit to dicej/wasmtime that referenced this issue Mar 14, 2025
* Do proper subtype checking for imported globals during instantiation (bytecodealliance#10304)

* pulley: Fix a panic compiling with debug info (bytecodealliance#10305)

Debug into doesn't work on Pulley anyway but it's better to return a
first-class error rather than a panic. This commit fills out a simple
missing instruction in the Pulley backend to ensure that compilation
gets far enough to the DWARF transform where there's no pulley support
and an error is returned.

* Improve rebuild detection of test-programs (bytecodealliance#10303)

This commit improves the logic of detecting when to rebuild the
`test-programs` artifacts used during test by parsing the `*.d` files
that Cargo emits as part of its compilation and using that as the
`cargo:rerun-if-changed` directive. This not only includes what was
previously depended on but additionally includes features such as `path`
dependencies which might temporarily be used during development.

* Add a missing `apt-get update` before install (bytecodealliance#10310)

I always forget this and it always bites us within a few months. Alas.

* wasmtime-wit-bindgen: gen is a reserved keyword in the lexer starting in 2024 edition (bytecodealliance#10308)

* Test natively on AArch64, not emulated (bytecodealliance#10306)

This commit updates the aarch64 tests to use the new `*-arm` images from
GitHub instead of running through QEMU emulation. This should be a bit
faster but primarily helps build confidence that it runs on real
hardware.

I'll note that release builds aren't updated at this time to run on
native hardware since that's moreso about glibc compatibility and it's a
bit easier to keep the setup we currently have for that. If `*-arm`
machines are noticably faster than the default x64 machines then we
could in theory move everything over to an aarch64-based machine as
opposed to just the aarch64 build.

* chore: fix some typos in comments (bytecodealliance#10309)

Signed-off-by: shenpengfeng <[email protected]>

* Rename `VMRuntimeLimits` to `VMStoreContext` (bytecodealliance#10307)

Way back in time, this struct originally contained the stack and fuel
limits. Then it also got the epoch deadline. Then it also got the exit FP/PC and
entry FP. Now it is just the place where we put per-store mutable data that is
accessed by JIT code and must be shared between all `VMContext`s. So it is time
to rename it.

This commit is purely mechanical and just renames the type and various methods
and variables that use/access it.

* wasmtime-wit-bindgen: emit a definition for all types in a wit interface (bytecodealliance#10311)

* wasmtime-wit-bindgen: emit a definition for all types in a wit

The calculation of TypeInfo only reaches types which are passed to or
from a function. For types which are not reachable, default to the
defining them according to the ownership setting given to bindgen.

I have my doubts that `with`-reuse of bindgen types actually works
properly when bindgen is set to Ownership::Borrowing but thats out
of scope for this PR, which is to fix bytecodealliance#10090

* component-macro: bless bindgen test output

* asm: sse orpd implementation (bytecodealliance#10273)

* sse orpd implementation

assembler integration with isle

format

add clippy reason, reorder avx priority in isle

bless tests for orpd

create separate xmm module

validate function rewrite sse condition

add quote from manual for sse prefix

format changes

move Xmm bits under Reg

* use new isle constructors for sse

* remove unused function

* minor changes

* Winch: Fix consts and multivalue returns (bytecodealliance#10315)

* fixed broken link (bytecodealliance#10318)

* chore: fix parenthesis balance (bytecodealliance#10317)

parentheses are not balanced here

* winch(aarch64): ABI integration (bytecodealliance#10312)

* winch(aarch64): ABI integration

This commit finalizes the ABI integration between Winch and Cranelift,
notably:

* Updates the Cranelift ABI to ensure that all the Winch register
  clobbers are taken into account.
* Updates the Winch ABI to treat x28 as callee-saved, since it's used as
  the shadow stack pointer.

The alternative to treating x28 as callee-saved is to treat it as
caller-saved and save it when required e.g., at call-sites, even though
this approach works, it's probably more efficient to perform a store/pop
once per function, to minimize the number of move instructions required.

There are still some changes needed in order to fully enable running
spec tests for aarch64, however, this change is one step further. If
interested, you can run the call.wast test via:

cargo run -- wast -Ccompiler=winch tests/spec_testsuite/call_indirect.wast

* Update disas tests

* Use `alloc_zeroed` to allocate dynamic table elements (bytecodealliance#10313)

* Use `alloc_zeroed` to allocate dynamic table elements

This allows us to get pre-zeroed memory from the global allocator, rather than
needing to manually zero-initialize the elements.

* Don't ask the global allocator to allocate a block of size zero

* x64: use Rust types for assembler immediates (bytecodealliance#10302)

Previously we used `AssemblerImm*` and `AssemblerSimm*` throughout the
x64 ISLE to indicate the type of immediate an instruction would receive.
Alex has noted previously that this is unnecessary; it does after all
create more ties between the `cranelift-codegen` ISLE and
`cranelift-assembler-x64` that could possibly break in the future. This
change removes those ties by using Rust types in ISLE (e.g., `u8`, `i8`,
`u16`, etc.) and converting to the expected assembler type in the ISLE
glue layer.

* Update ir.md (bytecodealliance#10319)

Hello,

A possible typo in this text:

"Forward" Should be "Foreword"

"Forward" means to move ahead, but the correct term for an introduction is "Foreword".

Thanks.

* Expose GC refs to Wasm in `gc_alloc_raw` libcall (bytecodealliance#10322)

* Expose GC refs to Wasm in `gc_alloc_raw` libcall

As we are returning a GC reference to Wasm, we need to mark that GC reference as
exposed to Wasm.

Fixes bytecodealliance#9669

* miri ignore test that calls wasm and therefore can't run in miri

* Bump Wasmtime to 32.0.0 (bytecodealliance#10330)

Co-authored-by: Wasmtime Publish <[email protected]>

* add component-model-async/{fused|futures|streams}.wast tests (bytecodealliance#10106)

* add component-model-async/{fused|futures|streams}.wast tests

This is another piece of bytecodealliance#9582 which I'm splitting out to make review easier.

The fused.wast test exercises fused adapter generation for various flavors of
intercomponent async->async, async->sync, and sync->async calls.

The futures.wast and streams.wast tests exercise the various intrinsics
(e.g. `stream.read`, `future.close_writable`, etc.) involving `future`s and
`stream`s.

The remaining changes fill in some TODOs to make the tests pass, plus plumbing
for a few intrinsics which aren't needed for these tests but which set the
foundation for future tests.

Signed-off-by: Joel Dice <[email protected]>

* address review feedback

Signed-off-by: Joel Dice <[email protected]>

---------

Signed-off-by: Joel Dice <[email protected]>

* Update wasm-tools to the latest revision (bytecodealliance#10314)

* Update wasm-tools to the latest revision

* Update component async bits for new intrinsics

* Ignore options for now

* I truly, and fundamentally, do not understand `cargo vet`

* Fix a test

* Ignore fuzzing `testcase0.wasm` et al in `.gitignore` (bytecodealliance#10341)

* remove temporary patch from Cargo.toml

Signed-off-by: Joel Dice <[email protected]>

* Fix building artifacts alone in isolation

Few more crate features needed

* Rename backpressure intrinsic

* Patch to work-in-progress branches

* temporarily stub-out waitable-set.wait calls in tests

Signed-off-by: Joel Dice <[email protected]>

* fix WAT/WAST test regressions

Signed-off-by: Joel Dice <[email protected]>

* Switch to published wasm-tools deps

* bless bindgen test output

Signed-off-by: Joel Dice <[email protected]>

* remove realloc from `task.return` in WAT tests

This is no longer allowed by wasmparser.

Signed-off-by: Joel Dice <[email protected]>

* Switch to wit-bindgen upstream

---------

Signed-off-by: shenpengfeng <[email protected]>
Signed-off-by: Joel Dice <[email protected]>
Co-authored-by: Nick Fitzgerald <[email protected]>
Co-authored-by: Alex Crichton <[email protected]>
Co-authored-by: Pat Hickey <[email protected]>
Co-authored-by: shenpengfeng <[email protected]>
Co-authored-by: Rahul <[email protected]>
Co-authored-by: Jeffrey Charles <[email protected]>
Co-authored-by: ifsheldon <[email protected]>
Co-authored-by: Bongjun Jang <[email protected]>
Co-authored-by: Saúl Cabrera <[email protected]>
Co-authored-by: Andrew Brown <[email protected]>
Co-authored-by: owen <[email protected]>
Co-authored-by: wasmtime-publish <[email protected]>
Co-authored-by: Wasmtime Publish <[email protected]>
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 a pull request may close this issue.

2 participants