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

Start work towards always-warnings in wasmtime #10131

Merged

Conversation

alexcrichton
Copy link
Member

@alexcrichton alexcrichton commented Jan 28, 2025

This PR is an attempt to make progress after #10108. The goal of this PR is to create a list of features to burn down over time and to have dead_code and unused_imports lints on-by-default in Wasmtime. The feature = "default" gate on this allow directive was replaced with individual features. The hope is that as individual features are removed from this list we can fix the resulting warnings and the commit such a ratchet.

The first commit here replaces not(feature = "default") with any(not(...)) for all features in the default feature. A few features are removed until one was found to trigger a warning. The warning was then fixed generating this PR.

My hope is that if others feel this is a good approach to take that we'd land this and then over time continue to ratchet down this list and remove more warnings. I don't think this is a perfect strategy because we don't test all feature combinations in CI, but it's hopefully at least progress.

This commit is an attempt to make progress after bytecodealliance#10108. The goal of
this commit is to create a list of features to burn down over time and
to have `dead_code` and `unused_imports` lints on-by-default in
Wasmtime. The `feature = "default"` gate on this `allow` directive was
replaced with individual features. The hope is that as individual
features are removed from this list we can fix the resulting warnings
and the commit such a ratchet.
Remove the features from the crate and test compiling with all other
features to ensure that no warnings are issued.
@alexcrichton alexcrichton requested a review from a team as a code owner January 28, 2025 01:27
@alexcrichton alexcrichton requested review from fitzgen and removed request for a team January 28, 2025 01:27
Switch `target_os = "macos"` to `target_vendor = "apple"` to align
requirements.
@github-actions github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label Jan 28, 2025
@alexcrichton alexcrichton added this pull request to the merge queue Jan 28, 2025
Merged via the queue into bytecodealliance:main with commit ebd9824 Jan 28, 2025
153 checks passed
@alexcrichton alexcrichton deleted the towards-warnings-always branch January 28, 2025 15:44
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Jan 28, 2025
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Jan 28, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jan 28, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jan 28, 2025
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Jan 28, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jan 28, 2025
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Jan 28, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jan 28, 2025
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Jan 28, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jan 28, 2025
* Enable warnings if `pooling-allocator` is disabled

Continuation of work in #10131

* Fix windows build
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Jan 28, 2025
Continuation of work in bytecodealliance#10131.

This required a number of organizational changes to help cut down on
`#[cfg]`, notably lots of async-related pieces from `store.rs` have
moved to `store/async_.rs` to avoid having lots of conditional imports.
Additionally this removes all of the `#[cfg]` annotations on those
methods already.

Additionally the signature of `AsyncCx::block_on` was updated to be a
bit more general to ideally remove the need for `Pin` but it ended up
not panning out quite just yet. In the future it should be possible to
remove the need for `Pin` at callsites though.
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Jan 29, 2025
Continuation of work in bytecodealliance#10131.

This required a number of organizational changes to help cut down on
`#[cfg]`, notably lots of async-related pieces from `store.rs` have
moved to `store/async_.rs` to avoid having lots of conditional imports.
Additionally this removes all of the `#[cfg]` annotations on those
methods already.

Additionally the signature of `AsyncCx::block_on` was updated to be a
bit more general to ideally remove the need for `Pin` but it ended up
not panning out quite just yet. In the future it should be possible to
remove the need for `Pin` at callsites though.
github-merge-queue bot pushed a commit that referenced this pull request Jan 29, 2025
* Enable warnings if `async` is disabled

Continuation of work in #10131.

This required a number of organizational changes to help cut down on
`#[cfg]`, notably lots of async-related pieces from `store.rs` have
moved to `store/async_.rs` to avoid having lots of conditional imports.
Additionally this removes all of the `#[cfg]` annotations on those
methods already.

Additionally the signature of `AsyncCx::block_on` was updated to be a
bit more general to ideally remove the need for `Pin` but it ended up
not panning out quite just yet. In the future it should be possible to
remove the need for `Pin` at callsites though.

* Rebase conflicts
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Jan 29, 2025
Continuation of work in bytecodealliance#10131. This additionally handles turning off
`gc-null` and `gc-drc` and the various combinations within.
github-merge-queue bot pushed a commit that referenced this pull request Jan 30, 2025
* Enable warnings if `gc` is disabled

Continuation of work in #10131. This additionally handles turning off
`gc-null` and `gc-drc` and the various combinations within.

* Fix some more warnings

* Fix a feature combo build
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Jan 30, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jan 30, 2025
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Jan 30, 2025
Finishes the work started in bytecodealliance#10131 and this means that the crate is no
longer special and has the same default settings of all other crates in
all situations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasmtime:api Related to the API of the `wasmtime` crate itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants