Make the wasm_c_abi future compat warning a hard error #133951

bjorn3 · 2024-12-06T08:53:59Z

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

lcnr · 2024-12-06T09:03:47Z

given #129534

r? @workingjubilee @alexcrichton

does this need a compiler FCP?

daxpedda

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)

workingjubilee · 2024-12-06T09:37:24Z

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?"

workingjubilee · 2024-12-06T22:45:27Z

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:

WebAssembly ABI mismatch between clang and rust #71871

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 · 2024-12-12T15:27:24Z

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 · 2024-12-12T21:04:11Z

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 · 2025-01-08T16:21:12Z

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 · 2025-01-08T16:25:11Z

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 · 2025-01-08T16:26:44Z

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 · 2025-01-08T16:30:20Z

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 · 2025-01-08T16:44:40Z

@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 · 2025-01-08T16:44:58Z

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

rfcbot · 2025-01-18T16:52:19Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

traviscross · 2025-01-19T05:04:09Z

@rustbot labels -S-waiting-on-team

With FCP complete, this is OK to go forward as far as lang is concerned.

bjorn3 · 2025-01-20T13:56:28Z

@rustbot ready

tests/ui/wasm/wasm-bindgen-broken-error.rs

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

Co-Authored-By: Alex Crichton <[email protected]>

workingjubilee · 2025-01-24T20:22:50Z

Thank you everyone!

@bors r+

bors · 2025-01-24T20:22:54Z

📌 Commit 49c3aaa has been approved by workingjubilee

It is now in the queue for this repository.

Rollup of 7 pull requests Successful merges: - rust-lang#130808 (Fix linking for symbols starting with ? on i686-pc-windows-msvc) - rust-lang#133138 (Target modifiers (special marked options) are recorded in metainfo) - rust-lang#133154 (Reword resolve errors caused by likely missing crate in dep tree) - rust-lang#135707 (Shorten linker output even more when `--verbose` is not present) - rust-lang#135764 (Fix tests on LLVM 20) - rust-lang#135785 (use `PassMode::Direct` for vector types on `s390x`) - rust-lang#135818 (tests: Port `translation` to rmake.rs) Failed merges: - rust-lang#133951 (Make the wasm_c_abi future compat warning a hard error) r? `@ghost` `@rustbot` modify labels: rollup try-job: aarch64-apple try-job: i686-mingw try-job: x86_64-gnu-llvm-19-3

… 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

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#133631 (Support QNX 7.1 with `io-sock`+libstd and QNX 8.0 (`no_std` only)) - rust-lang#133951 (Make the wasm_c_abi future compat warning a hard error) - rust-lang#134283 (fix(libtest): Deprecate '--logfile') - rust-lang#134679 (Windows: remove readonly files) - rust-lang#135635 (Move `std::io::pipe` code into its own file) - rust-lang#135842 (TRPL: more backward-compatible Edition changes) - rust-lang#135953 (ci.py: check the return code in `run-local`) - rust-lang#136031 (Expand polonius MIR dump) r? `@ghost` `@rustbot` modify labels: rollup

… 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

Rollup of 7 pull requests Successful merges: - rust-lang#133951 (Make the wasm_c_abi future compat warning a hard error) - rust-lang#134283 (fix(libtest): Deprecate '--logfile') - rust-lang#135785 (use `PassMode::Direct` for vector types on `s390x`) - rust-lang#135948 (Update emscripten std tests) - rust-lang#135951 (Use `fmt::from_fn` in more places in the compiler) - rust-lang#136031 (Expand polonius MIR dump) - rust-lang#136032 (Account for mutable borrow in argument suggestion) Failed merges: - rust-lang#135635 (Move `std::io::pipe` code into its own file) r? `@ghost` `@rustbot` modify labels: rollup

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 added O-wasm Target: WASM (WebAssembly), http://webassembly.org/ A-ABI Area: Concerning the application binary interface (ABI) labels Dec 6, 2024

rustbot assigned lcnr Dec 6, 2024

rustbot assigned workingjubilee and unassigned lcnr Dec 6, 2024

This comment has been minimized.

Sign in to view

daxpedda approved these changes Dec 6, 2024

View reviewed changes

bjorn3 force-pushed the wasm_c_abi_lint_hard_error branch from 0ed0661 to e1f75dd Compare December 6, 2024 09:16

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 6, 2024

bjorn3 mentioned this pull request Dec 6, 2024

Remove wasm legacy abi #133952

Draft

bjorn3 force-pushed the wasm_c_abi_lint_hard_error branch from e1f75dd to 10a3850 Compare December 6, 2024 13:40

alexcrichton approved these changes Dec 6, 2024

View reviewed changes

rustbot added the I-compiler-nominated Nominated for discussion during a compiler team meeting. label Dec 6, 2024

jieyouxu added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Dec 12, 2024

rustbot added I-lang-easy-decision Issue: The decision needed by the team is conjectured to be easy; this does not imply nomination and removed I-compiler-nominated Nominated for discussion during a compiler team meeting. labels Dec 12, 2024

bjorn3 force-pushed the wasm_c_abi_lint_hard_error branch from 10a3850 to fde2b1b Compare December 16, 2024 12:18

compiler-errors added the I-lang-nominated Nominated for discussion during a lang team meeting. label Dec 16, 2024

workingjubilee added the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Jan 4, 2025

rfcbot added the to-announce Announce this issue on triage meeting label Jan 18, 2025

rustbot mentioned this pull request Jan 18, 2025

Tracking issue for release notes of #133951: Make the wasm_c_abi future compat warning a hard error #135691

Open

3 tasks

rustbot removed the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Jan 19, 2025

bjorn3 force-pushed the wasm_c_abi_lint_hard_error branch from e84078a to b030685 Compare January 20, 2025 13:56

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 20, 2025

Urgau reviewed Jan 22, 2025

View reviewed changes

tests/ui/wasm/wasm-bindgen-broken-error.rs Show resolved Hide resolved

bjorn3 and others added 2 commits January 23, 2025 10:22

Make the wasm_c_abi future compat warning a hard error

4d1c16c

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

Add test

49c3aaa

Co-Authored-By: Alex Crichton <[email protected]>

bjorn3 force-pushed the wasm_c_abi_lint_hard_error branch from b030685 to 49c3aaa Compare January 23, 2025 10:55

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

matthiaskrgr mentioned this pull request Jan 25, 2025

Rollup of 7 pull requests #136045

Closed

matthiaskrgr mentioned this pull request Jan 25, 2025

Rollup of 8 pull requests #136050

Closed

matthiaskrgr mentioned this pull request Jan 26, 2025

Rollup of 7 pull requests #136081

Closed

jhpratt mentioned this pull request Jan 26, 2025

Rollup of 7 pull requests #136085

Merged

bors merged commit dc202df into rust-lang:master Jan 26, 2025
6 checks passed

rustbot added this to the 1.86.0 milestone Jan 26, 2025

bjorn3 deleted the wasm_c_abi_lint_hard_error branch January 26, 2025 09:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make the wasm_c_abi future compat warning a hard error #133951

Make the wasm_c_abi future compat warning a hard error #133951

bjorn3 commented Dec 6, 2024 •

edited

Loading

lcnr commented Dec 6, 2024

This comment has been minimized.

daxpedda left a comment

workingjubilee commented Dec 6, 2024 •

edited

Loading

workingjubilee commented Dec 6, 2024

apiraino commented Dec 12, 2024

workingjubilee commented Dec 12, 2024 •

edited

Loading

joshtriplett commented Jan 8, 2025 •

edited

Loading

scottmcm commented Jan 8, 2025

tmandry commented Jan 8, 2025

workingjubilee commented Jan 8, 2025

joshtriplett commented Jan 8, 2025

traviscross commented Jan 8, 2025

rfcbot commented Jan 18, 2025

traviscross commented Jan 19, 2025

bjorn3 commented Jan 20, 2025

workingjubilee commented Jan 24, 2025

bors commented Jan 24, 2025

Make the wasm_c_abi future compat warning a hard error #133951

Make the wasm_c_abi future compat warning a hard error #133951

Conversation

bjorn3 commented Dec 6, 2024 • edited Loading

lcnr commented Dec 6, 2024

This comment has been minimized.

daxpedda left a comment

Choose a reason for hiding this comment

workingjubilee commented Dec 6, 2024 • edited Loading

workingjubilee commented Dec 6, 2024

apiraino commented Dec 12, 2024

workingjubilee commented Dec 12, 2024 • edited Loading

joshtriplett commented Jan 8, 2025 • edited Loading

scottmcm commented Jan 8, 2025

tmandry commented Jan 8, 2025

workingjubilee commented Jan 8, 2025

joshtriplett commented Jan 8, 2025

traviscross commented Jan 8, 2025

rfcbot commented Jan 18, 2025

traviscross commented Jan 19, 2025

bjorn3 commented Jan 20, 2025

workingjubilee commented Jan 24, 2025

bors commented Jan 24, 2025

bjorn3 commented Dec 6, 2024 •

edited

Loading

workingjubilee commented Dec 6, 2024 •

edited

Loading

workingjubilee commented Dec 12, 2024 •

edited

Loading

joshtriplett commented Jan 8, 2025 •

edited

Loading