Safe Transmute: Change Answer type to Result

This patch updates the `Answer` type from `rustc_transmute` so that it just a
type alias to `Result`. This makes it so that the standard methods for `Result`
can be used to process the `Answer` tree, including being able to make use of
the `?` operator on `Answer`s.

Also, remove some unused functions
This commit is contained in:
Bryan Garza 2023-04-27 14:38:32 -07:00
parent 8f1cec8d84
commit 263a4f2cb6
6 changed files with 113 additions and 140 deletions

View File

@ -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),
}
}
}

View File

@ -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",
),

View File

@ -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<ImplSourceBuiltinData<PredicateObligation<'tcx>>, 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<rustc_transmute::layout::rustc::Ref<'tcx>>,
) -> Result<Vec<PredicateObligation<'tcx>>, 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.

View File

@ -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<R> {
/// `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<R> = Result<Option<Condition<R>>, Reason>;
/// A condition which must hold for safe transmutation to be possible
#[derive(Debug, Hash, Eq, PartialEq, Clone)]
pub enum Condition<R> {
/// `Src` is transmutable into `Dst`, if `src` is transmutable into `dst`.
IfTransmutable { src: R, dst: R },

View File

@ -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<L, C>
@ -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<R> Answer<R>
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<R>(lhs: Answer<R>, rhs: Answer<R>) -> Answer<R> {
// 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<R, I, F>(iter: I, f: F) -> Answer<R>
where
R: layout::Ref,
I: IntoIterator,
F: FnMut(<I as IntoIterator>::Item) -> Answer<R>,
{
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<R, I, F>(iter: I, f: F) -> Answer<R>
where
R: layout::Ref,
I: IntoIterator,
F: FnMut(<I as IntoIterator>::Item) -> Answer<R>,
{
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<R>(lhs: Answer<R>, rhs: Answer<R>) -> Answer<R> {
// 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<R>, next| {
match accum.or(next) {
Answer::Yes => Break(Answer::Yes),
maybe => Continue(maybe),
}
})
}
Self::ForAll => (Answer::Yes, |accum: Answer<R>, next| match accum.and(next) {
Answer::No(reason) => Break(Answer::No(reason)),
Self::ThereExists => (Err(Reason::DstIsBitIncompatible), |accum: Answer<R>, next| {
match or(accum, next) {
Ok(None) => Break(Ok(None)),
maybe => Continue(maybe),
}
}),
Self::ForAll => (Ok(None), |accum: Answer<R>, next| match and(accum, next) {
Err(reason) => Break(Err(reason)),
maybe => Continue(maybe),
}),
};

View File

@ -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(),