Skip to content
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

Never consider raw pointer casts to be trival #113262

Merged
merged 2 commits into from Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 15 additions & 3 deletions compiler/rustc_hir_typeck/src/cast.rs
Expand Up @@ -660,9 +660,21 @@ impl<'a, 'tcx> CastCheck<'tcx> {
} else {
match self.try_coercion_cast(fcx) {
Ok(()) => {
self.trivial_cast_lint(fcx);
debug!(" -> CoercionCast");
fcx.typeck_results.borrow_mut().set_coercion_cast(self.expr.hir_id.local_id);
if self.expr_ty.is_unsafe_ptr() && self.cast_ty.is_unsafe_ptr() {
// When casting a raw pointer to another raw pointer, we cannot convert the cast into
// a coercion because the pointee types might only differ in regions, which HIR typeck
// cannot distinguish. This would cause us to erroneously discard a cast which will
// lead to a borrowck error like #113257.
// We still did a coercion above to unify inference variables for `ptr as _` casts.
// This does cause us to miss some trivial casts in the trival cast lint.
debug!(" -> PointerCast");
} else {
self.trivial_cast_lint(fcx);
debug!(" -> CoercionCast");
fcx.typeck_results
.borrow_mut()
.set_coercion_cast(self.expr.hir_id.local_id);
}
}
Err(_) => {
match self.do_check(fcx) {
Expand Down
32 changes: 13 additions & 19 deletions compiler/rustc_mir_build/src/thir/cx/expr.rs
Expand Up @@ -191,11 +191,16 @@ impl<'tcx> Cx<'tcx> {
source: self.mirror_expr(source),
cast: PointerCoercion::ArrayToPointer,
}
} else {
// check whether this is casting an enum variant discriminant
// to prevent cycles, we refer to the discriminant initializer
} else if let hir::ExprKind::Path(ref qpath) = source.kind
&& let res = self.typeck_results().qpath_res(qpath, source.hir_id)
&& let ty = self.typeck_results().node_type(source.hir_id)
&& let ty::Adt(adt_def, args) = ty.kind()
&& let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Const), variant_ctor_id) = res
{
// Check whether this is casting an enum variant discriminant.
// To prevent cycles, we refer to the discriminant initializer,
// which is always an integer and thus doesn't need to know the
// enum's layout (or its tag type) to compute it during const eval
// enum's layout (or its tag type) to compute it during const eval.
// Example:
// enum Foo {
// A,
Expand All @@ -204,21 +209,6 @@ impl<'tcx> Cx<'tcx> {
// The correct solution would be to add symbolic computations to miri,
// so we wouldn't have to compute and store the actual value

let hir::ExprKind::Path(ref qpath) = source.kind else {
return ExprKind::Cast { source: self.mirror_expr(source) };
};

let res = self.typeck_results().qpath_res(qpath, source.hir_id);
let ty = self.typeck_results().node_type(source.hir_id);
let ty::Adt(adt_def, args) = ty.kind() else {
return ExprKind::Cast { source: self.mirror_expr(source) };
};

let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Const), variant_ctor_id) = res
else {
return ExprKind::Cast { source: self.mirror_expr(source) };
};

let idx = adt_def.variant_index_with_ctor_id(variant_ctor_id);
let (discr_did, discr_offset) = adt_def.discriminant_def_for_variant(idx);

Expand Down Expand Up @@ -255,6 +245,10 @@ impl<'tcx> Cx<'tcx> {
};

ExprKind::Cast { source }
} else {
// Default to `ExprKind::Cast` for all explicit casts.
// MIR building then picks the right MIR casts based on the types.
ExprKind::Cast { source: self.mirror_expr(source) }
}
}

Expand Down
19 changes: 12 additions & 7 deletions tests/mir-opt/instsimplify/casts.redundant.InstSimplify.diff
Expand Up @@ -6,22 +6,27 @@
let mut _0: *const &u8;
let mut _2: *const &u8;
let mut _3: *const &u8;
let mut _4: *const &u8;
scope 1 (inlined generic_cast::<&u8, &u8>) {
debug x => _3;
let mut _4: *const &u8;
debug x => _4;
let mut _5: *const &u8;
}

bb0: {
StorageLive(_2);
StorageLive(_3);
_3 = _1;
StorageLive(_4);
_4 = _3;
- _2 = move _4 as *const &u8 (PtrToPtr);
+ _2 = move _4;
_4 = _1;
StorageLive(_5);
_5 = _4;
- _3 = move _5 as *const &u8 (PtrToPtr);
+ _3 = move _5;
StorageDead(_5);
StorageDead(_4);
StorageDead(_3);
- _2 = move _3 as *const &u8 (PtrToPtr);
+ _2 = move _3;
_0 = _2;
StorageDead(_3);
StorageDead(_2);
return;
}
Expand Down
15 changes: 10 additions & 5 deletions tests/mir-opt/instsimplify/casts.roundtrip.InstSimplify.diff
Expand Up @@ -4,16 +4,21 @@
fn roundtrip(_1: *const u8) -> *const u8 {
debug x => _1;
let mut _0: *const u8;
let mut _2: *mut u8;
let mut _3: *const u8;
let mut _2: *const u8;
let mut _3: *mut u8;
let mut _4: *const u8;

bb0: {
StorageLive(_2);
StorageLive(_3);
_3 = _1;
_2 = move _3 as *mut u8 (PtrToPtr);
_0 = move _2 as *const u8 (PointerCoercion(MutToConstPointer));
StorageLive(_4);
_4 = _1;
_3 = move _4 as *mut u8 (PtrToPtr);
_2 = move _3 as *const u8 (PointerCoercion(MutToConstPointer));
StorageDead(_4);
StorageDead(_3);
- _0 = move _2 as *const u8 (PtrToPtr);
+ _0 = move _2;
StorageDead(_2);
return;
}
Expand Down
6 changes: 3 additions & 3 deletions tests/mir-opt/instsimplify/casts.rs
Expand Up @@ -18,8 +18,8 @@ pub fn redundant<'a, 'b: 'a>(x: *const &'a u8) -> *const &'a u8 {
// EMIT_MIR casts.roundtrip.InstSimplify.diff
pub fn roundtrip(x: *const u8) -> *const u8 {
// CHECK-LABEL: fn roundtrip(
// CHECK: _3 = _1;
// CHECK: _2 = move _3 as *mut u8 (PtrToPtr);
// CHECK: _0 = move _2 as *const u8 (PointerCoercion(MutToConstPointer));
// CHECK: _4 = _1;
// CHECK: _3 = move _4 as *mut u8 (PtrToPtr);
// CHECK: _2 = move _3 as *const u8 (PointerCoercion(MutToConstPointer));
x as *mut u8 as *const u8
}
24 changes: 24 additions & 0 deletions tests/ui/cast/ptr-to-ptr-different-regions.rs
@@ -0,0 +1,24 @@
// check-pass

// https://github.com/rust-lang/rust/issues/113257

#![deny(trivial_casts)] // The casts here are not trivial.

struct Foo<'a> { a: &'a () }

fn extend_lifetime_very_very_safely<'a>(v: *const Foo<'a>) -> *const Foo<'static> {
// This should pass because raw pointer casts can do anything they want.
v as *const Foo<'static>
}
Nilstrieb marked this conversation as resolved.
Show resolved Hide resolved

trait Trait {}

fn assert_static<'a>(ptr: *mut (dyn Trait + 'a)) -> *mut (dyn Trait + 'static) {
ptr as _
}

fn main() {
let unit = ();
let foo = Foo { a: &unit };
let _long: *const Foo<'static> = extend_lifetime_very_very_safely(&foo);
}