-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Remove the insta-stable cfg(wasm)
#83981
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
I thought we were going to add a feature instead of removing this |
I haven't checked in terms of this actually reverting the change, but in terms of the specific code patch here seems OK; r=me. I think we should remove this support for now, and then potentially re-add with a feature gate -- it looks like the code is minimal, and I'd rather not accidentally leave it in to hit stable :) |
This comment has been minimized.
This comment has been minimized.
That was what I thought as well, before I realized that adding I do encourage you to continue the discussion and propose to add something like this as a separate changeset. Sorry about that. |
The addition of `cfg(wasm)` was an oversight on my end that has a number of downsides: * It was introduced as an insta-stable addition, forgoing the usual staging mechanism we use for potentially far-reaching changes; * It is a breaking change for people who are using `--cfg wasm` either directly or via cargo for other purposes; * It is not entirely clear if a bare `wasm` cfg is a right option or whether `wasm` family of targets are special enough to warrant special-casing these targets specifically. As for the last point, there appears to be a fair amount of support for reducing the boilerplate in specifying architectures from the same family, while ignoring their pointer width. The suggested way forward would be to propose such a change as a separate RFC as it is potentially a quite contentious addition.
0c06c25
to
54dc7ce
Compare
@bors r+ |
📌 Commit 54dc7ce has been approved by |
☀️ Test successful - checks-actions |
The addition of
cfg(wasm)
was an oversight on my end that turns out to have a numberof downsides:
staging mechanism we use for potentially far-reaching changes;
--cfg wasm
eitherdirectly or via cargo for other purposes;
wasm
cfg is a right option orwhether
wasm
family of targets are special enough to warrantspecial-casing these targets specifically.
As for the last point, there appears to be a fair amount of support for
reducing the boilerplate in specifying architectures from the same
family, while ignoring their pointer width. The suggested way forward
would be to propose such a change as a separate RFC as it is potentially
a quite contentious addition.
cc #83879 @devsnek