-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Stability oddity with const intrinsics #80707
Stability oddity with const intrinsics #80707
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
0626088
to
7da07bf
Compare
r? @RalfJung |
The job Click to see the possible cause of the failure (guessed by this bot)
|
14e2c38
to
074a69b
Compare
Oh, that's interesting. For normal intrinsics this makes sense (when declared inside a |
So... should I try to fix it in this PR? |
I think it should be fixed... and that makes the tests added in this PR a bit odd since they test behavior that I consider buggy. But I'd also be okay with opening an issue and having a comment in the test saying that this is testing buggy behavior and should be adjusted once there's a fix. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
library/core/src/intrinsics.rs
Outdated
@@ -1849,6 +1849,7 @@ pub(crate) fn is_nonoverlapping<T>(src: *const T, dst: *const T, count: usize) - | |||
#[inline] | |||
pub const unsafe fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: usize) { | |||
extern "rust-intrinsic" { | |||
#[rustc_const_unstable(feature = "const_intrinsic_copy", issue = "none")] |
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.
Looks like we have another case of this a few lines down, around line 1940, according to CI.
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.
yea, I had to rebase first ^^
r=me with CI fixed and the commits squashed a bit. |
84e414c
to
2a412ae
Compare
This comment has been minimized.
This comment has been minimized.
2a412ae
to
5bac1c9
Compare
@bors r=RalfJung |
📌 Commit 5bac1c9 has been approved by |
…sics, r=RalfJung Stability oddity with const intrinsics cc `@RalfJung` In rust-lang#80699 (comment) `@usbalbin` realized we accepted some intrinsics as `const` without a `#[rustc_const_(un)stable]` attribute. I did some digging, and that example works because intrinsics inherit their stability from their parents... including `#[rustc_const_(un)stable]` attributes. While we may want to fix that (not sure, wasn't there just a MCPed PR that caused this on purpose?), we definitely want tests for it, thus this PR adding tests and some fun tracing statements.
☀️ Test successful - checks-actions |
cc @RalfJung
In #80699 (comment) @usbalbin realized we accepted some intrinsics as
const
without a#[rustc_const_(un)stable]
attribute. I did some digging, and that example works because intrinsics inherit their stability from their parents... including#[rustc_const_(un)stable]
attributes. While we may want to fix that (not sure, wasn't there just a MCPed PR that caused this on purpose?), we definitely want tests for it, thus this PR adding tests and some fun tracing statements.