-
Notifications
You must be signed in to change notification settings - Fork 13.8k
rename rustc_legacy_const_generics to make it more clear that it is (soft) deprecated #146577
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
base: master
Are you sure you want to change the base?
rename rustc_legacy_const_generics to make it more clear that it is (soft) deprecated #146577
Conversation
Some changes occurred in compiler/rustc_passes/src/check_attr.rs
cc @Amanieu, @folkertdev, @sayantn |
EncodeCrossCrate::No, ), | ||
rustc_attr!( | ||
rustc_legacy_const_generics, Normal, template!(List: &["N"]), ErrorFollowing, | ||
rustc_deprecated_legacy_const_generics, Normal, template!(List: &["N"]), ErrorFollowing, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it'd be worth adding a comment here to the effect that this should not be newly applied anywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I doubt anyone who might be tempted to apply it would see a comment deep inside the rustc sources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha, just an hour ago I was thinking to name it legacy_const_generics_do_not_use
just to avoid any and all ambiguity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I considered that name but it seemed a bit too cheesy ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also considered rustc_legacy_const_generics_deprecated
(instead of rustc_deprecated_legacy_const_generics
), not sure which order makes more sense.
This comment has been minimized.
This comment has been minimized.
895407f
to
6646334
Compare
r=me on the name change legacy const generics arguments have weird restriction and I also really feel like if we actually want better support for const generics, that should be an actually stable feature instead of some widely used std internal hack use std::arch::x86_64::_mm_aeskeygenassist_si128;
fn main() {
_mm_aeskeygenassist_si128(return, {
struct Foo;
1
});
}
I feel like ideally we have an explicit allow-list for all functions marked with that attribute and need people to add any new functions to it manually. Idk how much effort that is however. |
I don't think that's worth it, the compiler has to trust the standard library to not misuse the unstable features that exist solely to support the standard library. Ofc reviewers forget things so we should support them where we can, but an allowlist seems pretty heavy-handed. IOW, the attribute is the allowlist. |
This comment has been minimized.
This comment has been minimized.
Looks like there wasn't actually a team decision to deprecate this (rust-lang/stdarch#1916 (comment)), so let's hold off on this PR for now. |
6646334
to
760afcc
Compare
The job Click to see the possible cause of the failure (guessed by this bot)
|
Nominated this question as part of: |
☔ The latest upstream changes (presumably #146700) made this pull request unmergeable. Please resolve the merge conflicts. |
r? types or @oli-obk, you know best how deprecated this really is
Context: rust-lang/stdarch#1916