Auto merge of #145469 - cjgillot:split-transmute, r=nnethercote

Split transmute check from HIR typeck

This resolves a FIXME in the implementation of `check_transmute`.

`check_transmute` needs to compute type layout, hence needing to see reveal opaques and all type aliases.
Having this inside typeck causes a cycle. For instance: `tests/ui/impl-trait/transmute/in-defining-scope.rs`.

This PR moves the transmute check outside of typeck, by putting the list of deferred transmute checks in typeck results.

Author: bors

compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

@@ -83,14 +83,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         *self.deferred_cast_checks.borrow_mut() = deferred_cast_checks;
     }
 
-    pub(in super::super) fn check_transmutes(&self) {
-        let mut deferred_transmute_checks = self.deferred_transmute_checks.borrow_mut();
-        debug!("FnCtxt::check_transmutes: {} deferred checks", deferred_transmute_checks.len());
-        for (from, to, hir_id) in deferred_transmute_checks.drain(..) {
-            self.check_transmute(from, to, hir_id);
-        }
-    }
-
     pub(in super::super) fn check_asms(&self) {
         let mut deferred_asm_checks = self.deferred_asm_checks.borrow_mut();
         debug!("FnCtxt::check_asm: {} deferred checks", deferred_asm_checks.len());

compiler/rustc_hir_typeck/src/intrinsicck.rs

@@ -7,11 +7,10 @@ use rustc_hir as hir;
 use rustc_index::Idx;
 use rustc_middle::bug;
 use rustc_middle::ty::layout::{LayoutError, SizeSkeleton};
-use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt};
+use rustc_middle::ty::{self, Ty, TyCtxt};
+use rustc_span::def_id::LocalDefId;
 use tracing::trace;
 
-use super::FnCtxt;
-
 /// If the type is `Option<T>`, it will return `T`, otherwise
 /// the type itself. Works on most `Option`-like types.
 fn unpack_option_like<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Ty<'tcx> {
@@ -39,119 +38,117 @@ fn unpack_option_like<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Ty<'tcx> {
     ty
 }
 
-impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
-    /// FIXME: Move this check out of typeck, since it'll easily cycle when revealing opaques,
-    /// and we shouldn't need to check anything here if the typeck results are tainted.
-    pub(crate) fn check_transmute(&self, from: Ty<'tcx>, to: Ty<'tcx>, hir_id: HirId) {
-        let tcx = self.tcx;
-        let dl = &tcx.data_layout;
-        let span = tcx.hir_span(hir_id);
-        let normalize = |ty| {
-            let ty = self.resolve_vars_if_possible(ty);
-            if let Ok(ty) =
-                self.tcx.try_normalize_erasing_regions(self.typing_env(self.param_env), ty)
-            {
-                ty
+/// Try to display a sensible error with as much information as possible.
+fn skeleton_string<'tcx>(
+    ty: Ty<'tcx>,
+    sk: Result<SizeSkeleton<'tcx>, &'tcx LayoutError<'tcx>>,
+) -> String {
+    match sk {
+        Ok(SizeSkeleton::Pointer { tail, .. }) => format!("pointer to `{tail}`"),
+        Ok(SizeSkeleton::Known(size, _)) => {
+            if let Some(v) = u128::from(size.bytes()).checked_mul(8) {
+                format!("{v} bits")
             } else {
-                Ty::new_error_with_message(
-                    tcx,
-                    span,
-                    "tried to normalize non-wf type in check_transmute",
-                )
+                // `u128` should definitely be able to hold the size of different architectures
+                // larger sizes should be reported as error `are too big for the target architecture`
+                // otherwise we have a bug somewhere
+                bug!("{:?} overflow for u128", size)
             }
-        };
-        let from = normalize(from);
-        let to = normalize(to);
-        trace!(?from, ?to);
-        if from.has_non_region_infer() || to.has_non_region_infer() {
-            // Note: this path is currently not reached in any test, so any
-            // example that triggers this would be worth minimizing and
-            // converting into a test.
-            self.dcx().span_bug(span, "argument to transmute has inference variables");
         }
-        // Transmutes that are only changing lifetimes are always ok.
-        if from == to {
-            return;
+        Ok(SizeSkeleton::Generic(size)) => {
+            format!("generic size {size}")
+        }
+        Err(LayoutError::TooGeneric(bad)) => {
+            if *bad == ty {
+                "this type does not have a fixed size".to_owned()
+            } else {
+                format!("size can vary because of {bad}")
+            }
+        }
+        Err(err) => err.to_string(),
+    }
+}
+
+fn check_transmute<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    typing_env: ty::TypingEnv<'tcx>,
+    from: Ty<'tcx>,
+    to: Ty<'tcx>,
+    hir_id: HirId,
+) {
+    let span = || tcx.hir_span(hir_id);
+    let normalize = |ty| {
+        if let Ok(ty) = tcx.try_normalize_erasing_regions(typing_env, ty) {
+            ty
+        } else {
+            Ty::new_error_with_message(
+                tcx,
+                span(),
+                format!("tried to normalize non-wf type {ty:#?} in check_transmute"),
+            )
         }
+    };
 
-        let skel = |ty| SizeSkeleton::compute(ty, tcx, self.typing_env(self.param_env));
-        let sk_from = skel(from);
-        let sk_to = skel(to);
-        trace!(?sk_from, ?sk_to);
+    let from = normalize(from);
+    let to = normalize(to);
+    trace!(?from, ?to);
 
-        // Check for same size using the skeletons.
-        if let (Ok(sk_from), Ok(sk_to)) = (sk_from, sk_to) {
-            if sk_from.same_size(sk_to) {
-                return;
-            }
+    // Transmutes that are only changing lifetimes are always ok.
+    if from == to {
+        return;
+    }
 
-            // Special-case transmuting from `typeof(function)` and
-            // `Option<typeof(function)>` to present a clearer error.
-            let from = unpack_option_like(tcx, from);
-            if let (&ty::FnDef(..), SizeSkeleton::Known(size_to, _)) = (from.kind(), sk_to)
-                && size_to == Pointer(dl.instruction_address_space).size(&tcx)
-            {
-                struct_span_code_err!(self.dcx(), span, E0591, "can't transmute zero-sized type")
-                    .with_note(format!("source type: {from}"))
-                    .with_note(format!("target type: {to}"))
-                    .with_help("cast with `as` to a pointer instead")
-                    .emit();
-                return;
-            }
+    let sk_from = SizeSkeleton::compute(from, tcx, typing_env);
+    let sk_to = SizeSkeleton::compute(to, tcx, typing_env);
+    trace!(?sk_from, ?sk_to);
+
+    // Check for same size using the skeletons.
+    if let Ok(sk_from) = sk_from
+        && let Ok(sk_to) = sk_to
+    {
+        if sk_from.same_size(sk_to) {
+            return;
         }
 
-        // Try to display a sensible error with as much information as possible.
-        let skeleton_string = |ty: Ty<'tcx>, sk: Result<_, &_>| match sk {
-            Ok(SizeSkeleton::Pointer { tail, .. }) => format!("pointer to `{tail}`"),
-            Ok(SizeSkeleton::Known(size, _)) => {
-                if let Some(v) = u128::from(size.bytes()).checked_mul(8) {
-                    format!("{v} bits")
-                } else {
-                    // `u128` should definitely be able to hold the size of different architectures
-                    // larger sizes should be reported as error `are too big for the target architecture`
-                    // otherwise we have a bug somewhere
-                    bug!("{:?} overflow for u128", size)
-                }
-            }
-            Ok(SizeSkeleton::Generic(size)) => {
-                if let Some(size) =
-                    self.try_structurally_resolve_const(span, size).try_to_target_usize(tcx)
-                {
-                    format!("{size} bytes")
-                } else {
-                    format!("generic size {size}")
-                }
-            }
-            Err(LayoutError::TooGeneric(bad)) => {
-                if *bad == ty {
-                    "this type does not have a fixed size".to_owned()
-                } else {
-                    format!("size can vary because of {bad}")
-                }
-            }
-            Err(err) => err.to_string(),
-        };
-
-        let mut err = struct_span_code_err!(
-            self.dcx(),
-            span,
-            E0512,
-            "cannot transmute between types of different sizes, \
-                                        or dependently-sized types"
-        );
-        if from == to {
-            err.note(format!("`{from}` does not have a fixed size"));
-            err.emit();
-        } else {
-            err.note(format!("source type: `{}` ({})", from, skeleton_string(from, sk_from)))
-                .note(format!("target type: `{}` ({})", to, skeleton_string(to, sk_to)));
-            if let Err(LayoutError::ReferencesError(_)) = sk_from {
-                err.delay_as_bug();
-            } else if let Err(LayoutError::ReferencesError(_)) = sk_to {
-                err.delay_as_bug();
-            } else {
-                err.emit();
-            }
+        // Special-case transmuting from `typeof(function)` and
+        // `Option<typeof(function)>` to present a clearer error.
+        let from = unpack_option_like(tcx, from);
+        if let ty::FnDef(..) = from.kind()
+            && let SizeSkeleton::Known(size_to, _) = sk_to
+            && size_to == Pointer(tcx.data_layout.instruction_address_space).size(&tcx)
+        {
+            struct_span_code_err!(tcx.sess.dcx(), span(), E0591, "can't transmute zero-sized type")
+                .with_note(format!("source type: {from}"))
+                .with_note(format!("target type: {to}"))
+                .with_help("cast with `as` to a pointer instead")
+                .emit();
+            return;
         }
     }
+
+    let mut err = struct_span_code_err!(
+        tcx.sess.dcx(),
+        span(),
+        E0512,
+        "cannot transmute between types of different sizes, or dependently-sized types"
+    );
+    if from == to {
+        err.note(format!("`{from}` does not have a fixed size"));
+        err.emit();
+    } else {
+        err.note(format!("source type: `{}` ({})", from, skeleton_string(from, sk_from)));
+        err.note(format!("target type: `{}` ({})", to, skeleton_string(to, sk_to)));
+        err.emit();
+    }
+}
+
+pub(crate) fn check_transmutes(tcx: TyCtxt<'_>, owner: LocalDefId) {
+    assert!(!tcx.is_typeck_child(owner.to_def_id()));
+    let typeck_results = tcx.typeck(owner);
+    let None = typeck_results.tainted_by_errors else { return };
+
+    let typing_env = ty::TypingEnv::post_analysis(tcx, owner);
+    for &(from, to, hir_id) in &typeck_results.transmutes_to_check {
+        check_transmute(tcx, typing_env, from, to, hir_id);
+    }
 }

compiler/rustc_hir_typeck/src/lib.rs

@@ -251,10 +251,6 @@ fn typeck_with_inspect<'tcx>(
         fcx.report_ambiguity_errors();
     }
 
-    if let None = fcx.infcx.tainted_by_errors() {
-        fcx.check_transmutes();
-    }
-
     fcx.check_asms();
 
     let typeck_results = fcx.resolve_type_vars_in_body(body);
@@ -555,6 +551,7 @@ pub fn provide(providers: &mut Providers) {
         method_autoderef_steps: method::probe::method_autoderef_steps,
         typeck,
         used_trait_imports,
+        check_transmutes: intrinsicck::check_transmutes,
         ..*providers
     };
 }

compiler/rustc_hir_typeck/src/writeback.rs

@@ -74,6 +74,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         wbcx.visit_user_provided_tys();
         wbcx.visit_user_provided_sigs();
         wbcx.visit_coroutine_interior();
+        wbcx.visit_transmutes();
         wbcx.visit_offset_of_container_types();
 
         wbcx.typeck_results.rvalue_scopes =
@@ -532,6 +533,18 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
         }
     }
 
+    fn visit_transmutes(&mut self) {
+        let tcx = self.tcx();
+        let fcx_typeck_results = self.fcx.typeck_results.borrow();
+        assert_eq!(fcx_typeck_results.hir_owner, self.typeck_results.hir_owner);
+        for &(from, to, hir_id) in self.fcx.deferred_transmute_checks.borrow().iter() {
+            let span = tcx.hir_span(hir_id);
+            let from = self.resolve(from, &span);
+            let to = self.resolve(to, &span);
+            self.typeck_results.transmutes_to_check.push((from, to, hir_id));
+        }
+    }
+
     #[instrument(skip(self), level = "debug")]
     fn visit_opaque_types(&mut self) {
         let tcx = self.tcx();

compiler/rustc_interface/src/passes.rs

@@ -1080,7 +1080,8 @@ fn run_required_analyses(tcx: TyCtxt<'_>) {
             if !tcx.is_typeck_child(def_id.to_def_id()) {
                 // Child unsafety and borrowck happens together with the parent
                 tcx.ensure_ok().check_unsafety(def_id);
-                tcx.ensure_ok().mir_borrowck(def_id)
+                tcx.ensure_ok().mir_borrowck(def_id);
+                tcx.ensure_ok().check_transmutes(def_id);
             }
             tcx.ensure_ok().has_ffi_unwind_calls(def_id);
 

compiler/rustc_middle/src/query/mod.rs

@@ -1115,6 +1115,11 @@ rustc_queries! {
         desc { |tcx| "collecting all inherent impls for `{:?}`", key }
     }
 
+    /// Transmute-check this `LocalDefId`.
+    query check_transmutes(key: LocalDefId) {
+        desc { |tcx| "check transmute calls inside `{}`", tcx.def_path_str(key) }
+    }
+
     /// Unsafety-check this `LocalDefId`.
     query check_unsafety(key: LocalDefId) {
         desc { |tcx| "unsafety-checking `{}`", tcx.def_path_str(key) }

compiler/rustc_middle/src/ty/typeck_results.rs

@@ -210,6 +210,11 @@ pub struct TypeckResults<'tcx> {
     /// on closure size.
     pub closure_size_eval: LocalDefIdMap<ClosureSizeProfileData<'tcx>>,
 
+    /// Stores the types involved in calls to `transmute` intrinsic. These are meant to be checked
+    /// outside of typeck and borrowck to avoid cycles with opaque types and coroutine layout
+    /// computation.
+    pub transmutes_to_check: Vec<(Ty<'tcx>, Ty<'tcx>, HirId)>,
+
     /// Container types and field indices of `offset_of!` expressions
     offset_of_data: ItemLocalMap<(Ty<'tcx>, Vec<(VariantIdx, FieldIdx)>)>,
 }
@@ -241,6 +246,7 @@ impl<'tcx> TypeckResults<'tcx> {
             rvalue_scopes: Default::default(),
             coroutine_stalled_predicates: Default::default(),
             closure_size_eval: Default::default(),
+            transmutes_to_check: Default::default(),
             offset_of_data: Default::default(),
         }
     }

tests/ui/const-generics/transmute-fail.stderr

@@ -6,6 +6,14 @@ LL | fn bar<const W: bool, const H: usize>(v: [[u32; H]; W]) -> [[u32; W]; H] {
    |
    = note: the length of array `[[u32; H]; W]` must be type `usize`
 
+error: the constant `W` is not of type `usize`
+  --> $DIR/transmute-fail.rs:19:9
+   |
+LL |         std::mem::transmute(v)
+   |         ^^^^^^^^^^^^^^^^^^^ expected `usize`, found `bool`
+   |
+   = note: the length of array `[[u32; H]; W]` must be type `usize`
+
 error[E0512]: cannot transmute between types of different sizes, or dependently-sized types
   --> $DIR/transmute-fail.rs:11:9
    |
@@ -15,14 +23,6 @@ LL |         std::mem::transmute(v)
    = note: source type: `[[u32; H + 1]; W]` (size can vary because of [u32; H + 1])
    = note: target type: `[[u32; W + 1]; H]` (size can vary because of [u32; W + 1])
 
-error: the constant `W` is not of type `usize`
-  --> $DIR/transmute-fail.rs:19:9
-   |
-LL |         std::mem::transmute(v)
-   |         ^^^^^^^^^^^^^^^^^^^ expected `usize`, found `bool`
-   |
-   = note: the length of array `[[u32; H]; W]` must be type `usize`
-
 error[E0512]: cannot transmute between types of different sizes, or dependently-sized types
   --> $DIR/transmute-fail.rs:26:9
    |

tests/ui/consts/transmute-size-mismatch-before-typeck.rs

@@ -1,5 +1,9 @@
+//@ normalize-stderr-64bit: "8-byte" -> "word size"
+//@ normalize-stderr-32bit: "4-byte" -> "word size"
 //@ normalize-stderr-64bit: "64 bits" -> "word size"
 //@ normalize-stderr-32bit: "32 bits" -> "word size"
+//@ normalize-stderr-64bit: "16-byte" -> "2 * word size"
+//@ normalize-stderr-32bit: "8-byte" -> "2 * word size"
 //@ normalize-stderr-64bit: "128 bits" -> "2 * word size"
 //@ normalize-stderr-32bit: "64 bits" -> "2 * word size"
 
@@ -10,4 +14,5 @@ fn main() {
 }
 
 const ZST: &[u8] = unsafe { std::mem::transmute(1usize) };
-//~^ ERROR cannot transmute between types of different sizes
+//~^ ERROR transmuting from
+//~| ERROR cannot transmute between types of different sizes