-
Notifications
You must be signed in to change notification settings - Fork 13.4k
A way forward for pointer equality in const eval #73398
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
Changes from 5 commits
9245ba8
e09b620
84f1d73
9e88b48
53686b9
98e97a4
e465b22
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 |
---|---|---|
|
@@ -295,6 +295,72 @@ impl<T: ?Sized> *const T { | |
intrinsics::ptr_offset_from(self, origin) | ||
} | ||
|
||
/// Returns whether two pointers are guaranteed equal. | ||
/// | ||
/// At runtime this function behaves like `self == other`. | ||
/// However, in some contexts (e.g., compile-time evaluation), | ||
/// it is not always possible to determine equality of two pointers, so this function may | ||
/// spuriously return `false` for pointers that later actually turn out to be equal. | ||
/// But when it returns `true`, the pointers are guaranteed to be equal. | ||
/// | ||
/// This function is the mirror of [`guaranteed_ne`], but not its inverse. There are pointer | ||
/// comparisons for which both functions return `false`. | ||
/// | ||
/// [`guaranteed_ne`]: #method.guaranteed_ne | ||
/// | ||
/// The return value may change depending on the compiler version and unsafe code may not | ||
/// rely on the result of this function for soundness. It is suggested to only use this function | ||
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. Can unsafe code rely on a return value of 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. Yes, it can. The comment says
I thought that was clear enough? |
||
/// for performance optimizations where spurious `false` return values by this function do not | ||
/// affect the outcome, but just the performance. | ||
/// The consequences of using this method to make runtime and compile-time code behave | ||
/// differently have not been explored. This method should not be used to introduce such | ||
/// differences, and it should also not be stabilized before we have a better understanding | ||
/// of this issue. | ||
/// ``` | ||
#[unstable(feature = "const_raw_ptr_comparison", issue = "53020")] | ||
#[rustc_const_unstable(feature = "const_raw_ptr_comparison", issue = "53020")] | ||
#[inline] | ||
#[cfg(not(bootstrap))] | ||
pub const fn guaranteed_eq(self, other: *const T) -> bool | ||
where | ||
T: Sized, | ||
{ | ||
intrinsics::ptr_guaranteed_eq(self, other) | ||
} | ||
|
||
/// Returns whether two pointers are guaranteed not equal. | ||
oli-obk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// | ||
/// At runtime this function behaves like `self != other`. | ||
/// However, in some contexts (e.g., compile-time evaluation), | ||
/// it is not always possible to determine the inequality of two pointers, so this function may | ||
/// spuriously return `false` for pointers that later actually turn out to be inequal. | ||
/// But when it returns `true`, the pointers are guaranteed to be inequal. | ||
/// | ||
/// This function is the mirror of [`guaranteed_eq`], but not its inverse. There are pointer | ||
/// comparisons for which both functions return `false`. | ||
/// | ||
/// [`guaranteed_eq`]: #method.guaranteed_eq | ||
/// | ||
/// The return value may change depending on the compiler version and unsafe code may not | ||
/// rely on the result of this function for soundness. It is suggested to only use this function | ||
/// for performance optimizations where spurious `false` return values by this function do not | ||
/// affect the outcome, but just the performance. | ||
/// The consequences of using this method to make runtime and compile-time code behave | ||
/// differently have not been explored. This method should not be used to introduce such | ||
/// differences, and it should also not be stabilized before we have a better understanding | ||
/// of this issue. | ||
/// ``` | ||
#[unstable(feature = "const_raw_ptr_comparison", issue = "53020")] | ||
#[rustc_const_unstable(feature = "const_raw_ptr_comparison", issue = "53020")] | ||
#[inline] | ||
#[cfg(not(bootstrap))] | ||
pub const fn guaranteed_ne(self, other: *const T) -> bool | ||
where | ||
T: Sized, | ||
{ | ||
intrinsics::ptr_guaranteed_ne(self, other) | ||
} | ||
|
||
/// Calculates the distance between two pointers. The returned value is in | ||
/// units of T: the distance in bytes is divided by `mem::size_of::<T>()`. | ||
/// | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -12,7 +12,7 @@ use log::debug; | |||||||||||||||||||||||||||||||
use rustc_ast::ast; | ||||||||||||||||||||||||||||||||
use rustc_codegen_ssa::base::{compare_simd_types, to_immediate, wants_msvc_seh}; | ||||||||||||||||||||||||||||||||
use rustc_codegen_ssa::common::span_invalid_monomorphization_error; | ||||||||||||||||||||||||||||||||
use rustc_codegen_ssa::common::TypeKind; | ||||||||||||||||||||||||||||||||
use rustc_codegen_ssa::common::{IntPredicate, TypeKind}; | ||||||||||||||||||||||||||||||||
use rustc_codegen_ssa::glue; | ||||||||||||||||||||||||||||||||
use rustc_codegen_ssa::mir::operand::{OperandRef, OperandValue}; | ||||||||||||||||||||||||||||||||
use rustc_codegen_ssa::mir::place::PlaceRef; | ||||||||||||||||||||||||||||||||
|
@@ -731,6 +731,18 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { | |||||||||||||||||||||||||||||||
return; | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
"ptr_guaranteed_eq" | "ptr_guaranteed_ne" => { | ||||||||||||||||||||||||||||||||
let a = args[0].immediate(); | ||||||||||||||||||||||||||||||||
let b = args[1].immediate(); | ||||||||||||||||||||||||||||||||
let a = self.ptrtoint(a, self.type_isize()); | ||||||||||||||||||||||||||||||||
let b = self.ptrtoint(b, self.type_isize()); | ||||||||||||||||||||||||||||||||
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. Is there a non-obvious reason you 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. I didn't know that. I basically grabbed this off rust/src/librustc_codegen_llvm/intrinsic.rs Lines 734 to 748 in 38bd83d
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.
It does. LLVM supports comparison of pointer types but not subtraction. 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. So confusing. There's probably a good reason for that, but still... So many inconsistencies. 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. FWIW, there's a plan to add a (There is no pointer addition, either. Instead there is |
||||||||||||||||||||||||||||||||
if name == "ptr_guaranteed_eq" { | ||||||||||||||||||||||||||||||||
self.icmp(IntPredicate::IntEQ, a, b) | ||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||
self.icmp(IntPredicate::IntNE, a, b) | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
"ptr_offset_from" => { | ||||||||||||||||||||||||||||||||
let ty = substs.type_at(0); | ||||||||||||||||||||||||||||||||
let pointee_size = self.size_of(ty); | ||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -291,6 +291,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { | |
let offset_ptr = ptr.ptr_wrapping_signed_offset(offset_bytes, self); | ||
self.write_scalar(offset_ptr, dest)?; | ||
} | ||
sym::ptr_guaranteed_eq | sym::ptr_guaranteed_ne => { | ||
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 just realized that this implementation will also be used by Miri, which is a bug. Can we somehow make it be used only during CTCE? 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. doesn't miri fall back to this only if it has no own impl? So we can just give miri an impl that behaves differently. 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. Miri prefers the CTFE impls, to make sure we test those properly. I am working on a PR that adds these to Miri in a way that it prefers its own impls. 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. Here it is: rust-lang/miri#1459 |
||
// FIXME: return `true` for at least some comparisons where we can reliably | ||
// determine the result of runtime (in)equality tests at compile-time. | ||
self.write_scalar(Scalar::from_bool(false), dest)?; | ||
} | ||
sym::ptr_offset_from => { | ||
let a = self.read_immediate(args[0])?.to_scalar()?; | ||
let b = self.read_immediate(args[1])?.to_scalar()?; | ||
|
Uh oh!
There was an error while loading. Please reload this page.