Ignore zero-sized types in wasm future-compat warning #139498
Conversation
|
r? @wesleywiser rustbot has assigned @wesleywiser. Use |
rustbot
added
S-waiting-on-review
alexcrichton
added
the
beta-nominated
|
I've additionally tagged this for a beta backport since the false positive is currently in beta too. |
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
This commit fixes a false positive of the warning triggered for rust-lang#138762 and the fix is to codify that zero-sized types are "safe" in both the old and new ABIs.
alexcrichton
force-pushed
the
wasm-zst-safe
branch
from
88d8fd2 to
f9091e2
Compare
|
@bors: r=wesleywiser |
bors
added
S-waiting-on-bors
| // Zero-sized types are dropped in both ABIs, so they're safe | ||
| if arg.layout.is_zst() { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
That's not actually a stable guarantee, nobody should rely on this...
There was a problem hiding this comment.
I think that this at least respects the status quo where the purpose of the wasm_c_abi warning is "this will change" and for ZSTs behavior isn't changing.
Otherwise though for relying on ZSTs, I know wasm-bindgen definitely does rely on ZSTs getting dropped at the ABI level, and AFAIK it's quite difficult to remove the reliance. I can't speak for other projects though.
There was a problem hiding this comment.
for ZSTs behavior isn't changing.
Yeah, that's a fair point, we don't have to warn about them.
But it seems quite concerning that wasm-bindgen would rely on unstable undocumented ABI guarantees without (apparently) any effort to make those things into stable guarantees. We don't guarantee that ZST are skipped in any form for any ABI, and I'm not even aware of an issue someone opened asking for this as a guarantee.
|
Unsettled discussions re. lack of guarantee |
bors
added
S-waiting-on-author
|
@bors r=wesleywiser Silencing the warning for ZST is fine, relying on their being skipped does not become any less correct via this change. |
bors
removed
the
S-waiting-on-author
bors
added
the
S-waiting-on-bors
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#139351 (Autodiff batching2) - rust-lang#139483 (f*::NAN: guarantee that this is a quiet NaN) - rust-lang#139498 (Ignore zero-sized types in wasm future-compat warning) - rust-lang#139967 (Introduce and use specialized `//@ ignore-auxiliary` for test support files instead of using `//@ ignore-test`) - rust-lang#139969 (update libc) - rust-lang#139971 (Make C string merging test work on MIPS) - rust-lang#139974 (Change `InterpCx::instantiate*` function visibility to pub) - rust-lang#139977 (Fix drop handling in `hint::select_unpredictable`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#139498 - alexcrichton:wasm-zst-safe, r=wesleywiser Ignore zero-sized types in wasm future-compat warning This commit fixes a false positive of the warning triggered for rust-lang#138762 and the fix is to codify that zero-sized types are "safe" in both the old and new ABIs.
This commit fixes a false positive of the warning triggered for #138762 and the fix is to codify that zero-sized types are "safe" in both the old and new ABIs.