From 98ddc0f8b54ffae53165f219514232663b892940 Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Wed, 19 Jul 2023 13:49:27 -0700 Subject: [PATCH] Safe Transmute: Fix ICE (Inconsistent is_transmutable result) This patch updates the code that constructs the `Assume` type to return an error instead of silently falling back to a default `Assume`. This fixes an ICE where error reporting would get a different `is_transmutable` result that is inconsistent with the original one computed during trait confirmation. Fixes #110969 --- .../src/solve/trait_goals.rs | 10 +++-- .../src/traits/error_reporting/mod.rs | 27 +++++++----- .../src/traits/select/confirmation.rs | 2 +- compiler/rustc_transmute/src/layout/tree.rs | 4 +- compiler/rustc_transmute/src/lib.rs | 27 ++++-------- .../src/maybe_transmutable/mod.rs | 4 +- ...892.stderr => issue-110892.current.stderr} | 10 ++--- .../transmutability/issue-110892.next.stderr | 42 +++++++++++++++++++ tests/ui/transmutability/issue-110892.rs | 6 ++- .../issue-110969.current.stderr | 15 +++++++ .../transmutability/issue-110969.next.stderr | 15 +++++++ tests/ui/transmutability/issue-110969.rs | 30 +++++++++++++ 12 files changed, 147 insertions(+), 45 deletions(-) rename tests/ui/transmutability/{issue-110892.stderr => issue-110892.current.stderr} (89%) create mode 100644 tests/ui/transmutability/issue-110892.next.stderr create mode 100644 tests/ui/transmutability/issue-110969.current.stderr create mode 100644 tests/ui/transmutability/issue-110969.next.stderr create mode 100644 tests/ui/transmutability/issue-110969.rs diff --git a/compiler/rustc_trait_selection/src/solve/trait_goals.rs b/compiler/rustc_trait_selection/src/solve/trait_goals.rs index e7867eead1597..e52ee70868168 100644 --- a/compiler/rustc_trait_selection/src/solve/trait_goals.rs +++ b/compiler/rustc_trait_selection/src/solve/trait_goals.rs @@ -599,10 +599,12 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> { // which will ICE for region vars. let args = ecx.tcx().erase_regions(goal.predicate.trait_ref.args); - let Some(assume) = - rustc_transmute::Assume::from_const(ecx.tcx(), goal.param_env, args.const_at(3)) - else { - return Err(NoSolution); + let maybe_assume = + rustc_transmute::Assume::from_const(ecx.tcx(), goal.param_env, args.const_at(3)); + let assume = match maybe_assume { + Some(Ok(assume)) => assume, + Some(Err(_guar)) => return Err(NoSolution), + None => return Err(NoSolution), }; let certainty = ecx.is_transmutable( 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 c14839fe9be08..0a517d53dbe94 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -69,7 +69,9 @@ pub struct ImplCandidate<'tcx> { } enum GetSafeTransmuteErrorAndReason { - Silent, + ErrorGuaranteed(ErrorGuaranteed), + // Unable to compute Safe Transmute result because of ambiguity + Ambiguous, Error { err_msg: String, safe_transmute_explanation: String }, } @@ -756,7 +758,10 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { trait_ref, span, ) { - GetSafeTransmuteErrorAndReason::Silent => return, + GetSafeTransmuteErrorAndReason::ErrorGuaranteed(_guar) => return, + // Unable to compute whether Safe Transmute is possible (for example, due to an unevaluated const). + // The same thing occurred during trait selection/confirmation, so there is no error to report here. + GetSafeTransmuteErrorAndReason::Ambiguous => return, GetSafeTransmuteErrorAndReason::Error { err_msg, safe_transmute_explanation, @@ -2884,15 +2889,17 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { src: trait_ref.args.type_at(1), }; let scope = trait_ref.args.type_at(2); - let Some(assume) = rustc_transmute::Assume::from_const( + + let maybe_assume = rustc_transmute::Assume::from_const( self.infcx.tcx, obligation.param_env, trait_ref.args.const_at(3), - ) else { - span_bug!( - span, - "Unable to construct rustc_transmute::Assume where it was previously possible" - ); + ); + + let assume = match maybe_assume { + Some(Ok(assume)) => assume, + Some(Err(guar)) => return GetSafeTransmuteErrorAndReason::ErrorGuaranteed(guar), + None => return GetSafeTransmuteErrorAndReason::Ambiguous, }; match rustc_transmute::TransmuteTypeEnv::new(self.infcx).is_transmutable( @@ -2938,8 +2945,8 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { format!("`{src}` is a shared reference, but `{dst}` is a unique reference") } // Already reported by rustc - rustc_transmute::Reason::TypeError => { - return GetSafeTransmuteErrorAndReason::Silent; + rustc_transmute::Reason::TypeError(guar) => { + return GetSafeTransmuteErrorAndReason::ErrorGuaranteed(guar); } rustc_transmute::Reason::SrcLayoutUnknown => { format!("`{src}` has an unknown layout") diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 2cb2895b47642..a89ec904930b6 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -340,7 +340,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { let predicate = self.tcx().erase_regions(self.tcx().erase_late_bound_regions(obligation.predicate)); - let Some(assume) = rustc_transmute::Assume::from_const( + let Some(Ok(assume)) = rustc_transmute::Assume::from_const( self.infcx.tcx, obligation.param_env, predicate.trait_ref.args.const_at(3), diff --git a/compiler/rustc_transmute/src/layout/tree.rs b/compiler/rustc_transmute/src/layout/tree.rs index 8bd268182560a..fd64031d99c88 100644 --- a/compiler/rustc_transmute/src/layout/tree.rs +++ b/compiler/rustc_transmute/src/layout/tree.rs @@ -261,8 +261,8 @@ pub(crate) mod rustc { use rustc_middle::ty::UintTy::*; use rustc_target::abi::HasDataLayout; - if let Err(e) = ty.error_reported() { - return Err(Err::TypeError(e)); + if let Err(guar) = ty.error_reported() { + return Err(Err::TypeError(guar)); } let target = tcx.data_layout(); diff --git a/compiler/rustc_transmute/src/lib.rs b/compiler/rustc_transmute/src/lib.rs index 05ad4a4a12ab3..1eac207f97a4b 100644 --- a/compiler/rustc_transmute/src/lib.rs +++ b/compiler/rustc_transmute/src/lib.rs @@ -59,7 +59,7 @@ pub enum Reason { /// Can't go from shared pointer to unique pointer DstIsMoreUnique, /// Encountered a type error - TypeError, + TypeError(ErrorGuaranteed), /// The layout of src is unknown SrcLayoutUnknown, /// The layout of dst is unknown @@ -79,6 +79,7 @@ mod rustc { use rustc_middle::ty::Ty; use rustc_middle::ty::TyCtxt; use rustc_middle::ty::ValTree; + use rustc_span::ErrorGuaranteed; /// The source and destination types of a transmutation. #[derive(TypeVisitable, Debug, Clone, Copy)] @@ -123,20 +124,14 @@ mod rustc { tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, c: Const<'tcx>, - ) -> Option { + ) -> Option> { use rustc_middle::ty::ScalarInt; use rustc_middle::ty::TypeVisitableExt; use rustc_span::symbol::sym; let c = c.eval(tcx, param_env); - if let Err(err) = c.error_reported() { - return Some(Self { - alignment: true, - lifetimes: true, - safety: true, - validity: true, - }); + return Some(Err(err)); } let adt_def = c.ty().ty_adt_def()?; @@ -151,14 +146,7 @@ mod rustc { let variant = adt_def.non_enum_variant(); let fields = match c.try_to_valtree() { Some(ValTree::Branch(branch)) => branch, - _ => { - return Some(Self { - alignment: true, - lifetimes: true, - safety: true, - validity: true, - }); - } + _ => return None, }; let get_field = |name| { @@ -171,15 +159,16 @@ mod rustc { fields[field_idx].unwrap_leaf() == ScalarInt::TRUE }; - Some(Self { + Some(Ok(Self { alignment: get_field(sym::alignment), lifetimes: get_field(sym::lifetimes), safety: get_field(sym::safety), validity: get_field(sym::validity), - }) + })) } } } #[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 b223a90f7514b..e1101c476b61c 100644 --- a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs +++ b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs @@ -78,8 +78,8 @@ mod rustc { let dst = Tree::from_ty(dst, context); match (src, dst) { - (Err(Err::TypeError(_)), _) | (_, Err(Err::TypeError(_))) => { - Answer::No(Reason::TypeError) + (Err(Err::TypeError(guar)), _) | (_, Err(Err::TypeError(guar))) => { + Answer::No(Reason::TypeError(guar)) } (Err(Err::UnknownLayout), _) => Answer::No(Reason::SrcLayoutUnknown), (_, Err(Err::UnknownLayout)) => Answer::No(Reason::DstLayoutUnknown), diff --git a/tests/ui/transmutability/issue-110892.stderr b/tests/ui/transmutability/issue-110892.current.stderr similarity index 89% rename from tests/ui/transmutability/issue-110892.stderr rename to tests/ui/transmutability/issue-110892.current.stderr index 13654307aee73..998ab2ad6127b 100644 --- a/tests/ui/transmutability/issue-110892.stderr +++ b/tests/ui/transmutability/issue-110892.current.stderr @@ -1,29 +1,29 @@ error: expected parameter name, found `,` - --> $DIR/issue-110892.rs:27:9 + --> $DIR/issue-110892.rs:29:9 | LL | , | ^ expected parameter name error: expected parameter name, found `,` - --> $DIR/issue-110892.rs:28:9 + --> $DIR/issue-110892.rs:30:9 | LL | , | ^ expected parameter name error: expected parameter name, found `,` - --> $DIR/issue-110892.rs:29:9 + --> $DIR/issue-110892.rs:31:9 | LL | , | ^ expected parameter name error: expected parameter name, found `,` - --> $DIR/issue-110892.rs:30:9 + --> $DIR/issue-110892.rs:32:9 | LL | , | ^ expected parameter name error[E0308]: mismatched types - --> $DIR/issue-110892.rs:31:10 + --> $DIR/issue-110892.rs:33:10 | LL | const fn from_options( | ------------ implicitly returns `()` as its body has no tail or `return` expression diff --git a/tests/ui/transmutability/issue-110892.next.stderr b/tests/ui/transmutability/issue-110892.next.stderr new file mode 100644 index 0000000000000..1b0f32be72679 --- /dev/null +++ b/tests/ui/transmutability/issue-110892.next.stderr @@ -0,0 +1,42 @@ +error: expected parameter name, found `,` + --> $DIR/issue-110892.rs:29:9 + | +LL | , + | ^ expected parameter name + +error: expected parameter name, found `,` + --> $DIR/issue-110892.rs:30:9 + | +LL | , + | ^ expected parameter name + +error: expected parameter name, found `,` + --> $DIR/issue-110892.rs:31:9 + | +LL | , + | ^ expected parameter name + +error: expected parameter name, found `,` + --> $DIR/issue-110892.rs:32:9 + | +LL | , + | ^ expected parameter name + +error[E0284]: type annotations needed: cannot satisfy `the constant `{ from_options(ASSUME_ALIGNMENT, ASSUME_LIFETIMES, ASSUME_SAFETY, ASSUME_VALIDITY) }` can be evaluated` + --> $DIR/issue-110892.rs:23:13 + | +LL | { from_options(ASSUME_ALIGNMENT, ASSUME_LIFETIMES, ASSUME_SAFETY, ASSUME_VALIDITY) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot satisfy `the constant `{ from_options(ASSUME_ALIGNMENT, ASSUME_LIFETIMES, ASSUME_SAFETY, ASSUME_VALIDITY) }` can be evaluated` + | +note: required by a bound in `is_transmutable` + --> $DIR/issue-110892.rs:23:13 + | +LL | pub fn is_transmutable< + | --------------- required by a bound in this function +... +LL | { from_options(ASSUME_ALIGNMENT, ASSUME_LIFETIMES, ASSUME_SAFETY, ASSUME_VALIDITY) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `is_transmutable` + +error: aborting due to 5 previous errors + +For more information about this error, try `rustc --explain E0284`. diff --git a/tests/ui/transmutability/issue-110892.rs b/tests/ui/transmutability/issue-110892.rs index ce926b3999632..354f19512fa6c 100644 --- a/tests/ui/transmutability/issue-110892.rs +++ b/tests/ui/transmutability/issue-110892.rs @@ -1,4 +1,6 @@ // check-fail +// revisions: current next +//[next] compile-flags: -Ztrait-solver=next #![feature(generic_const_exprs, transmutability)] #![allow(incomplete_features)] @@ -18,7 +20,7 @@ mod assert { Dst: BikeshedIntrinsicFrom< Src, Context, - { from_options(ASSUME_ALIGNMENT, ASSUME_LIFETIMES, ASSUME_SAFETY, ASSUME_VALIDITY) } + { from_options(ASSUME_ALIGNMENT, ASSUME_LIFETIMES, ASSUME_SAFETY, ASSUME_VALIDITY) } //[next]~ ERROR type annotations needed >, {} @@ -28,7 +30,7 @@ mod assert { , //~ ERROR expected parameter name, found `,` , //~ ERROR expected parameter name, found `,` , //~ ERROR expected parameter name, found `,` - ) -> Assume {} //~ ERROR mismatched types + ) -> Assume {} //[current]~ ERROR mismatched types } fn main() { diff --git a/tests/ui/transmutability/issue-110969.current.stderr b/tests/ui/transmutability/issue-110969.current.stderr new file mode 100644 index 0000000000000..9ab856b67632b --- /dev/null +++ b/tests/ui/transmutability/issue-110969.current.stderr @@ -0,0 +1,15 @@ +error[E0308]: mismatched types + --> $DIR/issue-110969.rs:26:74 + | +LL | const FALSE: bool = assert::is_transmutable::(); + | ^^ expected `Assume`, found `()` + +error[E0308]: mismatched types + --> $DIR/issue-110969.rs:26:29 + | +LL | const FALSE: bool = assert::is_transmutable::(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `bool`, found `()` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/transmutability/issue-110969.next.stderr b/tests/ui/transmutability/issue-110969.next.stderr new file mode 100644 index 0000000000000..9ab856b67632b --- /dev/null +++ b/tests/ui/transmutability/issue-110969.next.stderr @@ -0,0 +1,15 @@ +error[E0308]: mismatched types + --> $DIR/issue-110969.rs:26:74 + | +LL | const FALSE: bool = assert::is_transmutable::(); + | ^^ expected `Assume`, found `()` + +error[E0308]: mismatched types + --> $DIR/issue-110969.rs:26:29 + | +LL | const FALSE: bool = assert::is_transmutable::(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `bool`, found `()` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/transmutability/issue-110969.rs b/tests/ui/transmutability/issue-110969.rs new file mode 100644 index 0000000000000..3496b98f9ba35 --- /dev/null +++ b/tests/ui/transmutability/issue-110969.rs @@ -0,0 +1,30 @@ +// check-fail +// revisions: current next +//[next] compile-flags: -Ztrait-solver=next +#![feature(adt_const_params, generic_const_exprs, transmutability)] +#![allow(incomplete_features)] + +mod assert { + use std::mem::BikeshedIntrinsicFrom; + + pub fn is_transmutable() + where + Dst: BikeshedIntrinsicFrom, + { + } +} + +fn main() { + struct Context; + #[repr(C)] + struct Src; + #[repr(C)] + struct Dst; + + trait Trait { + // The `{}` should not cause an ICE + const FALSE: bool = assert::is_transmutable::(); + //~^ ERROR mismatched types + //~^^ ERROR mismatched types + } +}