-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Constier maybe uninit #79621
Constier maybe uninit #79621
Changes from 2 commits
8bd80e2
91772c3
1ef5dbe
f311db1
4f9fd2a
d366ed2
7bd754c
9476241
4255a5a
3282b54
345f230
d0a1e40
bdda98a
69ab0bc
1749359
0775271
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -407,6 +407,30 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { | |
sym::transmute => { | ||
self.copy_op_transmute(args[0], dest)?; | ||
} | ||
sym::assert_inhabited | sym::assert_zero_valid | sym::assert_uninit_valid => { | ||
let ty = instance.substs.type_at(0); | ||
let layout = self.layout_of(ty)?; | ||
|
||
if layout.abi.is_uninhabited() { | ||
throw_ub_format!("attempted to instantiate uninhabited type `{}`", ty); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is wrong. It is not UB to fail an assertion. That is the entire point of this check, to avoid UB and replace it by a well-defined assertion. |
||
} | ||
if intrinsic_name == sym::assert_zero_valid | ||
&& !layout.might_permit_raw_init(self, /*zero:*/ true).unwrap() | ||
{ | ||
throw_ub_format!( | ||
"attempted to zero-initialize type `{}`, which is invalid", | ||
ty | ||
); | ||
} | ||
if intrinsic_name == sym::assert_uninit_valid | ||
&& !layout.might_permit_raw_init(self, /*zero:*/ false).unwrap() | ||
{ | ||
throw_ub_format!( | ||
"attempted to leave type `{}` uninitialized, which is invalid", | ||
ty | ||
); | ||
} | ||
} | ||
sym::simd_insert => { | ||
let index = u64::from(self.read_scalar(args[1])?.to_u32()?); | ||
let elem = args[2]; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -815,6 +815,7 @@ extern "rust-intrinsic" { | |
/// This will statically either panic, or do nothing. | ||
/// | ||
/// This intrinsic does not have a stable counterpart. | ||
#[rustc_const_unstable(feature = "const_maybe_assume_init", issue = "none")] | ||
usbalbin marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the feature gate name... we do not intend to ever stabilize these intrinsics. @oli-obk what is the plan for that case? Usually perma-unstable intrinsics just do not have a feature gate name. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we don't stabilize the intrinsics, but we will "stabilize their constness", thus allowing them to be used in libcore/libstd from stable There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, so even if it is const-stable it cannot be used outside libstd? Curious. Sounds like we want a feature gate collecting the three intrinsics then. Something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, does that mean you can have a type that's There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I have removed the constification of the other two intrinsics (see comments). Does the name There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Either is fine, I'd say. |
||
pub fn assert_inhabited<T>(); | ||
|
||
/// A guard for unsafe functions that cannot ever be executed if `T` does not permit | ||
|
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.
@oli-obk how will such aborting intrinsics interact with function memoization?
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.
In particular with #75390 I am wondering if we'll get decent error messages here.
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.
that works fine, it will cause the const eval queries to return an error, similar to how the memoization of
size_of
can error if the type is not representable on the platform