Safe Transmute: Pass ErrorGuaranteed up to error reporting

This patch updates the `Answer` enum used by Safe Transmute to have a variant
for errors, and includes the `ErrorGuaranteed` type so that we know that an
error was already emitted. Before this change, we would return `Answer::Yes`
for failures, which gives the same end-result, but is confusing because
`Answer::Yes` is supposed to mean that safe transmutation is possible. Now,
it's clear when we have an error, and we can decide what to do with it during
error reporting. Also, we now know for sure that an error was emitted for
`LayoutError` (previously, we just assumed without confirming).

Author: bryangarza

compiler/rustc_middle/src/ty/context/tls.rs

@@ -4,6 +4,7 @@ use crate::dep_graph::TaskDepsRef;
 use crate::ty::query;
 use rustc_data_structures::sync::{self, Lock};
 use rustc_errors::Diagnostic;
+use rustc_errors::ErrorGuaranteed;
 #[cfg(not(parallel_compiler))]
 use std::cell::Cell;
 use std::mem;
@@ -153,3 +154,11 @@ where
 {
     with_context_opt(|opt_context| f(opt_context.map(|context| context.tcx)))
 }
+
+pub fn expect_compilation_to_fail() -> ErrorGuaranteed {
+    if let Some(guar) = with(|tcx| tcx.sess.is_compilation_going_to_fail()) {
+        guar
+    } else {
+        bug!("expect tcx.sess.is_compilation_going_to_fail() to return `Some`");
+    }
+}

compiler/rustc_middle/src/ty/visit.rs

@@ -59,15 +59,7 @@ pub trait TypeVisitableExt<'tcx>: TypeVisitable<TyCtxt<'tcx>> {
         self.has_type_flags(TypeFlags::HAS_ERROR)
     }
     fn error_reported(&self) -> Result<(), ErrorGuaranteed> {
-        if self.references_error() {
-            if let Some(reported) = ty::tls::with(|tcx| tcx.sess.is_compilation_going_to_fail()) {
-                Err(reported)
-            } else {
-                bug!("expect tcx.sess.is_compilation_going_to_fail return `Some`");
-            }
-        } else {
-            Ok(())
-        }
+        if self.references_error() { Err(ty::tls::expect_compilation_to_fail()) } else { Ok(()) }
     }
     fn has_non_region_param(&self) -> bool {
         self.has_type_flags(TypeFlags::NEEDS_SUBST - TypeFlags::HAS_RE_PARAM)

compiler/rustc_trait_selection/src/solve/eval_ctxt.rs

@@ -672,7 +672,9 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
             rustc_transmute::Answer::No(_)
             | rustc_transmute::Answer::IfTransmutable { .. }
             | rustc_transmute::Answer::IfAll(_)
-            | rustc_transmute::Answer::IfAny(_) => Err(NoSolution),
+            | rustc_transmute::Answer::IfAny(_)
+            // FIXME(bryangarza): Pass the `ErrorGuaranteed` along instead of losing it?
+            | rustc_transmute::Answer::Err(_) => Err(NoSolution),
         }
     }
 }

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

@@ -741,11 +741,17 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                             == self.tcx.lang_items().transmute_trait()
                         {
                             // Recompute the safe transmute reason and use that for the error reporting
-                            self.get_safe_transmute_error_and_reason(
+                            match self.get_safe_transmute_error_and_reason(
                                 obligation.clone(),
                                 trait_ref,
                                 span,
-                            )
+                            ) {
+                                SafeTransmuteError::ShouldReport { err_msg, explanation } => {
+                                    (err_msg, Some(explanation))
+                                }
+                                // An error is guaranteed to already have been reported at this point, no need to continue
+                                SafeTransmuteError::ErrorGuaranteed(_) => return,
+                            }
                         } else {
                             (err_msg, None)
                         };
@@ -1631,7 +1637,7 @@ trait InferCtxtPrivExt<'tcx> {
         obligation: Obligation<'tcx, ty::Predicate<'tcx>>,
         trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>,
         span: Span,
-    ) -> (String, Option<String>);
+    ) -> SafeTransmuteError;
 }
 
 impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
@@ -2922,7 +2928,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
         obligation: Obligation<'tcx, ty::Predicate<'tcx>>,
         trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>,
         span: Span,
-    ) -> (String, Option<String>) {
+    ) -> SafeTransmuteError {
         // Erase regions because layout code doesn't particularly care about regions.
         let trait_ref = self.tcx.erase_regions(self.tcx.erase_late_bound_regions(trait_ref));
 
@@ -2944,10 +2950,10 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
             rustc_transmute::Answer::No(reason) => {
                 let dst = trait_ref.substs.type_at(0);
                 let src = trait_ref.substs.type_at(1);
-                let custom_err_msg = format!(
+                let err_msg = format!(
                     "`{src}` cannot be safely transmuted into `{dst}` in the defining scope of `{scope}`"
                 );
-                let reason_msg = match reason {
+                let explanation = match reason {
                     rustc_transmute::Reason::SrcIsUnspecified => {
                         format!("`{src}` does not have a well-specified layout")
                     }
@@ -2968,13 +2974,14 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
                         format!("The size of `{src}` is smaller than the size of `{dst}`")
                     }
                 };
-                (custom_err_msg, Some(reason_msg))
+                SafeTransmuteError::ShouldReport { err_msg, explanation }
             }
             // Should never get a Yes at this point! We already ran it before, and did not get a Yes.
             rustc_transmute::Answer::Yes => span_bug!(
                 span,
                 "Inconsistent rustc_transmute::is_transmutable(...) result, got Yes",
             ),
+            rustc_transmute::Answer::Err(guar) => SafeTransmuteError::ErrorGuaranteed(guar),
             _ => span_bug!(span, "Unsupported rustc_transmute::Reason variant"),
         }
     }
@@ -3103,3 +3110,8 @@ pub enum DefIdOrName {
     DefId(DefId),
     Name(&'static str),
 }
+
+enum SafeTransmuteError {
+    ShouldReport { err_msg: String, explanation: String },
+    ErrorGuaranteed(ErrorGuaranteed),
+}

compiler/rustc_transmute/src/layout/tree.rs

@@ -187,15 +187,15 @@ pub(crate) mod rustc {
     pub(crate) enum Err {
         /// The layout of the type is unspecified.
         Unspecified,
-        /// This error will be surfaced elsewhere by rustc, so don't surface it.
-        Unknown,
+        Unknown(ErrorGuaranteed),
         TypeError(ErrorGuaranteed),
     }
 
     impl<'tcx> From<LayoutError<'tcx>> for Err {
         fn from(err: LayoutError<'tcx>) -> Self {
+            let guar = rustc_middle::ty::tls::expect_compilation_to_fail();
             match err {
-                LayoutError::Unknown(..) => Self::Unknown,
+                LayoutError::Unknown(..) => Self::Unknown(guar),
                 err => unimplemented!("{:?}", err),
             }
         }

compiler/rustc_transmute/src/lib.rs

@@ -39,6 +39,9 @@ where
 
     /// `Src` is transmutable into `Dst` if any of the enclosed requirements are met.
     IfAny(Vec<Answer<R>>),
+
+    /// Encountered an error during safe transmute computation
+    Err(ErrorGuaranteed),
 }
 
 /// Answers: Why wasn't the source type transmutable into the destination type?
@@ -162,3 +165,4 @@ mod rustc {
 
 #[cfg(feature = "rustc")]
 pub use rustc::*;
+use rustc_span::ErrorGuaranteed;

compiler/rustc_transmute/src/maybe_transmutable/mod.rs

@@ -75,12 +75,10 @@ mod rustc {
                 let dst = Tree::from_ty(dst, context);
 
                 match (src, dst) {
-                    // Answer `Yes` here, because 'unknown layout' and type errors will already
-                    // be reported by rustc. No need to spam the user with more errors.
-                    (Err(Err::TypeError(_)), _) => Err(Answer::Yes),
-                    (_, Err(Err::TypeError(_))) => Err(Answer::Yes),
-                    (Err(Err::Unknown), _) => Err(Answer::Yes),
-                    (_, Err(Err::Unknown)) => Err(Answer::Yes),
+                    (Err(Err::TypeError(guar)), _) => Err(Answer::Err(guar)),
+                    (_, Err(Err::TypeError(guar))) => Err(Answer::Err(guar)),
+                    (Err(Err::Unknown(guar)), _) => Err(Answer::Err(guar)),
+                    (_, Err(Err::Unknown(guar))) => Err(Answer::Err(guar)),
                     (Err(Err::Unspecified), _) => Err(Answer::No(Reason::SrcIsUnspecified)),
                     (_, Err(Err::Unspecified)) => Err(Answer::No(Reason::DstIsUnspecified)),
                     (Ok(src), Ok(dst)) => Ok((src, dst)),