From 0d74752a385285d87b14c88cbc7eacdf11766dea Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Fri, 21 Apr 2023 20:07:13 -0700 Subject: [PATCH] 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). --- compiler/rustc_middle/src/ty/context/tls.rs | 9 +++++++ compiler/rustc_middle/src/ty/visit.rs | 10 +------ .../src/solve/eval_ctxt.rs | 4 ++- .../src/traits/error_reporting/mod.rs | 26 ++++++++++++++----- compiler/rustc_transmute/src/layout/tree.rs | 6 ++--- compiler/rustc_transmute/src/lib.rs | 4 +++ .../src/maybe_transmutable/mod.rs | 10 +++---- 7 files changed, 43 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_middle/src/ty/context/tls.rs b/compiler/rustc_middle/src/ty/context/tls.rs index fb0d909307e78..d3f6b7d899ba0 100644 --- a/compiler/rustc_middle/src/ty/context/tls.rs +++ b/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`"); + } +} diff --git a/compiler/rustc_middle/src/ty/visit.rs b/compiler/rustc_middle/src/ty/visit.rs index 1b07f52afca92..c9ce1e80d9fda 100644 --- a/compiler/rustc_middle/src/ty/visit.rs +++ b/compiler/rustc_middle/src/ty/visit.rs @@ -59,15 +59,7 @@ pub trait TypeVisitableExt<'tcx>: TypeVisitable> { 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) diff --git a/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs b/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs index bd52957d162f2..2c709699d4fa0 100644 --- a/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs +++ b/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), } } } diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs index ae21dcd2a360c..6a00fb4df6da0 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/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); + ) -> 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) { + ) -> 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), +} diff --git a/compiler/rustc_transmute/src/layout/tree.rs b/compiler/rustc_transmute/src/layout/tree.rs index a6d88b1342ae8..2acec31bb3afc 100644 --- a/compiler/rustc_transmute/src/layout/tree.rs +++ b/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> 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), } } diff --git a/compiler/rustc_transmute/src/lib.rs b/compiler/rustc_transmute/src/lib.rs index 8be02c1d9888a..f3a1bb4b335e2 100644 --- a/compiler/rustc_transmute/src/lib.rs +++ b/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>), + + /// 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; diff --git a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs index 2e2fb90e71c1a..f429387495422 100644 --- a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs +++ b/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)),