Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Deny wasm_c_abi lint to nudge the last 25% #129534

Merged

Conversation

workingjubilee
Copy link
Member

This shouldn't affect projects indirectly depending on wasm-bindgen because cargo passes --cap-lints=allow when building dependencies.

The motivation is that the ecosystem has mostly taken up the versions of wasm-bindgen that are compatible in general, but ~25% or so of recent downloads remain on lower versions. However, this change might still be unnecessarily disruptive. I mostly propose it as a discussion point.

Related:

This shouldn't affect projects indirectly depending on wasm-bindgen
because cargo passes `--cap-lints=allow` when building dependencies.
@rustbot
Copy link
Collaborator

rustbot commented Aug 24, 2024

r? @davidtwco

rustbot has assigned @davidtwco.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 24, 2024
@workingjubilee
Copy link
Member Author

cc @daxpedda Thoughts?

Copy link
Contributor

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was considering opening a stabilization PR soon, but I think doing this first is a good idea.

@alexcrichton
Copy link
Member

@bors: r=daxpedda,me

@bors
Copy link
Contributor

bors commented Aug 26, 2024

📌 Commit af05882 has been approved by daxpedda,me

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 26, 2024
@bjorn3
Copy link
Member

bjorn3 commented Aug 26, 2024

Shouldn't it be r=daxpedda,alexcrichton?

@alexcrichton
Copy link
Member

@bors: r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 26, 2024
@alexcrichton
Copy link
Member

@bors: r=daxpedda,alexcrichton

I am clearly a bors pro

@bors
Copy link
Contributor

bors commented Aug 26, 2024

📌 Commit af05882 has been approved by daxpedda,alexcrichton

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 26, 2024
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Aug 29, 2024
…fcw-to-deny, r=daxpedda,alexcrichton

Deny `wasm_c_abi` lint to nudge the last 25%

This shouldn't affect projects indirectly depending on wasm-bindgen because cargo passes `--cap-lints=allow` when building dependencies.

The motivation is that the ecosystem has mostly taken up the versions of wasm-bindgen that are compatible in general, but ~25% or so of recent downloads remain on lower versions. However, this change might still be unnecessarily disruptive. I mostly propose it as a discussion point.
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Aug 29, 2024
…fcw-to-deny, r=daxpedda,alexcrichton

Deny `wasm_c_abi` lint to nudge the last 25%

This shouldn't affect projects indirectly depending on wasm-bindgen because cargo passes `--cap-lints=allow` when building dependencies.

The motivation is that the ecosystem has mostly taken up the versions of wasm-bindgen that are compatible in general, but ~25% or so of recent downloads remain on lower versions. However, this change might still be unnecessarily disruptive. I mostly propose it as a discussion point.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 29, 2024
…kingjubilee

Rollup of 9 pull requests

Successful merges:

 - rust-lang#120221 (Don't make statement nonterminals match pattern nonterminals)
 - rust-lang#127912 (std: make `thread::current` available in all `thread_local!` destructors)
 - rust-lang#128166 (Improved `checked_isqrt` and `isqrt` methods)
 - rust-lang#129123 (rustdoc-json: Add test for `Self` type)
 - rust-lang#129366 (linker: Synchronize native library search in rustc and linker)
 - rust-lang#129527 (Don't use `TyKind` in a lint)
 - rust-lang#129534 (Deny `wasm_c_abi` lint to nudge the last 25%)
 - rust-lang#129640 (Re-enable android tests/benches in alloc/core)
 - rust-lang#129675 (allow BufReader::peek to be called on unsized types)

r? `@ghost`
`@rustbot` modify labels: rollup
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Aug 29, 2024
…fcw-to-deny, r=daxpedda,alexcrichton

Deny `wasm_c_abi` lint to nudge the last 25%

This shouldn't affect projects indirectly depending on wasm-bindgen because cargo passes `--cap-lints=allow` when building dependencies.

The motivation is that the ecosystem has mostly taken up the versions of wasm-bindgen that are compatible in general, but ~25% or so of recent downloads remain on lower versions. However, this change might still be unnecessarily disruptive. I mostly propose it as a discussion point.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 29, 2024
…kingjubilee

Rollup of 8 pull requests

Successful merges:

 - rust-lang#120221 (Don't make statement nonterminals match pattern nonterminals)
 - rust-lang#129123 (rustdoc-json: Add test for `Self` type)
 - rust-lang#129366 (linker: Synchronize native library search in rustc and linker)
 - rust-lang#129527 (Don't use `TyKind` in a lint)
 - rust-lang#129534 (Deny `wasm_c_abi` lint to nudge the last 25%)
 - rust-lang#129640 (Re-enable android tests/benches in alloc/core)
 - rust-lang#129675 (allow BufReader::peek to be called on unsized types)
 - rust-lang#129731 (Allow running `./x.py test compiler`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 29, 2024
…kingjubilee

Rollup of 8 pull requests

Successful merges:

 - rust-lang#120221 (Don't make statement nonterminals match pattern nonterminals)
 - rust-lang#129123 (rustdoc-json: Add test for `Self` type)
 - rust-lang#129366 (linker: Synchronize native library search in rustc and linker)
 - rust-lang#129527 (Don't use `TyKind` in a lint)
 - rust-lang#129534 (Deny `wasm_c_abi` lint to nudge the last 25%)
 - rust-lang#129640 (Re-enable android tests/benches in alloc/core)
 - rust-lang#129675 (allow BufReader::peek to be called on unsized types)
 - rust-lang#129731 (Allow running `./x.py test compiler`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 29, 2024
…kingjubilee

Rollup of 8 pull requests

Successful merges:

 - rust-lang#120221 (Don't make statement nonterminals match pattern nonterminals)
 - rust-lang#129123 (rustdoc-json: Add test for `Self` type)
 - rust-lang#129366 (linker: Synchronize native library search in rustc and linker)
 - rust-lang#129527 (Don't use `TyKind` in a lint)
 - rust-lang#129534 (Deny `wasm_c_abi` lint to nudge the last 25%)
 - rust-lang#129640 (Re-enable android tests/benches in alloc/core)
 - rust-lang#129675 (allow BufReader::peek to be called on unsized types)
 - rust-lang#129731 (Allow running `./x.py test compiler`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 29, 2024
…kingjubilee

Rollup of 8 pull requests

Successful merges:

 - rust-lang#120221 (Don't make statement nonterminals match pattern nonterminals)
 - rust-lang#129123 (rustdoc-json: Add test for `Self` type)
 - rust-lang#129366 (linker: Synchronize native library search in rustc and linker)
 - rust-lang#129527 (Don't use `TyKind` in a lint)
 - rust-lang#129534 (Deny `wasm_c_abi` lint to nudge the last 25%)
 - rust-lang#129640 (Re-enable android tests/benches in alloc/core)
 - rust-lang#129675 (allow BufReader::peek to be called on unsized types)
 - rust-lang#129731 (Allow running `./x.py test compiler`)

r? `@ghost`
`@rustbot` modify labels: rollup
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Aug 30, 2024
…fcw-to-deny, r=daxpedda,alexcrichton

Deny `wasm_c_abi` lint to nudge the last 25%

This shouldn't affect projects indirectly depending on wasm-bindgen because cargo passes `--cap-lints=allow` when building dependencies.

The motivation is that the ecosystem has mostly taken up the versions of wasm-bindgen that are compatible in general, but ~25% or so of recent downloads remain on lower versions. However, this change might still be unnecessarily disruptive. I mostly propose it as a discussion point.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 30, 2024
…kingjubilee

Rollup of 8 pull requests

Successful merges:

 - rust-lang#126183 (Separate core search logic with search ui)
 - rust-lang#129366 (linker: Synchronize native library search in rustc and linker)
 - rust-lang#129403 (Ban non-array SIMD)
 - rust-lang#129527 (Don't use `TyKind` in a lint)
 - rust-lang#129534 (Deny `wasm_c_abi` lint to nudge the last 25%)
 - rust-lang#129640 (Re-enable android tests/benches in alloc/core)
 - rust-lang#129731 (Allow running `./x.py test compiler`)
 - rust-lang#129754 (wasi: Fix sleeping for `Duration::MAX`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 30, 2024
…kingjubilee

Rollup of 8 pull requests

Successful merges:

 - rust-lang#126183 (Separate core search logic with search ui)
 - rust-lang#129366 (linker: Synchronize native library search in rustc and linker)
 - rust-lang#129403 (Ban non-array SIMD)
 - rust-lang#129527 (Don't use `TyKind` in a lint)
 - rust-lang#129534 (Deny `wasm_c_abi` lint to nudge the last 25%)
 - rust-lang#129640 (Re-enable android tests/benches in alloc/core)
 - rust-lang#129731 (Allow running `./x.py test compiler`)
 - rust-lang#129754 (wasi: Fix sleeping for `Duration::MAX`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 30, 2024
…kingjubilee

Rollup of 8 pull requests

Successful merges:

 - rust-lang#126183 (Separate core search logic with search ui)
 - rust-lang#129366 (linker: Synchronize native library search in rustc and linker)
 - rust-lang#129403 (Ban non-array SIMD)
 - rust-lang#129527 (Don't use `TyKind` in a lint)
 - rust-lang#129534 (Deny `wasm_c_abi` lint to nudge the last 25%)
 - rust-lang#129640 (Re-enable android tests/benches in alloc/core)
 - rust-lang#129731 (Allow running `./x.py test compiler`)
 - rust-lang#129754 (wasi: Fix sleeping for `Duration::MAX`)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 31, 2024
…fcw-to-deny, r=daxpedda,alexcrichton

Deny `wasm_c_abi` lint to nudge the last 25%

This shouldn't affect projects indirectly depending on wasm-bindgen because cargo passes `--cap-lints=allow` when building dependencies.

The motivation is that the ecosystem has mostly taken up the versions of wasm-bindgen that are compatible in general, but ~25% or so of recent downloads remain on lower versions. However, this change might still be unnecessarily disruptive. I mostly propose it as a discussion point.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 31, 2024
…iaskrgr

Rollup of 15 pull requests

Successful merges:

 - rust-lang#120221 (Don't make statement nonterminals match pattern nonterminals)
 - rust-lang#126183 (Separate core search logic with search ui)
 - rust-lang#129123 (rustdoc-json: Add test for `Self` type)
 - rust-lang#129366 (linker: Synchronize native library search in rustc and linker)
 - rust-lang#129527 (Don't use `TyKind` in a lint)
 - rust-lang#129534 (Deny `wasm_c_abi` lint to nudge the last 25%)
 - rust-lang#129640 (Re-enable android tests/benches in alloc/core)
 - rust-lang#129642 (Bump backtrace to 0.3.74~ish)
 - rust-lang#129675 (allow BufReader::peek to be called on unsized types)
 - rust-lang#129723 (Simplify some extern providers)
 - rust-lang#129724 (Remove `Option<!>` return types.)
 - rust-lang#129725 (Stop using `ty::GenericPredicates` for non-predicates_of queries)
 - rust-lang#129731 (Allow running `./x.py test compiler`)
 - rust-lang#129751 (interpret/visitor: make memory order iteration slightly more efficient)
 - rust-lang#129754 (wasi: Fix sleeping for `Duration::MAX`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8f35a4f into rust-lang:master Aug 31, 2024
6 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 31, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 31, 2024
Rollup merge of rust-lang#129534 - workingjubilee:ratchet-wasm-c-abi-fcw-to-deny, r=daxpedda,alexcrichton

Deny `wasm_c_abi` lint to nudge the last 25%

This shouldn't affect projects indirectly depending on wasm-bindgen because cargo passes `--cap-lints=allow` when building dependencies.

The motivation is that the ecosystem has mostly taken up the versions of wasm-bindgen that are compatible in general, but ~25% or so of recent downloads remain on lower versions. However, this change might still be unnecessarily disruptive. I mostly propose it as a discussion point.
@Mark-Simulacrum Mark-Simulacrum added the relnotes Marks issues that should be documented in the release notes of the next release. label Oct 2, 2024
@workingjubilee workingjubilee deleted the ratchet-wasm-c-abi-fcw-to-deny branch October 2, 2024 04:12
@lqd
Copy link
Member

lqd commented Oct 2, 2024

This shouldn't affect projects indirectly depending on wasm-bindgen because cargo passes --cap-lints=allow when building dependencies.

IIRC this lint opts out of this by being a "report in deps" FCW, right?

@workingjubilee
Copy link
Member Author

hm. perhaps I misunderstood something then about the interactions here?

@daxpedda
Copy link
Contributor

daxpedda commented Oct 3, 2024

I played around with this a bit, but I couldn't actually see any difference between deny and warning no matter if the dependency is direct or indirect, they all end up as FCW warnings. But compiling old versions of wasm-bindgen directly does trigger a deny lint.

So this change only affects compiling wasm-bindgen directly, but not as a dependency, ergo unfortunately this PR doesn't do anything practically useful :/

I assume this needs a change in Cargo as well to work as intended?
https://github.com/rust-lang/cargo/blob/8725e78484d62c4dd0ef22bb64fb22e4af2f4a1c/src/cargo/core/compiler/future_incompat.rs#L421

@workingjubilee
Copy link
Member Author

...well, at least it doesn't break anyone! 🫠

@bjorn3
Copy link
Member

bjorn3 commented Oct 3, 2024

Isn't the code that has the ABI incompatibility produced by macro expansion of #[wasm_bindgen] inside the user crate? So it would warn for every user of an old version of wasm_bindgen, but be silenced when the user of wasm_bindgen is itself a non-local dependency.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 25, 2025
… r=workingjubilee

Make the wasm_c_abi future compat warning a hard error

This is the next step in getting rid of the broken C abi for wasm32-unknown-unknown.

The lint was made deny-by-default in rust-lang#129534 3 months ago. This still keeps the `-Zwasm-c-abi` flag set to `legacy` by default. It will be flipped in a future PR.

cc rust-lang#122532
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 26, 2025
… r=workingjubilee

Make the wasm_c_abi future compat warning a hard error

This is the next step in getting rid of the broken C abi for wasm32-unknown-unknown.

The lint was made deny-by-default in rust-lang#129534 3 months ago. This still keeps the `-Zwasm-c-abi` flag set to `legacy` by default. It will be flipped in a future PR.

cc rust-lang#122532
jhpratt added a commit to jhpratt/rust that referenced this pull request Jan 26, 2025
… r=workingjubilee

Make the wasm_c_abi future compat warning a hard error

This is the next step in getting rid of the broken C abi for wasm32-unknown-unknown.

The lint was made deny-by-default in rust-lang#129534 3 months ago. This still keeps the `-Zwasm-c-abi` flag set to `legacy` by default. It will be flipped in a future PR.

cc rust-lang#122532
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 26, 2025
Rollup merge of rust-lang#133951 - bjorn3:wasm_c_abi_lint_hard_error, r=workingjubilee

Make the wasm_c_abi future compat warning a hard error

This is the next step in getting rid of the broken C abi for wasm32-unknown-unknown.

The lint was made deny-by-default in rust-lang#129534 3 months ago. This still keeps the `-Zwasm-c-abi` flag set to `legacy` by default. It will be flipped in a future PR.

cc rust-lang#122532
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants