Make the wasm_c_abi future compat warning a hard error by bjorn3 · Pull Request #133951 · rust-lang/rust (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

bjorn3

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 #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 #122532

@bjorn3 bjorn3 added O-wasm

Target: WASM (WebAssembly), http://webassembly.org/

A-ABI

Area: Concerning the application binary interface (ABI)

labels

Dec 6, 2024

@lcnr

@rust-log-analyzer

This comment has been minimized.

daxpedda

Choose a reason for hiding this comment

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

I should note that the changes in #129534 did not actually start showing an error to users, it was still a warning by default.
See #129534 (comment)

@rustbot rustbot added the T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.

label

Dec 6, 2024

@workingjubilee

Technically we are breaking like ~85 versions of a foundation ecosystem crate, so yeah, an FCP might be warranted, but also technically we're just fixing a bug.

If we do this, we should make sure we only do one FCP between this and #133952 though since they're the same decision: "We're killing this dead, yeah?"

alexcrichton

@workingjubilee

Nominating for T-compiler:

We already accepted the MCP to phase out this behavior, but this is the "hard break" edge. If an FCP is desired before proceeding, now is the time to decide. We are technically breaking a crate on future versions of Rust. However it is the crate and target's maintainer wish that we do so, and this is the solution to a 4 year old issue:

We have heard no strong objection and this has been discussed for 4 years at this point, so any box-ticking formality is exactly that: a formality.

@rustbot label: +I-compiler-nominated

Also cc @daxpedda

@apiraino

Discussed in T-compiler triage on Zulip.

Even if it's just a sign-off, we want to loop in T-lang, seems the usual procedure for them is to FCP such changes.

@rustbot label -I-compiler-nominated +I-lang-easy-decision

@workingjubilee

To summarize even more summarily for T-lang:

extern "C" fn on wasm32 targets has used a creative ABI that emerged because Rust and C toolchain development were more historically contemporary to each other, as opposed to the usual "first C, then 10~40 years later, Rust". Unfortunately, wasm-bindgen needed something for a stable-ish wasm ABI to talk to JavaScript. A flawed lowering was written. It was not immediately fixed because fixing it would break wasm-bindgen. For a while the wasm32-unknown-emscripten target was an acceptable workaround, and in a nascent ecosystem which might have further growing pains, the next move was not obvious.

Things have settled and it has become clear that Rust should align the ABI of extern "C" fn with that used by C toolchains for other wasm32 targets.

Thanks to @alexcrichton and @daxpedda we have implemented a relatively-smooth migration path for wasm-bindgen. There should be no meaningful concerns going forward, and this will make code that wasn't compatible with C code be compatible again, according to the conventional understanding of extern "C" fn.

@joshtriplett

If I understand correctly, people who want to use the new interoperable ABI between C and Rust on WebAssembly will need to either use some flavor of wasi target or use some hypothetical new -none target that we don't yet have?

Or is the migration plan here to wait some amount of time with this as a hard error and then change it to the new ABI?

@scottmcm

I might be misunderstanding something here, but I would have expected a change in tests/ui for this? What's the code that used to emit "future compat" and now would, or that no longer mentions the "because this lint is on by default" because it's a hard error not a lint?

@tmandry

We discussed in today's lang team meeting and were confused what exactly we are rubber stamping here. Specifically, we wanted to know:

Does this break any use of extern "C" on wasm32-unknown-unknown target? What action is the user expected to take when we break them?

Also echoing @scottmcm's point that a test change would be ideal.

@workingjubilee

If I understand correctly, people who want to use the new interoperable ABI between C and Rust on WebAssembly will need to either use some flavor of wasi target or use some hypothetical new -none target that we don't yet have?

That is the precise opposite of the situation.

@joshtriplett

@workingjubilee came to the @rust-lang/lang meeting and explained the situation in great detail.

It sounds like this hard error will happen for people using old wasm-bindgen, and people using new wasm-bindgen will have things Just Work.

@traviscross

Thanks to @workingjubilee for joining the call and clarifying the situation for us.

The current situation is that extern "C" with wasm32-unknown-unknown works with old versions of wasm-bindgen, but it's broken and we lint against this. People who want this to work correctly today with new versions of wasm-bindgen have to pass the -Zwasm-c-abi flag. What this change does is to make that -Zwasm-c-abi flag the default so that users doing extern "C" with new versions of wasm-bindgen will find things to just work.

Since there would otherwise be subtle breakage for people using old versions of wasm-bindgen, this includes a heuristic to hopefully provide a hard error in those cases.

On that basis...

@rfcbot fcp merge

@bjorn3

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

@bjorn3 @alexcrichton

Co-Authored-By: Alex Crichton alex@alexcrichton.com

@workingjubilee

Thank you everyone!

@bors r+

@bors

📌 Commit 49c3aaa has been approved by workingjubilee

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

Jan 24, 2025

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jan 25, 2025

@bors

Rollup of 7 pull requests

Successful merges:

Failed merges:

r? @ghost @rustbot modify labels: rollup

try-job: aarch64-apple try-job: i686-mingw try-job: x86_64-gnu-llvm-19-3

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Jan 25, 2025

@matthiaskrgr

… 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

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jan 25, 2025

@bors

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jan 25, 2025

@bors

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Jan 26, 2025

@matthiaskrgr

… 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

bors added a commit to rust-lang-ci/rust that referenced this pull request

Jan 26, 2025

@bors

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Jan 26, 2025

@rust-timer

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

@bjorn3 bjorn3 deleted the wasm_c_abi_lint_hard_error branch

January 26, 2025 09:52

@Mark-Simulacrum Mark-Simulacrum added the relnotes

Marks issues that should be documented in the release notes of the next release.

label

Mar 11, 2025

RalfJung

@@ -143,7 +143,6 @@ declare_lint_pass! {
UNUSED_VARIABLES,
USELESS_DEPRECATED,
WARNINGS,
WASM_C_ABI,

Choose a reason for hiding this comment

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

FYI whenever removing something from this list, it must be added to the removed lint list. There is a comment above the list explaining that.

//! When removing a lint, make sure to also add a call to `register_removed` in
//! compiler/rustc_lint/src/lib.rs.

Choose a reason for hiding this comment

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

My bad. In any case #138601 added another lint that is more thorough than the lint that was removed in this PR.

wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request

Apr 9, 2025

@he32

Upstream changes relative to 1.85.1:

Version 1.86.0 (2025-04-03)

Language

Compiler

Platform Support

Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support.

Libraries

Stabilized APIs

These APIs are now stable in const contexts:

Cargo

Rustdoc

Compatibility Notes

Internal Changes

These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools.

tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request

May 10, 2025

@tmeijn

This MR contains the following updates:

Package Update Change
rust minor 1.85.1 -> 1.86.0

MR created with the help of el-capitano/tools/renovate-bot.

Proposed changes to behavior should be submitted there as MRs.


Release Notes

rust-lang/rust (rust)

v1.86.0

Compare Source

==========================

Language

Compiler

Platform Support

Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support.

Libraries

Stabilized APIs

These APIs are now stable in const contexts:

Cargo

Rustdoc

Compatibility Notes

Internal Changes

These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools.


Configuration

📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

Rebasing: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 Ignore: Close this MR and you won't be reminded about this update again.



This MR has been generated by Renovate Bot.

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request

Jun 16, 2025

@he32

Pkgsrc changes:

Upstream changes:

Version 1.86.0 (2025-04-03)

Language

Compiler

Platform Support

Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support.

Libraries

Stabilized APIs

These APIs are now stable in const contexts:

Cargo

Rustdoc

Compatibility Notes

Internal Changes

These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools.

Labels

A-ABI

Area: Concerning the application binary interface (ABI)

disposition-merge

This issue / PR is in PFCP or FCP with a disposition to merge it.

finished-final-comment-period

The final comment period is finished for this PR / Issue.

I-lang-easy-decision

Issue: The decision needed by the team is conjectured to be easy; this does not imply nomination

O-wasm

Target: WASM (WebAssembly), http://webassembly.org/

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-lang

Relevant to the language team