diff --git a/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs b/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs index 63a73f8d50d..e8abd5964bb 100644 --- a/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs +++ b/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs @@ -675,11 +675,11 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { scope, assume, ) { - rustc_transmute::Answer::Yes => Ok(Certainty::Yes), - rustc_transmute::Answer::No(_) - | rustc_transmute::Answer::IfTransmutable { .. } - | rustc_transmute::Answer::IfAll(_) - | rustc_transmute::Answer::IfAny(_) => Err(NoSolution), + Ok(None) => Ok(Certainty::Yes), + Err(_) + | Ok(Some(rustc_transmute::Condition::IfTransmutable { .. })) + | Ok(Some(rustc_transmute::Condition::IfAll(_))) + | Ok(Some(rustc_transmute::Condition::IfAny(_))) => 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 7e132e0ab60..71419fe0eee 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -2751,13 +2751,14 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { rustc_transmute::Assume::from_const(self.infcx.tcx, obligation.param_env, trait_ref.substs.const_at(3)) else { span_bug!(span, "Unable to construct rustc_transmute::Assume where it was previously possible"); }; + // FIXME(bryangarza): Need to flatten here too match rustc_transmute::TransmuteTypeEnv::new(self.infcx).is_transmutable( obligation.cause, src_and_dst, scope, assume, ) { - rustc_transmute::Answer::No(reason) => { + Err(reason) => { let dst = trait_ref.substs.type_at(0); let src = trait_ref.substs.type_at(1); let custom_err_msg = format!( @@ -2795,7 +2796,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { (custom_err_msg, Some(reason_msg)) } // 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!( + Ok(None) => span_bug!( span, "Inconsistent rustc_transmute::is_transmutable(...) result, got Yes", ), diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 9b28873f709..528a5f9dc61 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -279,11 +279,12 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ImplSourceBuiltinData { nested: obligations } } - #[instrument(skip(self))] + #[instrument(level = "debug", skip(self))] fn confirm_transmutability_candidate( &mut self, obligation: &TraitObligation<'tcx>, ) -> Result>, SelectionError<'tcx>> { + #[instrument(level = "debug", skip(tcx, obligation, predicate))] fn flatten_answer_tree<'tcx>( tcx: TyCtxt<'tcx>, obligation: &TraitObligation<'tcx>, @@ -291,11 +292,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { answer: rustc_transmute::Answer>, ) -> Result>, SelectionError<'tcx>> { match answer { - rustc_transmute::Answer::Yes => Ok(vec![]), - rustc_transmute::Answer::No(_) => Err(Unimplemented), + Ok(None) => Ok(vec![]), + Err(_) => Err(Unimplemented), // FIXME(bryangarza): Add separate `IfAny` case, instead of treating as `IfAll` - rustc_transmute::Answer::IfAll(answers) - | rustc_transmute::Answer::IfAny(answers) => { + Ok(Some(rustc_transmute::Condition::IfAll(answers))) + | Ok(Some(rustc_transmute::Condition::IfAny(answers))) => { let mut nested = vec![]; for flattened in answers .into_iter() @@ -305,7 +306,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { } Ok(nested) } - rustc_transmute::Answer::IfTransmutable { src, dst } => { + Ok(Some(rustc_transmute::Condition::IfTransmutable { src, dst })) => { let trait_def_id = obligation.predicate.def_id(); let scope = predicate.trait_ref.substs.type_at(2); let assume_const = predicate.trait_ref.substs.const_at(3); @@ -334,8 +335,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { } } - debug!(?obligation, "confirm_transmutability_candidate"); - // We erase regions here because transmutability calls layout queries, // which does not handle inference regions and doesn't particularly // care about other regions. Erasing late-bound regions is equivalent @@ -352,21 +351,21 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { return Err(Unimplemented); }; + let dst = predicate.trait_ref.substs.type_at(0); + let src = predicate.trait_ref.substs.type_at(1); let mut transmute_env = rustc_transmute::TransmuteTypeEnv::new(self.infcx); let maybe_transmutable = transmute_env.is_transmutable( obligation.cause.clone(), - rustc_transmute::Types { - dst: predicate.trait_ref.substs.type_at(0), - src: predicate.trait_ref.substs.type_at(1), - }, + rustc_transmute::Types { dst, src }, predicate.trait_ref.substs.type_at(2), assume, ); - info!(?maybe_transmutable); - let nested = flatten_answer_tree(self.tcx(), obligation, predicate, maybe_transmutable)?; - info!(?nested); - Ok(ImplSourceBuiltinData { nested }) + debug!(?src, ?dst); + let fully_flattened = + flatten_answer_tree(self.tcx(), obligation, predicate, maybe_transmutable)?; + debug!(?fully_flattened); + Ok(ImplSourceBuiltinData { nested: fully_flattened }) } /// This handles the case where an `auto trait Foo` impl is being used. diff --git a/compiler/rustc_transmute/src/lib.rs b/compiler/rustc_transmute/src/lib.rs index c4a99d9eb89..baf63e6d3a2 100644 --- a/compiler/rustc_transmute/src/lib.rs +++ b/compiler/rustc_transmute/src/lib.rs @@ -19,15 +19,12 @@ pub struct Assume { pub validity: bool, } -/// The type encodes answers to the question: "Are these types transmutable?" -#[derive(Debug, Hash, Eq, PartialEq, PartialOrd, Ord, Clone)] -pub enum Answer { - /// `Src` is transmutable into `Dst`. - Yes, - - /// `Src` is NOT transmutable into `Dst`. - No(Reason), +/// Either we have an error, or we have an optional Condition that must hold. +pub type Answer = Result>, Reason>; +/// A condition which must hold for safe transmutation to be possible +#[derive(Debug, Hash, Eq, PartialEq, Clone)] +pub enum Condition { /// `Src` is transmutable into `Dst`, if `src` is transmutable into `dst`. IfTransmutable { src: R, dst: R }, diff --git a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs index d1077488c79..de00efb1614 100644 --- a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs +++ b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs @@ -5,7 +5,7 @@ mod tests; use crate::{ layout::{self, dfa, Byte, Dfa, Nfa, Ref, Tree, Uninhabited}, maybe_transmutable::query_context::QueryContext, - Answer, Map, Reason, + Answer, Condition, Map, Reason, }; pub(crate) struct MaybeTransmutableQuery @@ -76,12 +76,12 @@ mod rustc { let dst = Tree::from_ty(dst, context); match (src, dst) { - // Answer `Yes` here, because 'unknown layout' and type errors will already + // Answer `Ok(None)` 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(Err::TypeError(_))) => Err(Answer::Yes), - (Err(Err::Unknown), _) | (_, Err(Err::Unknown)) => Err(Answer::Yes), + (Err(Err::TypeError(_)), _) | (_, Err(Err::TypeError(_))) => Err(Ok(None)), + (Err(Err::Unknown), _) | (_, Err(Err::Unknown)) => Err(Ok(None)), (Err(Err::Unspecified), _) | (_, Err(Err::Unspecified)) => { - Err(Answer::No(Reason::SrcIsUnspecified)) + Err(Err(Reason::SrcIsUnspecified)) } (Ok(src), Ok(dst)) => Ok((src, dst)), } @@ -127,13 +127,12 @@ where // Convert `src` from a tree-based representation to an NFA-based representation. // If the conversion fails because `src` is uninhabited, conclude that the transmutation // is acceptable, because instances of the `src` type do not exist. - let src = Nfa::from_tree(src).map_err(|Uninhabited| Answer::Yes)?; + let src = Nfa::from_tree(src).map_err(|Uninhabited| Ok(None))?; // Convert `dst` from a tree-based representation to an NFA-based representation. // If the conversion fails because `src` is uninhabited, conclude that the transmutation // is unacceptable, because instances of the `dst` type do not exist. - let dst = - Nfa::from_tree(dst).map_err(|Uninhabited| Answer::No(Reason::DstIsPrivate))?; + let dst = Nfa::from_tree(dst).map_err(|Uninhabited| Err(Reason::DstIsPrivate))?; Ok((src, dst)) }); @@ -205,13 +204,13 @@ where } else { let answer = if dst_state == self.dst.accepting { // truncation: `size_of(Src) >= size_of(Dst)` - Answer::Yes + Ok(None) } else if src_state == self.src.accepting { // extension: `size_of(Src) >= size_of(Dst)` if let Some(dst_state_prime) = self.dst.byte_from(dst_state, Byte::Uninit) { self.answer_memo(cache, src_state, dst_state_prime) } else { - Answer::No(Reason::DstIsTooBig) + Err(Reason::DstIsTooBig) } } else { let src_quantifier = if self.assume.validity { @@ -244,7 +243,7 @@ where } else { // otherwise, we've exhausted our options. // the DFAs, from this point onwards, are bit-incompatible. - Answer::No(Reason::DstIsBitIncompatible) + Err(Reason::DstIsBitIncompatible) } }, ), @@ -252,16 +251,16 @@ where // The below early returns reflect how this code would behave: // if self.assume.validity { - // bytes_answer.or(refs_answer) + // or(bytes_answer, refs_answer) // } else { - // bytes_answer.and(refs_answer) + // and(bytes_answer, refs_answer) // } // ...if `refs_answer` was computed lazily. The below early // returns can be deleted without impacting the correctness of // the algoritm; only its performance. match bytes_answer { - Answer::No(..) if !self.assume.validity => return bytes_answer, - Answer::Yes if self.assume.validity => return bytes_answer, + Err(_) if !self.assume.validity => return bytes_answer, + Ok(None) if self.assume.validity => return bytes_answer, _ => {} }; @@ -277,20 +276,25 @@ where .into_iter() .map(|(&dst_ref, &dst_state_prime)| { if !src_ref.is_mutable() && dst_ref.is_mutable() { - Answer::No(Reason::DstIsMoreUnique) + Err(Reason::DstIsMoreUnique) } else if !self.assume.alignment && src_ref.min_align() < dst_ref.min_align() { - Answer::No(Reason::DstHasStricterAlignment) + Err(Reason::DstHasStricterAlignment) } else { // ...such that `src` is transmutable into `dst`, if // `src_ref` is transmutability into `dst_ref`. - Answer::IfTransmutable { src: src_ref, dst: dst_ref } - .and(self.answer_memo( + and( + Ok(Some(Condition::IfTransmutable { + src: src_ref, + dst: dst_ref, + })), + self.answer_memo( cache, src_state_prime, dst_state_prime, - )) + ), + ) } }), ) @@ -299,9 +303,9 @@ where ); if self.assume.validity { - bytes_answer.or(refs_answer) + or(bytes_answer, refs_answer) } else { - bytes_answer.and(refs_answer) + and(bytes_answer, refs_answer) } }; if let Some(..) = cache.insert((src_state, dst_state), answer.clone()) { @@ -312,81 +316,55 @@ where } } -impl Answer -where - R: layout::Ref, -{ - pub(crate) fn and(self, rhs: Self) -> Self { - match (self, rhs) { - (_, Self::No(reason)) | (Self::No(reason), _) => Self::No(reason), - - (Self::Yes, other) | (other, Self::Yes) => other, - - (Self::IfAll(mut lhs), Self::IfAll(ref mut rhs)) => { - lhs.append(rhs); - Self::IfAll(lhs) - } - - (constraint, Self::IfAll(mut constraints)) - | (Self::IfAll(mut constraints), constraint) => { - constraints.push(constraint); - Self::IfAll(constraints) - } - - (lhs, rhs) => Self::IfAll(vec![lhs, rhs]), +fn and(lhs: Answer, rhs: Answer) -> Answer { + // Should propagate errors on the right side, because the initial value + // used in `apply` is on the left side. + let rhs = rhs?; + let lhs = lhs?; + Ok(match (lhs, rhs) { + // If only one side has a condition, pass it along + (None, other) | (other, None) => other, + // If both sides have IfAll conditions, merge them + (Some(Condition::IfAll(mut lhs)), Some(Condition::IfAll(ref mut rhs))) => { + lhs.append(rhs); + Some(Condition::IfAll(lhs)) } - } - - pub(crate) fn or(self, rhs: Self) -> Self { - match (self, rhs) { - (Self::Yes, _) | (_, Self::Yes) => Self::Yes, - (other, Self::No(reason)) | (Self::No(reason), other) => other, - (Self::IfAny(mut lhs), Self::IfAny(ref mut rhs)) => { - lhs.append(rhs); - Self::IfAny(lhs) - } - (constraint, Self::IfAny(mut constraints)) - | (Self::IfAny(mut constraints), constraint) => { - constraints.push(constraint); - Self::IfAny(constraints) - } - (lhs, rhs) => Self::IfAny(vec![lhs, rhs]), + // If only one side is an IfAll, add the other Condition to it + (constraint, Some(Condition::IfAll(mut constraints))) + | (Some(Condition::IfAll(mut constraints)), constraint) => { + constraints.push(Ok(constraint)); + Some(Condition::IfAll(constraints)) } - } + // Otherwise, both lhs and rhs conditions can be combined in a parent IfAll + (lhs, rhs) => Some(Condition::IfAll(vec![Ok(lhs), Ok(rhs)])), + }) } -pub fn for_all(iter: I, f: F) -> Answer -where - R: layout::Ref, - I: IntoIterator, - F: FnMut(::Item) -> Answer, -{ - use std::ops::ControlFlow::{Break, Continue}; - let (Continue(result) | Break(result)) = - iter.into_iter().map(f).try_fold(Answer::Yes, |constraints, constraint| { - match constraint.and(constraints) { - Answer::No(reason) => Break(Answer::No(reason)), - maybe => Continue(maybe), - } - }); - result -} - -pub fn there_exists(iter: I, f: F) -> Answer -where - R: layout::Ref, - I: IntoIterator, - F: FnMut(::Item) -> Answer, -{ - use std::ops::ControlFlow::{Break, Continue}; - let (Continue(result) | Break(result)) = iter.into_iter().map(f).try_fold( - Answer::No(Reason::DstIsBitIncompatible), - |constraints, constraint| match constraint.or(constraints) { - Answer::Yes => Break(Answer::Yes), - maybe => Continue(maybe), - }, - ); - result +fn or(lhs: Answer, rhs: Answer) -> Answer { + // If both are errors, then we should return the one on the right + if lhs.is_err() && rhs.is_err() { + return rhs; + } + // Otherwise, errors can be ignored for the rest of the pattern matching + let lhs = lhs.unwrap_or(None); + let rhs = rhs.unwrap_or(None); + Ok(match (lhs, rhs) { + // If only one side has a condition, pass it along + (None, other) | (other, None) => other, + // If both sides have IfAny conditions, merge them + (Some(Condition::IfAny(mut lhs)), Some(Condition::IfAny(ref mut rhs))) => { + lhs.append(rhs); + Some(Condition::IfAny(lhs)) + } + // If only one side is an IfAny, add the other Condition to it + (constraint, Some(Condition::IfAny(mut constraints))) + | (Some(Condition::IfAny(mut constraints)), constraint) => { + constraints.push(Ok(constraint)); + Some(Condition::IfAny(constraints)) + } + // Otherwise, both lhs and rhs conditions can be combined in a parent IfAny + (lhs, rhs) => Some(Condition::IfAny(vec![Ok(lhs), Ok(rhs)])), + }) } pub enum Quantifier { @@ -403,16 +381,14 @@ impl Quantifier { use std::ops::ControlFlow::{Break, Continue}; let (init, try_fold_f): (_, fn(_, _) -> _) = match self { - Self::ThereExists => { - (Answer::No(Reason::DstIsBitIncompatible), |accum: Answer, next| { - match accum.or(next) { - Answer::Yes => Break(Answer::Yes), - maybe => Continue(maybe), - } - }) - } - Self::ForAll => (Answer::Yes, |accum: Answer, next| match accum.and(next) { - Answer::No(reason) => Break(Answer::No(reason)), + Self::ThereExists => (Err(Reason::DstIsBitIncompatible), |accum: Answer, next| { + match or(accum, next) { + Ok(None) => Break(Ok(None)), + maybe => Continue(maybe), + } + }), + Self::ForAll => (Ok(None), |accum: Answer, next| match and(accum, next) { + Err(reason) => Break(Err(reason)), maybe => Continue(maybe), }), }; diff --git a/compiler/rustc_transmute/src/maybe_transmutable/tests.rs b/compiler/rustc_transmute/src/maybe_transmutable/tests.rs index a8675f4ae37..df6a83df23f 100644 --- a/compiler/rustc_transmute/src/maybe_transmutable/tests.rs +++ b/compiler/rustc_transmute/src/maybe_transmutable/tests.rs @@ -1,6 +1,6 @@ use super::query_context::test::{Def, UltraMinimal}; use crate::maybe_transmutable::MaybeTransmutableQuery; -use crate::{layout, Answer, Reason}; +use crate::{layout, Reason}; use itertools::Itertools; mod bool { @@ -17,7 +17,7 @@ mod bool { UltraMinimal, ) .answer(); - assert_eq!(answer, Answer::Yes); + assert_eq!(answer, Ok(None)); } #[test] @@ -30,7 +30,7 @@ mod bool { UltraMinimal, ) .answer(); - assert_eq!(answer, Answer::Yes); + assert_eq!(answer, Ok(None)); } #[test] @@ -65,7 +65,7 @@ mod bool { if src_set.is_subset(&dst_set) { assert_eq!( - Answer::Yes, + Ok(None), MaybeTransmutableQuery::new( src_layout.clone(), dst_layout.clone(), @@ -80,7 +80,7 @@ mod bool { ); } else if !src_set.is_disjoint(&dst_set) { assert_eq!( - Answer::Yes, + Ok(None), MaybeTransmutableQuery::new( src_layout.clone(), dst_layout.clone(), @@ -95,7 +95,7 @@ mod bool { ); } else { assert_eq!( - Answer::No(Reason::DstIsBitIncompatible), + Err(Reason::DstIsBitIncompatible), MaybeTransmutableQuery::new( src_layout.clone(), dst_layout.clone(),