From b6657a8ad43baf28a540879af55d588b7bb29029 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Sun, 2 Jul 2023 17:50:24 +0200 Subject: [PATCH 1/2] Never consider raw pointer casts to be trival HIR typeck tries to figure out which casts are trivial by doing them as coercions and seeing whether this works. Since HIR typeck is oblivious of lifetimes, this doesn't work for pointer casts that only change the lifetime of the pointee, which are, as borrowck will tell you, not trivial. This change makes it so that raw pointer casts are never considered trivial. This also incidentally fixes the "trivial cast" lint false positive on the same code. Unfortunately, "trivial cast" lints are now never emitted on raw pointer casts, even if they truly are trivial. This could be fixed by also doing the lint in borrowck for raw pointers specifically. --- compiler/rustc_hir_typeck/src/cast.rs | 18 +++++++++++--- .../casts.redundant.InstSimplify.diff | 19 +++++++++------ .../casts.roundtrip.InstSimplify.diff | 15 ++++++++---- tests/mir-opt/instsimplify/casts.rs | 6 ++--- tests/ui/cast/ptr-to-ptr-different-regions.rs | 24 +++++++++++++++++++ 5 files changed, 64 insertions(+), 18 deletions(-) create mode 100644 tests/ui/cast/ptr-to-ptr-different-regions.rs diff --git a/compiler/rustc_hir_typeck/src/cast.rs b/compiler/rustc_hir_typeck/src/cast.rs index 419e154a17a..e423cf20e04 100644 --- a/compiler/rustc_hir_typeck/src/cast.rs +++ b/compiler/rustc_hir_typeck/src/cast.rs @@ -660,9 +660,21 @@ pub fn check(mut self, fcx: &FnCtxt<'a, '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) { diff --git a/tests/mir-opt/instsimplify/casts.redundant.InstSimplify.diff b/tests/mir-opt/instsimplify/casts.redundant.InstSimplify.diff index 1c417d4346d..9e1bce1ee20 100644 --- a/tests/mir-opt/instsimplify/casts.redundant.InstSimplify.diff +++ b/tests/mir-opt/instsimplify/casts.redundant.InstSimplify.diff @@ -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; } diff --git a/tests/mir-opt/instsimplify/casts.roundtrip.InstSimplify.diff b/tests/mir-opt/instsimplify/casts.roundtrip.InstSimplify.diff index 1ae9d45e66c..a6d68cd4e4b 100644 --- a/tests/mir-opt/instsimplify/casts.roundtrip.InstSimplify.diff +++ b/tests/mir-opt/instsimplify/casts.roundtrip.InstSimplify.diff @@ -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; } diff --git a/tests/mir-opt/instsimplify/casts.rs b/tests/mir-opt/instsimplify/casts.rs index 18bab524c64..86f9b34ea04 100644 --- a/tests/mir-opt/instsimplify/casts.rs +++ b/tests/mir-opt/instsimplify/casts.rs @@ -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 } diff --git a/tests/ui/cast/ptr-to-ptr-different-regions.rs b/tests/ui/cast/ptr-to-ptr-different-regions.rs new file mode 100644 index 00000000000..5592e613ac1 --- /dev/null +++ b/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> +} + +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); +} From e8a4814d6d37a9a2cc4607ca66a020301f7efd25 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Sun, 2 Jul 2023 17:50:42 +0200 Subject: [PATCH 2/2] Use let chains instead of let else This makes it more obvious that we're looking at a special case. --- compiler/rustc_mir_build/src/thir/cx/expr.rs | 32 ++++++++------------ 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index 0535ea24b82..ccaa996a070 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -191,11 +191,16 @@ fn mirror_expr_cast( 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, @@ -204,21 +209,6 @@ fn mirror_expr_cast( // 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); @@ -255,6 +245,10 @@ fn mirror_expr_cast( }; 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) } } }