From 8f1cec8d8472c3ffacedd4783c64182a407c72df Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Fri, 21 Apr 2023 16:49:36 -0700 Subject: [PATCH 1/8] Safe Transmute: Enable handling references, including recursive types This patch enables support for references in Safe Transmute, by generating nested obligations during trait selection. Specifically, when we call `confirm_transmutability_candidate(...)`, we now recursively traverse the `rustc_transmute::Answer` tree and create obligations for all the `Answer` variants, some of which include multiple nested `Answer`s. Also, to handle recursive types, enable support for coinduction for the Safe Transmute trait (`BikeshedIntrinsicFrom`) by adding the `#[rustc_coinduction]` annotation. Also fix some small logic issues when reducing the `or` and `and` combinations in `rustc_transmute`, so that we don't end up with additional redundant `Answer`s in the tree. Co-authored-by: Jack Wrenn --- .../src/traits/error_reporting/mod.rs | 10 +- .../src/traits/select/confirmation.rs | 61 ++++++- compiler/rustc_transmute/src/layout/mod.rs | 37 ++-- compiler/rustc_transmute/src/layout/tree.rs | 11 ++ compiler/rustc_transmute/src/lib.rs | 11 +- .../src/maybe_transmutable/mod.rs | 170 ++++++++++++++---- library/core/src/mem/transmutability.rs | 1 + .../should_require_well_defined_layout.stderr | 6 +- ...ve_reprs_should_have_correct_length.stderr | 20 +-- .../should_require_well_defined_layout.stderr | 6 +- .../enums/should_pad_variants.stderr | 2 +- .../recursive-wrapper-types-bit-compatible.rs | 27 +++ ...ursive-wrapper-types-bit-compatible.stderr | 25 +++ ...ecursive-wrapper-types-bit-incompatible.rs | 25 +++ ...sive-wrapper-types-bit-incompatible.stderr | 25 +++ .../references/recursive-wrapper-types.rs | 26 +++ .../u8-to-unit.rs} | 12 +- .../u8-to-unit.stderr} | 10 +- .../references/unit-to-itself.rs | 24 +++ .../transmutability/references/unit-to-u8.rs | 24 +++ .../unit-to-u8.stderr} | 10 +- tests/ui/transmutability/region-infer.stderr | 2 +- .../should_require_well_defined_layout.stderr | 12 +- .../should_require_well_defined_layout.stderr | 2 +- .../unions/should_pad_variants.stderr | 2 +- .../ui/transmute/transmute-padding-ice.stderr | 2 +- 26 files changed, 460 insertions(+), 103 deletions(-) create mode 100644 tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.rs create mode 100644 tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.stderr create mode 100644 tests/ui/transmutability/references/recursive-wrapper-types-bit-incompatible.rs create mode 100644 tests/ui/transmutability/references/recursive-wrapper-types-bit-incompatible.stderr create mode 100644 tests/ui/transmutability/references/recursive-wrapper-types.rs rename tests/ui/transmutability/{references.rs => references/u8-to-unit.rs} (55%) rename tests/ui/transmutability/{references.next.stderr => references/u8-to-unit.stderr} (63%) create mode 100644 tests/ui/transmutability/references/unit-to-itself.rs create mode 100644 tests/ui/transmutability/references/unit-to-u8.rs rename tests/ui/transmutability/{references.current.stderr => references/unit-to-u8.stderr} (76%) 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 dc43a3d154ab3..7e132e0ab60c7 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -2783,6 +2783,14 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { rustc_transmute::Reason::DstIsTooBig => { format!("The size of `{src}` is smaller than the size of `{dst}`") } + rustc_transmute::Reason::DstHasStricterAlignment => { + format!( + "The alignment of `{src}` should be stricter than that of `{dst}`, but it is not" + ) + } + rustc_transmute::Reason::DstIsMoreUnique => { + format!("`{src}` is a shared reference, but `{dst}` is a unique reference") + } }; (custom_err_msg, Some(reason_msg)) } @@ -2791,7 +2799,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { span, "Inconsistent rustc_transmute::is_transmutable(...) result, got Yes", ), - _ => span_bug!(span, "Unsupported rustc_transmute::Reason variant"), + other => span_bug!(span, "Unsupported rustc_transmute::Answer variant: `{other:?}`"), } } diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 0d9f55d4c2edf..9b28873f7099c 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -13,7 +13,7 @@ use rustc_infer::infer::{DefineOpaqueTypes, InferOk}; use rustc_middle::traits::SelectionOutputTypeParameterMismatch; use rustc_middle::ty::{ self, Binder, GenericParamDefKind, InternalSubsts, SubstsRef, ToPolyTraitRef, ToPredicate, - TraitRef, Ty, TyCtxt, TypeVisitableExt, + TraitPredicate, TraitRef, Ty, TyCtxt, TypeVisitableExt, }; use rustc_session::config::TraitSolver; use rustc_span::def_id::DefId; @@ -279,10 +279,61 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ImplSourceBuiltinData { nested: obligations } } + #[instrument(skip(self))] fn confirm_transmutability_candidate( &mut self, obligation: &TraitObligation<'tcx>, ) -> Result>, SelectionError<'tcx>> { + fn flatten_answer_tree<'tcx>( + tcx: TyCtxt<'tcx>, + obligation: &TraitObligation<'tcx>, + predicate: TraitPredicate<'tcx>, + answer: rustc_transmute::Answer>, + ) -> Result>, SelectionError<'tcx>> { + match answer { + rustc_transmute::Answer::Yes => Ok(vec![]), + rustc_transmute::Answer::No(_) => Err(Unimplemented), + // FIXME(bryangarza): Add separate `IfAny` case, instead of treating as `IfAll` + rustc_transmute::Answer::IfAll(answers) + | rustc_transmute::Answer::IfAny(answers) => { + let mut nested = vec![]; + for flattened in answers + .into_iter() + .map(|answer| flatten_answer_tree(tcx, obligation, predicate, answer)) + { + nested.extend(flattened?); + } + Ok(nested) + } + rustc_transmute::Answer::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); + let make_obl = |from_ty, to_ty| { + let trait_ref1 = tcx.mk_trait_ref( + trait_def_id, + [ + ty::GenericArg::from(to_ty), + ty::GenericArg::from(from_ty), + ty::GenericArg::from(scope), + ty::GenericArg::from(assume_const), + ], + ); + Obligation::with_depth( + tcx, + obligation.cause.clone(), + obligation.recursion_depth + 1, + obligation.param_env, + trait_ref1, + ) + }; + + // // FIXME(bryangarza): Check src.mutability or dst.mutability to know whether dst -> src obligation is needed + Ok(vec![make_obl(src.ty, dst.ty), make_obl(dst.ty, src.ty)]) + } + } + } + debug!(?obligation, "confirm_transmutability_candidate"); // We erase regions here because transmutability calls layout queries, @@ -312,10 +363,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { assume, ); - match maybe_transmutable { - rustc_transmute::Answer::Yes => Ok(ImplSourceBuiltinData { nested: vec![] }), - _ => Err(Unimplemented), - } + info!(?maybe_transmutable); + let nested = flatten_answer_tree(self.tcx(), obligation, predicate, maybe_transmutable)?; + info!(?nested); + Ok(ImplSourceBuiltinData { nested }) } /// This handles the case where an `auto trait Foo` impl is being used. diff --git a/compiler/rustc_transmute/src/layout/mod.rs b/compiler/rustc_transmute/src/layout/mod.rs index f8d05bc89d26d..b318447e581c7 100644 --- a/compiler/rustc_transmute/src/layout/mod.rs +++ b/compiler/rustc_transmute/src/layout/mod.rs @@ -30,33 +30,46 @@ impl fmt::Debug for Byte { } pub(crate) trait Def: Debug + Hash + Eq + PartialEq + Copy + Clone {} -pub trait Ref: Debug + Hash + Eq + PartialEq + Copy + Clone {} +pub trait Ref: Debug + Hash + Eq + PartialEq + Copy + Clone { + fn min_align(&self) -> usize { + 1 + } + + fn is_mutable(&self) -> bool { + false + } +} impl Def for ! {} impl Ref for ! {} #[cfg(feature = "rustc")] -pub(crate) mod rustc { +pub mod rustc { use rustc_middle::mir::Mutability; - use rustc_middle::ty; - use rustc_middle::ty::Region; - use rustc_middle::ty::Ty; + use rustc_middle::ty::{self, Ty}; /// A reference in the layout. #[derive(Debug, Hash, Eq, PartialEq, PartialOrd, Ord, Clone, Copy)] pub struct Ref<'tcx> { - lifetime: Region<'tcx>, - ty: Ty<'tcx>, - mutability: Mutability, + pub lifetime: ty::Region<'tcx>, + pub ty: Ty<'tcx>, + pub mutability: Mutability, + pub align: usize, } - impl<'tcx> super::Ref for Ref<'tcx> {} + impl<'tcx> super::Ref for Ref<'tcx> { + fn min_align(&self) -> usize { + self.align + } - impl<'tcx> Ref<'tcx> { - pub fn min_align(&self) -> usize { - todo!() + fn is_mutable(&self) -> bool { + match self.mutability { + Mutability::Mut => true, + Mutability::Not => false, + } } } + impl<'tcx> Ref<'tcx> {} /// A visibility node in the layout. #[derive(Debug, Hash, Eq, PartialEq, Clone, Copy)] diff --git a/compiler/rustc_transmute/src/layout/tree.rs b/compiler/rustc_transmute/src/layout/tree.rs index a6d88b1342ae8..ed9309b015d64 100644 --- a/compiler/rustc_transmute/src/layout/tree.rs +++ b/compiler/rustc_transmute/src/layout/tree.rs @@ -365,6 +365,17 @@ pub(crate) mod rustc { } })) } + + ty::Ref(lifetime, ty, mutability) => { + let align = layout_of(tcx, *ty)?.align(); + Ok(Tree::Ref(Ref { + lifetime: *lifetime, + ty: *ty, + mutability: *mutability, + align, + })) + } + _ => Err(Err::Unspecified), } } diff --git a/compiler/rustc_transmute/src/lib.rs b/compiler/rustc_transmute/src/lib.rs index 77c0526e3aabe..c4a99d9eb89a2 100644 --- a/compiler/rustc_transmute/src/lib.rs +++ b/compiler/rustc_transmute/src/lib.rs @@ -8,7 +8,7 @@ extern crate tracing; pub(crate) use rustc_data_structures::fx::{FxIndexMap as Map, FxIndexSet as Set}; -pub(crate) mod layout; +pub mod layout; pub(crate) mod maybe_transmutable; #[derive(Default)] @@ -21,10 +21,7 @@ pub struct Assume { /// The type encodes answers to the question: "Are these types transmutable?" #[derive(Debug, Hash, Eq, PartialEq, PartialOrd, Ord, Clone)] -pub enum Answer -where - R: layout::Ref, -{ +pub enum Answer { /// `Src` is transmutable into `Dst`. Yes, @@ -54,6 +51,10 @@ pub enum Reason { DstIsPrivate, /// `Dst` is larger than `Src`, and the excess bytes were not exclusively uninitialized. DstIsTooBig, + /// Src should have a stricter alignment than Dst, but it does not. + DstHasStricterAlignment, + /// Can't go from shared pointer to unique pointer + DstIsMoreUnique, } #[cfg(feature = "rustc")] diff --git a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs index 2e2fb90e71c1a..d1077488c79de 100644 --- a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs +++ b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs @@ -1,13 +1,13 @@ -use crate::Map; -use crate::{Answer, Reason}; - +pub(crate) mod query_context; #[cfg(test)] mod tests; -mod query_context; -use query_context::QueryContext; +use crate::{ + layout::{self, dfa, Byte, Dfa, Nfa, Ref, Tree, Uninhabited}, + maybe_transmutable::query_context::QueryContext, + Answer, Map, Reason, +}; -use crate::layout::{self, dfa, Byte, Dfa, Nfa, Tree, Uninhabited}; pub(crate) struct MaybeTransmutableQuery where C: QueryContext, @@ -53,6 +53,7 @@ where } } +// FIXME: Nix this cfg, so we can write unit tests independently of rustc #[cfg(feature = "rustc")] mod rustc { use super::*; @@ -77,12 +78,11 @@ mod rustc { 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::Unspecified), _) => Err(Answer::No(Reason::SrcIsUnspecified)), - (_, Err(Err::Unspecified)) => Err(Answer::No(Reason::DstIsUnspecified)), + (Err(Err::TypeError(_)), _) | (_, Err(Err::TypeError(_))) => Err(Answer::Yes), + (Err(Err::Unknown), _) | (_, Err(Err::Unknown)) => Err(Answer::Yes), + (Err(Err::Unspecified), _) | (_, Err(Err::Unspecified)) => { + Err(Answer::No(Reason::SrcIsUnspecified)) + } (Ok(src), Ok(dst)) => Ok((src, dst)), } }); @@ -214,34 +214,99 @@ where Answer::No(Reason::DstIsTooBig) } } else { - let src_quantification = if self.assume.validity { + let src_quantifier = if self.assume.validity { // if the compiler may assume that the programmer is doing additional validity checks, // (e.g.: that `src != 3u8` when the destination type is `bool`) // then there must exist at least one transition out of `src_state` such that the transmute is viable... - there_exists + Quantifier::ThereExists } else { // if the compiler cannot assume that the programmer is doing additional validity checks, // then for all transitions out of `src_state`, such that the transmute is viable... - // then there must exist at least one transition out of `src_state` such that the transmute is viable... - for_all + // then there must exist at least one transition out of `dst_state` such that the transmute is viable... + Quantifier::ForAll }; - src_quantification( - self.src.bytes_from(src_state).unwrap_or(&Map::default()), - |(&src_validity, &src_state_prime)| { - if let Some(dst_state_prime) = self.dst.byte_from(dst_state, src_validity) { - self.answer_memo(cache, src_state_prime, dst_state_prime) - } else if let Some(dst_state_prime) = - self.dst.byte_from(dst_state, Byte::Uninit) - { - self.answer_memo(cache, src_state_prime, dst_state_prime) - } else { - Answer::No(Reason::DstIsBitIncompatible) - } - }, - ) + let bytes_answer = src_quantifier.apply( + // for each of the byte transitions out of the `src_state`... + self.src.bytes_from(src_state).unwrap_or(&Map::default()).into_iter().map( + |(&src_validity, &src_state_prime)| { + // ...try to find a matching transition out of `dst_state`. + if let Some(dst_state_prime) = + self.dst.byte_from(dst_state, src_validity) + { + self.answer_memo(cache, src_state_prime, dst_state_prime) + } else if let Some(dst_state_prime) = + // otherwise, see if `dst_state` has any outgoing `Uninit` transitions + // (any init byte is a valid uninit byte) + self.dst.byte_from(dst_state, Byte::Uninit) + { + self.answer_memo(cache, src_state_prime, dst_state_prime) + } else { + // otherwise, we've exhausted our options. + // the DFAs, from this point onwards, are bit-incompatible. + Answer::No(Reason::DstIsBitIncompatible) + } + }, + ), + ); + + // The below early returns reflect how this code would behave: + // if self.assume.validity { + // bytes_answer.or(refs_answer) + // } else { + // bytes_answer.and(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, + _ => {} + }; + + let refs_answer = src_quantifier.apply( + // for each reference transition out of `src_state`... + self.src.refs_from(src_state).unwrap_or(&Map::default()).into_iter().map( + |(&src_ref, &src_state_prime)| { + // ...there exists a reference transition out of `dst_state`... + Quantifier::ThereExists.apply( + self.dst + .refs_from(dst_state) + .unwrap_or(&Map::default()) + .into_iter() + .map(|(&dst_ref, &dst_state_prime)| { + if !src_ref.is_mutable() && dst_ref.is_mutable() { + Answer::No(Reason::DstIsMoreUnique) + } else if !self.assume.alignment + && src_ref.min_align() < dst_ref.min_align() + { + Answer::No(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( + cache, + src_state_prime, + dst_state_prime, + )) + } + }), + ) + }, + ), + ); + + if self.assume.validity { + bytes_answer.or(refs_answer) + } else { + bytes_answer.and(refs_answer) + } }; - cache.insert((src_state, dst_state), answer.clone()); + if let Some(..) = cache.insert((src_state, dst_state), answer.clone()) { + panic!("failed to correctly cache transmutability") + } answer } } @@ -253,17 +318,21 @@ where { pub(crate) fn and(self, rhs: Self) -> Self { match (self, rhs) { - (Self::No(reason), _) | (_, Self::No(reason)) => Self::No(reason), - (Self::Yes, Self::Yes) => Self::Yes, + (_, 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]), } } @@ -271,7 +340,7 @@ where pub(crate) fn or(self, rhs: Self) -> Self { match (self, rhs) { (Self::Yes, _) | (_, Self::Yes) => Self::Yes, - (Self::No(lhr), Self::No(rhr)) => Self::No(lhr), + (other, Self::No(reason)) | (Self::No(reason), other) => other, (Self::IfAny(mut lhs), Self::IfAny(ref mut rhs)) => { lhs.append(rhs); Self::IfAny(lhs) @@ -319,3 +388,36 @@ where ); result } + +pub enum Quantifier { + ThereExists, + ForAll, +} + +impl Quantifier { + pub fn apply(&self, iter: I) -> Answer + where + R: layout::Ref, + I: IntoIterator>, + { + 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)), + maybe => Continue(maybe), + }), + }; + + let (Continue(result) | Break(result)) = iter.into_iter().try_fold(init, try_fold_f); + result + } +} diff --git a/library/core/src/mem/transmutability.rs b/library/core/src/mem/transmutability.rs index 87ae30619c63b..d0c30e715d56b 100644 --- a/library/core/src/mem/transmutability.rs +++ b/library/core/src/mem/transmutability.rs @@ -5,6 +5,7 @@ /// notwithstanding whatever safety checks you have asked the compiler to [`Assume`] are satisfied. #[unstable(feature = "transmutability", issue = "99571")] #[lang = "transmute_trait"] +#[rustc_coinductive] pub unsafe trait BikeshedIntrinsicFrom where Src: ?Sized, diff --git a/tests/ui/transmutability/arrays/should_require_well_defined_layout.stderr b/tests/ui/transmutability/arrays/should_require_well_defined_layout.stderr index 1a0a5d3ae9462..4a0cd17640801 100644 --- a/tests/ui/transmutability/arrays/should_require_well_defined_layout.stderr +++ b/tests/ui/transmutability/arrays/should_require_well_defined_layout.stderr @@ -23,7 +23,7 @@ error[E0277]: `u128` cannot be safely transmuted into `[String; 0]` in the defin --> $DIR/should_require_well_defined_layout.rs:27:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `[String; 0]` does not have a well-specified layout + | ^^^^^^^^^ `u128` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 @@ -65,7 +65,7 @@ error[E0277]: `u128` cannot be safely transmuted into `[String; 1]` in the defin --> $DIR/should_require_well_defined_layout.rs:33:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `[String; 1]` does not have a well-specified layout + | ^^^^^^^^^ `u128` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 @@ -107,7 +107,7 @@ error[E0277]: `u128` cannot be safely transmuted into `[String; 2]` in the defin --> $DIR/should_require_well_defined_layout.rs:39:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `[String; 2]` does not have a well-specified layout + | ^^^^^^^^^ `u128` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 diff --git a/tests/ui/transmutability/enums/repr/primitive_reprs_should_have_correct_length.stderr b/tests/ui/transmutability/enums/repr/primitive_reprs_should_have_correct_length.stderr index 9877a6606a9fa..77d2dc4f50ca3 100644 --- a/tests/ui/transmutability/enums/repr/primitive_reprs_should_have_correct_length.stderr +++ b/tests/ui/transmutability/enums/repr/primitive_reprs_should_have_correct_length.stderr @@ -24,7 +24,7 @@ error[E0277]: `V0i8` cannot be safely transmuted into `u16` in the defining scop --> $DIR/primitive_reprs_should_have_correct_length.rs:50:44 | LL | assert::is_transmutable::(); - | ^^^^^^ The size of `V0i8` is smaller than the size of `u16` + | ^^^^^^ At least one value of `V0i8` isn't a bit-valid value of `u16` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -68,7 +68,7 @@ error[E0277]: `V0u8` cannot be safely transmuted into `u16` in the defining scop --> $DIR/primitive_reprs_should_have_correct_length.rs:58:44 | LL | assert::is_transmutable::(); - | ^^^^^^ The size of `V0u8` is smaller than the size of `u16` + | ^^^^^^ At least one value of `V0u8` isn't a bit-valid value of `u16` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -112,7 +112,7 @@ error[E0277]: `V0i16` cannot be safely transmuted into `u32` in the defining sco --> $DIR/primitive_reprs_should_have_correct_length.rs:74:44 | LL | assert::is_transmutable::(); - | ^^^^^^ The size of `V0i16` is smaller than the size of `u32` + | ^^^^^^ At least one value of `V0i16` isn't a bit-valid value of `u32` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -156,7 +156,7 @@ error[E0277]: `V0u16` cannot be safely transmuted into `u32` in the defining sco --> $DIR/primitive_reprs_should_have_correct_length.rs:82:44 | LL | assert::is_transmutable::(); - | ^^^^^^ The size of `V0u16` is smaller than the size of `u32` + | ^^^^^^ At least one value of `V0u16` isn't a bit-valid value of `u32` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -200,7 +200,7 @@ error[E0277]: `V0i32` cannot be safely transmuted into `u64` in the defining sco --> $DIR/primitive_reprs_should_have_correct_length.rs:98:44 | LL | assert::is_transmutable::(); - | ^^^^^^ The size of `V0i32` is smaller than the size of `u64` + | ^^^^^^ At least one value of `V0i32` isn't a bit-valid value of `u64` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -244,7 +244,7 @@ error[E0277]: `V0u32` cannot be safely transmuted into `u64` in the defining sco --> $DIR/primitive_reprs_should_have_correct_length.rs:106:44 | LL | assert::is_transmutable::(); - | ^^^^^^ The size of `V0u32` is smaller than the size of `u64` + | ^^^^^^ At least one value of `V0u32` isn't a bit-valid value of `u64` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -288,7 +288,7 @@ error[E0277]: `V0i64` cannot be safely transmuted into `u128` in the defining sc --> $DIR/primitive_reprs_should_have_correct_length.rs:122:44 | LL | assert::is_transmutable::(); - | ^^^^^^ The size of `V0i64` is smaller than the size of `u128` + | ^^^^^^ At least one value of `V0i64` isn't a bit-valid value of `u128` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -332,7 +332,7 @@ error[E0277]: `V0u64` cannot be safely transmuted into `u128` in the defining sc --> $DIR/primitive_reprs_should_have_correct_length.rs:130:44 | LL | assert::is_transmutable::(); - | ^^^^^^ The size of `V0u64` is smaller than the size of `u128` + | ^^^^^^ At least one value of `V0u64` isn't a bit-valid value of `u128` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -376,7 +376,7 @@ error[E0277]: `V0isize` cannot be safely transmuted into `[usize; 2]` in the def --> $DIR/primitive_reprs_should_have_correct_length.rs:146:44 | LL | assert::is_transmutable::(); - | ^^^^^^ The size of `V0isize` is smaller than the size of `[usize; 2]` + | ^^^^^^ At least one value of `V0isize` isn't a bit-valid value of `[usize; 2]` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -420,7 +420,7 @@ error[E0277]: `V0usize` cannot be safely transmuted into `[usize; 2]` in the def --> $DIR/primitive_reprs_should_have_correct_length.rs:154:44 | LL | assert::is_transmutable::(); - | ^^^^^^ The size of `V0usize` is smaller than the size of `[usize; 2]` + | ^^^^^^ At least one value of `V0usize` isn't a bit-valid value of `[usize; 2]` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 diff --git a/tests/ui/transmutability/enums/repr/should_require_well_defined_layout.stderr b/tests/ui/transmutability/enums/repr/should_require_well_defined_layout.stderr index 1612b6b3661ef..6508d535b05c3 100644 --- a/tests/ui/transmutability/enums/repr/should_require_well_defined_layout.stderr +++ b/tests/ui/transmutability/enums/repr/should_require_well_defined_layout.stderr @@ -24,7 +24,7 @@ error[E0277]: `u128` cannot be safely transmuted into `void::repr_rust` in the d --> $DIR/should_require_well_defined_layout.rs:29:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `void::repr_rust` does not have a well-specified layout + | ^^^^^^^^^ `u128` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:14:14 @@ -68,7 +68,7 @@ error[E0277]: `u128` cannot be safely transmuted into `singleton::repr_rust` in --> $DIR/should_require_well_defined_layout.rs:35:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `singleton::repr_rust` does not have a well-specified layout + | ^^^^^^^^^ `u128` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:14:14 @@ -112,7 +112,7 @@ error[E0277]: `u128` cannot be safely transmuted into `duplex::repr_rust` in the --> $DIR/should_require_well_defined_layout.rs:41:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `duplex::repr_rust` does not have a well-specified layout + | ^^^^^^^^^ `u128` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:14:14 diff --git a/tests/ui/transmutability/enums/should_pad_variants.stderr b/tests/ui/transmutability/enums/should_pad_variants.stderr index bfbef8b25fcf4..c82a79680225d 100644 --- a/tests/ui/transmutability/enums/should_pad_variants.stderr +++ b/tests/ui/transmutability/enums/should_pad_variants.stderr @@ -2,7 +2,7 @@ error[E0277]: `Src` cannot be safely transmuted into `Dst` in the defining scope --> $DIR/should_pad_variants.rs:44:36 | LL | assert::is_transmutable::(); - | ^^^ The size of `Src` is smaller than the size of `Dst` + | ^^^ At least one value of `Src` isn't a bit-valid value of `Dst` | note: required by a bound in `is_transmutable` --> $DIR/should_pad_variants.rs:13:14 diff --git a/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.rs b/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.rs new file mode 100644 index 0000000000000..918147a086211 --- /dev/null +++ b/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.rs @@ -0,0 +1,27 @@ +// check-fail +#![feature(transmutability)] + +mod assert { + use std::mem::{Assume, BikeshedIntrinsicFrom}; + pub struct Context; + + pub fn is_maybe_transmutable() + where + Dst: BikeshedIntrinsicFrom + {} +} + +fn main() { + #[repr(C)] struct A(bool, &'static A); + #[repr(C)] struct B(u8, &'static B); + // FIXME(bryangarza): Make 2 variants of this test, depending on mutability. + // Right now, we are being strict by default and checking A->B and B->A both. + assert::is_maybe_transmutable::<&'static A, &'static B>(); //~ ERROR `B` cannot be safely transmuted into `A` +} diff --git a/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.stderr b/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.stderr new file mode 100644 index 0000000000000..fac0e4f032e00 --- /dev/null +++ b/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.stderr @@ -0,0 +1,25 @@ +error[E0277]: `B` cannot be safely transmuted into `A` in the defining scope of `assert::Context` + --> $DIR/recursive-wrapper-types-bit-compatible.rs:26:49 + | +LL | assert::is_maybe_transmutable::<&'static A, &'static B>(); + | ^^^^^^^^^^ At least one value of `B` isn't a bit-valid value of `A` + | +note: required by a bound in `is_maybe_transmutable` + --> $DIR/recursive-wrapper-types-bit-compatible.rs:10:14 + | +LL | pub fn is_maybe_transmutable() + | --------------------- required by a bound in this function +LL | where +LL | Dst: BikeshedIntrinsicFrom + | |__________^ required by this bound in `is_maybe_transmutable` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/transmutability/references/recursive-wrapper-types-bit-incompatible.rs b/tests/ui/transmutability/references/recursive-wrapper-types-bit-incompatible.rs new file mode 100644 index 0000000000000..6dcb7df9feb1c --- /dev/null +++ b/tests/ui/transmutability/references/recursive-wrapper-types-bit-incompatible.rs @@ -0,0 +1,25 @@ +// check-fail +#![feature(transmutability)] + +mod assert { + use std::mem::{Assume, BikeshedIntrinsicFrom}; + pub struct Context; + + pub fn is_maybe_transmutable() + where + Dst: BikeshedIntrinsicFrom + {} +} + +fn main() { + #[repr(C)] struct A(bool, &'static A); + #[repr(C)] struct B(u8, &'static B); + assert::is_maybe_transmutable::<&'static B, &'static A>(); //~ ERROR `B` cannot be safely transmuted into `A` +} diff --git a/tests/ui/transmutability/references/recursive-wrapper-types-bit-incompatible.stderr b/tests/ui/transmutability/references/recursive-wrapper-types-bit-incompatible.stderr new file mode 100644 index 0000000000000..ecfe4865962f7 --- /dev/null +++ b/tests/ui/transmutability/references/recursive-wrapper-types-bit-incompatible.stderr @@ -0,0 +1,25 @@ +error[E0277]: `B` cannot be safely transmuted into `A` in the defining scope of `assert::Context` + --> $DIR/recursive-wrapper-types-bit-incompatible.rs:24:49 + | +LL | assert::is_maybe_transmutable::<&'static B, &'static A>(); + | ^^^^^^^^^^ At least one value of `B` isn't a bit-valid value of `A` + | +note: required by a bound in `is_maybe_transmutable` + --> $DIR/recursive-wrapper-types-bit-incompatible.rs:10:14 + | +LL | pub fn is_maybe_transmutable() + | --------------------- required by a bound in this function +LL | where +LL | Dst: BikeshedIntrinsicFrom + | |__________^ required by this bound in `is_maybe_transmutable` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/transmutability/references/recursive-wrapper-types.rs b/tests/ui/transmutability/references/recursive-wrapper-types.rs new file mode 100644 index 0000000000000..090c1fea6dbdf --- /dev/null +++ b/tests/ui/transmutability/references/recursive-wrapper-types.rs @@ -0,0 +1,26 @@ +// check-pass +#![feature(transmutability)] + +mod assert { + use std::mem::{Assume, BikeshedIntrinsicFrom}; + pub struct Context; + + pub fn is_maybe_transmutable() + where + Dst: BikeshedIntrinsicFrom + {} +} + +fn main() { + #[repr(C)] struct A(&'static B); + #[repr(C)] struct B(&'static A); + assert::is_maybe_transmutable::<&'static A, &'static B>(); + assert::is_maybe_transmutable::<&'static B, &'static A>(); +} diff --git a/tests/ui/transmutability/references.rs b/tests/ui/transmutability/references/u8-to-unit.rs similarity index 55% rename from tests/ui/transmutability/references.rs rename to tests/ui/transmutability/references/u8-to-unit.rs index 8c2b25ebba1e3..b7dd638b952ad 100644 --- a/tests/ui/transmutability/references.rs +++ b/tests/ui/transmutability/references/u8-to-unit.rs @@ -1,11 +1,5 @@ -// revisions: current next -//[next] compile-flags: -Ztrait-solver=next - -//! Transmutations involving references are not yet supported. - -#![crate_type = "lib"] +// check-fail #![feature(transmutability)] -#![allow(dead_code, incomplete_features, non_camel_case_types)] mod assert { use std::mem::{Assume, BikeshedIntrinsicFrom}; @@ -24,7 +18,7 @@ mod assert { {} } -fn not_yet_implemented() { +fn main() { #[repr(C)] struct Unit; - assert::is_maybe_transmutable::<&'static Unit, &'static Unit>(); //~ ERROR cannot be safely transmuted + assert::is_maybe_transmutable::<&'static u8, &'static Unit>(); //~ ERROR `Unit` cannot be safely transmuted into `u8` } diff --git a/tests/ui/transmutability/references.next.stderr b/tests/ui/transmutability/references/u8-to-unit.stderr similarity index 63% rename from tests/ui/transmutability/references.next.stderr rename to tests/ui/transmutability/references/u8-to-unit.stderr index 819c9b92bc81a..81b0b57f0cf4b 100644 --- a/tests/ui/transmutability/references.next.stderr +++ b/tests/ui/transmutability/references/u8-to-unit.stderr @@ -1,11 +1,11 @@ -error[E0277]: `&Unit` cannot be safely transmuted into `&Unit` in the defining scope of `assert::Context` - --> $DIR/references.rs:29:52 +error[E0277]: `Unit` cannot be safely transmuted into `u8` in the defining scope of `assert::Context` + --> $DIR/u8-to-unit.rs:23:50 | -LL | assert::is_maybe_transmutable::<&'static Unit, &'static Unit>(); - | ^^^^^^^^^^^^^ `&Unit` does not have a well-specified layout +LL | assert::is_maybe_transmutable::<&'static u8, &'static Unit>(); + | ^^^^^^^^^^^^^ The size of `Unit` is smaller than the size of `u8` | note: required by a bound in `is_maybe_transmutable` - --> $DIR/references.rs:16:14 + --> $DIR/u8-to-unit.rs:10:14 | LL | pub fn is_maybe_transmutable() | --------------------- required by a bound in this function diff --git a/tests/ui/transmutability/references/unit-to-itself.rs b/tests/ui/transmutability/references/unit-to-itself.rs new file mode 100644 index 0000000000000..04a7e16d7cccc --- /dev/null +++ b/tests/ui/transmutability/references/unit-to-itself.rs @@ -0,0 +1,24 @@ +// check-pass +#![feature(transmutability)] + +mod assert { + use std::mem::{Assume, BikeshedIntrinsicFrom}; + pub struct Context; + + pub fn is_maybe_transmutable() + where + Dst: BikeshedIntrinsicFrom + {} +} + +fn main() { + #[repr(C)] struct Unit; + assert::is_maybe_transmutable::<&'static Unit, &'static Unit>(); +} diff --git a/tests/ui/transmutability/references/unit-to-u8.rs b/tests/ui/transmutability/references/unit-to-u8.rs new file mode 100644 index 0000000000000..73e1db3d2d58b --- /dev/null +++ b/tests/ui/transmutability/references/unit-to-u8.rs @@ -0,0 +1,24 @@ +// check-fail +#![feature(transmutability)] + +mod assert { + use std::mem::{Assume, BikeshedIntrinsicFrom}; + pub struct Context; + + pub fn is_maybe_transmutable() + where + Dst: BikeshedIntrinsicFrom + {} +} + +fn main() { + #[repr(C)] struct Unit; + assert::is_maybe_transmutable::<&'static Unit, &'static u8>(); //~ ERROR `Unit` cannot be safely transmuted into `u8` +} diff --git a/tests/ui/transmutability/references.current.stderr b/tests/ui/transmutability/references/unit-to-u8.stderr similarity index 76% rename from tests/ui/transmutability/references.current.stderr rename to tests/ui/transmutability/references/unit-to-u8.stderr index 819c9b92bc81a..f2b72357f792b 100644 --- a/tests/ui/transmutability/references.current.stderr +++ b/tests/ui/transmutability/references/unit-to-u8.stderr @@ -1,11 +1,11 @@ -error[E0277]: `&Unit` cannot be safely transmuted into `&Unit` in the defining scope of `assert::Context` - --> $DIR/references.rs:29:52 +error[E0277]: `Unit` cannot be safely transmuted into `u8` in the defining scope of `assert::Context` + --> $DIR/unit-to-u8.rs:23:52 | -LL | assert::is_maybe_transmutable::<&'static Unit, &'static Unit>(); - | ^^^^^^^^^^^^^ `&Unit` does not have a well-specified layout +LL | assert::is_maybe_transmutable::<&'static Unit, &'static u8>(); + | ^^^^^^^^^^^ The size of `Unit` is smaller than the size of `u8` | note: required by a bound in `is_maybe_transmutable` - --> $DIR/references.rs:16:14 + --> $DIR/unit-to-u8.rs:10:14 | LL | pub fn is_maybe_transmutable() | --------------------- required by a bound in this function diff --git a/tests/ui/transmutability/region-infer.stderr b/tests/ui/transmutability/region-infer.stderr index d6b65e9e4a08b..307d0dfe50d22 100644 --- a/tests/ui/transmutability/region-infer.stderr +++ b/tests/ui/transmutability/region-infer.stderr @@ -2,7 +2,7 @@ error[E0277]: `()` cannot be safely transmuted into `W<'_>` in the defining scop --> $DIR/region-infer.rs:20:5 | LL | test(); - | ^^^^ `W<'_>` does not have a well-specified layout + | ^^^^ The size of `()` is smaller than the size of `W<'_>` | note: required by a bound in `test` --> $DIR/region-infer.rs:11:12 diff --git a/tests/ui/transmutability/structs/repr/should_require_well_defined_layout.stderr b/tests/ui/transmutability/structs/repr/should_require_well_defined_layout.stderr index 4c5062cd3b303..7f26bc498176c 100644 --- a/tests/ui/transmutability/structs/repr/should_require_well_defined_layout.stderr +++ b/tests/ui/transmutability/structs/repr/should_require_well_defined_layout.stderr @@ -24,7 +24,7 @@ error[E0277]: `u128` cannot be safely transmuted into `should_reject_repr_rust:: --> $DIR/should_require_well_defined_layout.rs:29:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `should_reject_repr_rust::unit::repr_rust` does not have a well-specified layout + | ^^^^^^^^^ `u128` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 @@ -68,7 +68,7 @@ error[E0277]: `u128` cannot be safely transmuted into `should_reject_repr_rust:: --> $DIR/should_require_well_defined_layout.rs:35:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `should_reject_repr_rust::tuple::repr_rust` does not have a well-specified layout + | ^^^^^^^^^ `u128` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 @@ -112,7 +112,7 @@ error[E0277]: `u128` cannot be safely transmuted into `should_reject_repr_rust:: --> $DIR/should_require_well_defined_layout.rs:41:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `should_reject_repr_rust::braces::repr_rust` does not have a well-specified layout + | ^^^^^^^^^ `u128` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 @@ -156,7 +156,7 @@ error[E0277]: `u128` cannot be safely transmuted into `aligned::repr_rust` in th --> $DIR/should_require_well_defined_layout.rs:47:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `aligned::repr_rust` does not have a well-specified layout + | ^^^^^^^^^ `u128` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 @@ -200,7 +200,7 @@ error[E0277]: `u128` cannot be safely transmuted into `packed::repr_rust` in the --> $DIR/should_require_well_defined_layout.rs:53:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `packed::repr_rust` does not have a well-specified layout + | ^^^^^^^^^ `u128` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 @@ -244,7 +244,7 @@ error[E0277]: `u128` cannot be safely transmuted into `nested::repr_c` in the de --> $DIR/should_require_well_defined_layout.rs:60:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^ `nested::repr_c` does not have a well-specified layout + | ^^^^^^ `u128` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 diff --git a/tests/ui/transmutability/unions/repr/should_require_well_defined_layout.stderr b/tests/ui/transmutability/unions/repr/should_require_well_defined_layout.stderr index 4293d34f47b23..305ecc71733d3 100644 --- a/tests/ui/transmutability/unions/repr/should_require_well_defined_layout.stderr +++ b/tests/ui/transmutability/unions/repr/should_require_well_defined_layout.stderr @@ -24,7 +24,7 @@ error[E0277]: `u128` cannot be safely transmuted into `should_reject_repr_rust:: --> $DIR/should_require_well_defined_layout.rs:31:43 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `should_reject_repr_rust::repr_rust` does not have a well-specified layout + | ^^^^^^^^^ `u128` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 diff --git a/tests/ui/transmutability/unions/should_pad_variants.stderr b/tests/ui/transmutability/unions/should_pad_variants.stderr index bfbef8b25fcf4..c82a79680225d 100644 --- a/tests/ui/transmutability/unions/should_pad_variants.stderr +++ b/tests/ui/transmutability/unions/should_pad_variants.stderr @@ -2,7 +2,7 @@ error[E0277]: `Src` cannot be safely transmuted into `Dst` in the defining scope --> $DIR/should_pad_variants.rs:44:36 | LL | assert::is_transmutable::(); - | ^^^ The size of `Src` is smaller than the size of `Dst` + | ^^^ At least one value of `Src` isn't a bit-valid value of `Dst` | note: required by a bound in `is_transmutable` --> $DIR/should_pad_variants.rs:13:14 diff --git a/tests/ui/transmute/transmute-padding-ice.stderr b/tests/ui/transmute/transmute-padding-ice.stderr index f5480e0b9fb82..2739fd16645ab 100644 --- a/tests/ui/transmute/transmute-padding-ice.stderr +++ b/tests/ui/transmute/transmute-padding-ice.stderr @@ -2,7 +2,7 @@ error[E0277]: `B` cannot be safely transmuted into `A` in the defining scope of --> $DIR/transmute-padding-ice.rs:27:40 | LL | assert::is_maybe_transmutable::(); - | ^ The size of `B` is smaller than the size of `A` + | ^ At least one value of `B` isn't a bit-valid value of `A` | note: required by a bound in `is_maybe_transmutable` --> $DIR/transmute-padding-ice.rs:11:14 From 263a4f2cb6b455f9c4ae46493d59369c378a85ea Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Thu, 27 Apr 2023 14:38:32 -0700 Subject: [PATCH 2/8] 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 --- .../src/solve/eval_ctxt.rs | 10 +- .../src/traits/error_reporting/mod.rs | 5 +- .../src/traits/select/confirmation.rs | 31 ++- compiler/rustc_transmute/src/lib.rs | 13 +- .../src/maybe_transmutable/mod.rs | 182 ++++++++---------- .../src/maybe_transmutable/tests.rs | 12 +- 6 files changed, 113 insertions(+), 140 deletions(-) diff --git a/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs b/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs index 63a73f8d50d93..e8abd5964bbf3 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 7e132e0ab60c7..71419fe0eee0b 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 9b28873f7099c..528a5f9dc616a 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 c4a99d9eb89a2..baf63e6d3a225 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 d1077488c79de..de00efb16148f 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 a8675f4ae37d8..df6a83df23f20 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(), From 94ad084ac611b7c21d4f08b1eb30623f09db2cd0 Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Thu, 27 Apr 2023 17:19:16 -0700 Subject: [PATCH 3/8] Safe Transmute: Fix propagation of errors - Make sure that the most specific Reason is the one that bubbles up when we are folding over the `Answer` tree. `Reason::DstIsBitIncompatible` is the least specific, so that should be used only when there isn't anything else available. - Small fixes where we used the wrong Reason variant. - Tiny cleanups --- .../src/traits/error_reporting/mod.rs | 4 +- .../src/traits/select/confirmation.rs | 2 +- compiler/rustc_transmute/src/lib.rs | 2 +- .../src/maybe_transmutable/mod.rs | 43 +++++++++++++------ .../should_require_well_defined_layout.stderr | 6 +-- ...ve_reprs_should_have_correct_length.stderr | 36 ++++++++-------- .../should_require_well_defined_layout.stderr | 6 +-- .../enums/should_pad_variants.stderr | 2 +- .../should_require_well_defined_layout.stderr | 12 +++--- .../should_require_well_defined_layout.stderr | 2 +- .../unions/should_pad_variants.stderr | 2 +- .../ui/transmute/transmute-padding-ice.stderr | 2 +- 12 files changed, 67 insertions(+), 52 deletions(-) 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 71419fe0eee0b..8224fdf591afb 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -2751,7 +2751,8 @@ 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 + // FIXME(bryangarza): Is this enough, or should we resolve all nested + // obligations like we do for `confirm_transmutability_candidate(...)?` match rustc_transmute::TransmuteTypeEnv::new(self.infcx).is_transmutable( obligation.cause, src_and_dst, @@ -2780,7 +2781,6 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { rustc_transmute::Reason::DstIsPrivate => format!( "`{dst}` is or contains a type or field that is not visible in that scope" ), - // FIXME(bryangarza): Include the number of bytes of src and dst rustc_transmute::Reason::DstIsTooBig => { format!("The size of `{src}` is smaller than the size of `{dst}`") } diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 528a5f9dc616a..e3d982b5c3f8b 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -329,7 +329,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ) }; - // // FIXME(bryangarza): Check src.mutability or dst.mutability to know whether dst -> src obligation is needed + // FIXME(bryangarza): Check src.mutability or dst.mutability to know whether dst -> src obligation is needed Ok(vec![make_obl(src.ty, dst.ty), make_obl(dst.ty, src.ty)]) } } diff --git a/compiler/rustc_transmute/src/lib.rs b/compiler/rustc_transmute/src/lib.rs index baf63e6d3a225..60adbc1b470c8 100644 --- a/compiler/rustc_transmute/src/lib.rs +++ b/compiler/rustc_transmute/src/lib.rs @@ -1,4 +1,4 @@ -#![feature(alloc_layout_extra, decl_macro, iterator_try_reduce, never_type)] +#![feature(alloc_layout_extra, decl_macro, iterator_try_reduce, never_type, let_chains)] #![allow(dead_code, unused_variables)] #![deny(rustc::untranslatable_diagnostic)] #![deny(rustc::diagnostic_outside_of_impl)] diff --git a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs index de00efb16148f..47f61a808403b 100644 --- a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs +++ b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs @@ -78,11 +78,12 @@ mod rustc { match (src, dst) { // 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(Ok(None)), - (Err(Err::Unknown), _) | (_, Err(Err::Unknown)) => Err(Ok(None)), - (Err(Err::Unspecified), _) | (_, Err(Err::Unspecified)) => { - Err(Err(Reason::SrcIsUnspecified)) - } + (Err(Err::TypeError(_)), _) + | (_, Err(Err::TypeError(_))) + | (Err(Err::Unknown), _) + | (_, Err(Err::Unknown)) => Err(Ok(None)), + (Err(Err::Unspecified), _) => Err(Err(Reason::SrcIsUnspecified)), + (_, Err(Err::Unspecified)) => Err(Err(Reason::DstIsUnspecified)), (Ok(src), Ok(dst)) => Ok((src, dst)), } }); @@ -316,12 +317,19 @@ where } } -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) { +fn and(lhs: Answer, rhs: Answer) -> Answer +where + R: PartialEq, +{ + // If both are errors, then we should return the more specific one + if lhs.is_err() && rhs.is_err() { + if lhs == Err(Reason::DstIsBitIncompatible) { + return rhs; + } else { + return 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 @@ -340,10 +348,17 @@ fn and(lhs: Answer, rhs: Answer) -> Answer { }) } -fn or(lhs: Answer, rhs: Answer) -> Answer { - // If both are errors, then we should return the one on the right +fn or(lhs: Answer, rhs: Answer) -> Answer +where + R: PartialEq, +{ + // If both are errors, then we should return the more specific one if lhs.is_err() && rhs.is_err() { - return rhs; + if lhs == Err(Reason::DstIsBitIncompatible) { + return rhs; + } else { + return lhs; + } } // Otherwise, errors can be ignored for the rest of the pattern matching let lhs = lhs.unwrap_or(None); diff --git a/tests/ui/transmutability/arrays/should_require_well_defined_layout.stderr b/tests/ui/transmutability/arrays/should_require_well_defined_layout.stderr index 4a0cd17640801..1a0a5d3ae9462 100644 --- a/tests/ui/transmutability/arrays/should_require_well_defined_layout.stderr +++ b/tests/ui/transmutability/arrays/should_require_well_defined_layout.stderr @@ -23,7 +23,7 @@ error[E0277]: `u128` cannot be safely transmuted into `[String; 0]` in the defin --> $DIR/should_require_well_defined_layout.rs:27:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `u128` does not have a well-specified layout + | ^^^^^^^^^ `[String; 0]` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 @@ -65,7 +65,7 @@ error[E0277]: `u128` cannot be safely transmuted into `[String; 1]` in the defin --> $DIR/should_require_well_defined_layout.rs:33:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `u128` does not have a well-specified layout + | ^^^^^^^^^ `[String; 1]` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 @@ -107,7 +107,7 @@ error[E0277]: `u128` cannot be safely transmuted into `[String; 2]` in the defin --> $DIR/should_require_well_defined_layout.rs:39:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `u128` does not have a well-specified layout + | ^^^^^^^^^ `[String; 2]` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 diff --git a/tests/ui/transmutability/enums/repr/primitive_reprs_should_have_correct_length.stderr b/tests/ui/transmutability/enums/repr/primitive_reprs_should_have_correct_length.stderr index 77d2dc4f50ca3..46cdaa925630a 100644 --- a/tests/ui/transmutability/enums/repr/primitive_reprs_should_have_correct_length.stderr +++ b/tests/ui/transmutability/enums/repr/primitive_reprs_should_have_correct_length.stderr @@ -24,7 +24,7 @@ error[E0277]: `V0i8` cannot be safely transmuted into `u16` in the defining scop --> $DIR/primitive_reprs_should_have_correct_length.rs:50:44 | LL | assert::is_transmutable::(); - | ^^^^^^ At least one value of `V0i8` isn't a bit-valid value of `u16` + | ^^^^^^ The size of `V0i8` is smaller than the size of `u16` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -68,7 +68,7 @@ error[E0277]: `V0u8` cannot be safely transmuted into `u16` in the defining scop --> $DIR/primitive_reprs_should_have_correct_length.rs:58:44 | LL | assert::is_transmutable::(); - | ^^^^^^ At least one value of `V0u8` isn't a bit-valid value of `u16` + | ^^^^^^ The size of `V0u8` is smaller than the size of `u16` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -90,7 +90,7 @@ error[E0277]: `u8` cannot be safely transmuted into `V0i16` in the defining scop --> $DIR/primitive_reprs_should_have_correct_length.rs:72:44 | LL | assert::is_transmutable::(); - | ^^^^^^^ At least one value of `u8` isn't a bit-valid value of `V0i16` + | ^^^^^^^ The size of `u8` is smaller than the size of `V0i16` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -112,7 +112,7 @@ error[E0277]: `V0i16` cannot be safely transmuted into `u32` in the defining sco --> $DIR/primitive_reprs_should_have_correct_length.rs:74:44 | LL | assert::is_transmutable::(); - | ^^^^^^ At least one value of `V0i16` isn't a bit-valid value of `u32` + | ^^^^^^ The size of `V0i16` is smaller than the size of `u32` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -134,7 +134,7 @@ error[E0277]: `u8` cannot be safely transmuted into `V0u16` in the defining scop --> $DIR/primitive_reprs_should_have_correct_length.rs:80:44 | LL | assert::is_transmutable::(); - | ^^^^^^^ At least one value of `u8` isn't a bit-valid value of `V0u16` + | ^^^^^^^ The size of `u8` is smaller than the size of `V0u16` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -156,7 +156,7 @@ error[E0277]: `V0u16` cannot be safely transmuted into `u32` in the defining sco --> $DIR/primitive_reprs_should_have_correct_length.rs:82:44 | LL | assert::is_transmutable::(); - | ^^^^^^ At least one value of `V0u16` isn't a bit-valid value of `u32` + | ^^^^^^ The size of `V0u16` is smaller than the size of `u32` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -178,7 +178,7 @@ error[E0277]: `u16` cannot be safely transmuted into `V0i32` in the defining sco --> $DIR/primitive_reprs_should_have_correct_length.rs:96:44 | LL | assert::is_transmutable::(); - | ^^^^^^^ At least one value of `u16` isn't a bit-valid value of `V0i32` + | ^^^^^^^ The size of `u16` is smaller than the size of `V0i32` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -200,7 +200,7 @@ error[E0277]: `V0i32` cannot be safely transmuted into `u64` in the defining sco --> $DIR/primitive_reprs_should_have_correct_length.rs:98:44 | LL | assert::is_transmutable::(); - | ^^^^^^ At least one value of `V0i32` isn't a bit-valid value of `u64` + | ^^^^^^ The size of `V0i32` is smaller than the size of `u64` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -222,7 +222,7 @@ error[E0277]: `u16` cannot be safely transmuted into `V0u32` in the defining sco --> $DIR/primitive_reprs_should_have_correct_length.rs:104:44 | LL | assert::is_transmutable::(); - | ^^^^^^^ At least one value of `u16` isn't a bit-valid value of `V0u32` + | ^^^^^^^ The size of `u16` is smaller than the size of `V0u32` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -244,7 +244,7 @@ error[E0277]: `V0u32` cannot be safely transmuted into `u64` in the defining sco --> $DIR/primitive_reprs_should_have_correct_length.rs:106:44 | LL | assert::is_transmutable::(); - | ^^^^^^ At least one value of `V0u32` isn't a bit-valid value of `u64` + | ^^^^^^ The size of `V0u32` is smaller than the size of `u64` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -266,7 +266,7 @@ error[E0277]: `u32` cannot be safely transmuted into `V0i64` in the defining sco --> $DIR/primitive_reprs_should_have_correct_length.rs:120:44 | LL | assert::is_transmutable::(); - | ^^^^^^^ At least one value of `u32` isn't a bit-valid value of `V0i64` + | ^^^^^^^ The size of `u32` is smaller than the size of `V0i64` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -288,7 +288,7 @@ error[E0277]: `V0i64` cannot be safely transmuted into `u128` in the defining sc --> $DIR/primitive_reprs_should_have_correct_length.rs:122:44 | LL | assert::is_transmutable::(); - | ^^^^^^ At least one value of `V0i64` isn't a bit-valid value of `u128` + | ^^^^^^ The size of `V0i64` is smaller than the size of `u128` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -310,7 +310,7 @@ error[E0277]: `u32` cannot be safely transmuted into `V0u64` in the defining sco --> $DIR/primitive_reprs_should_have_correct_length.rs:128:44 | LL | assert::is_transmutable::(); - | ^^^^^^^ At least one value of `u32` isn't a bit-valid value of `V0u64` + | ^^^^^^^ The size of `u32` is smaller than the size of `V0u64` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -332,7 +332,7 @@ error[E0277]: `V0u64` cannot be safely transmuted into `u128` in the defining sc --> $DIR/primitive_reprs_should_have_correct_length.rs:130:44 | LL | assert::is_transmutable::(); - | ^^^^^^ At least one value of `V0u64` isn't a bit-valid value of `u128` + | ^^^^^^ The size of `V0u64` is smaller than the size of `u128` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -354,7 +354,7 @@ error[E0277]: `u8` cannot be safely transmuted into `V0isize` in the defining sc --> $DIR/primitive_reprs_should_have_correct_length.rs:144:44 | LL | assert::is_transmutable::(); - | ^^^^^^^ At least one value of `u8` isn't a bit-valid value of `V0isize` + | ^^^^^^^ The size of `u8` is smaller than the size of `V0isize` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -376,7 +376,7 @@ error[E0277]: `V0isize` cannot be safely transmuted into `[usize; 2]` in the def --> $DIR/primitive_reprs_should_have_correct_length.rs:146:44 | LL | assert::is_transmutable::(); - | ^^^^^^ At least one value of `V0isize` isn't a bit-valid value of `[usize; 2]` + | ^^^^^^ The size of `V0isize` is smaller than the size of `[usize; 2]` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -398,7 +398,7 @@ error[E0277]: `u8` cannot be safely transmuted into `V0usize` in the defining sc --> $DIR/primitive_reprs_should_have_correct_length.rs:152:44 | LL | assert::is_transmutable::(); - | ^^^^^^^ At least one value of `u8` isn't a bit-valid value of `V0usize` + | ^^^^^^^ The size of `u8` is smaller than the size of `V0usize` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 @@ -420,7 +420,7 @@ error[E0277]: `V0usize` cannot be safely transmuted into `[usize; 2]` in the def --> $DIR/primitive_reprs_should_have_correct_length.rs:154:44 | LL | assert::is_transmutable::(); - | ^^^^^^ At least one value of `V0usize` isn't a bit-valid value of `[usize; 2]` + | ^^^^^^ The size of `V0usize` is smaller than the size of `[usize; 2]` | note: required by a bound in `is_transmutable` --> $DIR/primitive_reprs_should_have_correct_length.rs:12:14 diff --git a/tests/ui/transmutability/enums/repr/should_require_well_defined_layout.stderr b/tests/ui/transmutability/enums/repr/should_require_well_defined_layout.stderr index 6508d535b05c3..1612b6b3661ef 100644 --- a/tests/ui/transmutability/enums/repr/should_require_well_defined_layout.stderr +++ b/tests/ui/transmutability/enums/repr/should_require_well_defined_layout.stderr @@ -24,7 +24,7 @@ error[E0277]: `u128` cannot be safely transmuted into `void::repr_rust` in the d --> $DIR/should_require_well_defined_layout.rs:29:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `u128` does not have a well-specified layout + | ^^^^^^^^^ `void::repr_rust` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:14:14 @@ -68,7 +68,7 @@ error[E0277]: `u128` cannot be safely transmuted into `singleton::repr_rust` in --> $DIR/should_require_well_defined_layout.rs:35:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `u128` does not have a well-specified layout + | ^^^^^^^^^ `singleton::repr_rust` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:14:14 @@ -112,7 +112,7 @@ error[E0277]: `u128` cannot be safely transmuted into `duplex::repr_rust` in the --> $DIR/should_require_well_defined_layout.rs:41:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `u128` does not have a well-specified layout + | ^^^^^^^^^ `duplex::repr_rust` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:14:14 diff --git a/tests/ui/transmutability/enums/should_pad_variants.stderr b/tests/ui/transmutability/enums/should_pad_variants.stderr index c82a79680225d..bfbef8b25fcf4 100644 --- a/tests/ui/transmutability/enums/should_pad_variants.stderr +++ b/tests/ui/transmutability/enums/should_pad_variants.stderr @@ -2,7 +2,7 @@ error[E0277]: `Src` cannot be safely transmuted into `Dst` in the defining scope --> $DIR/should_pad_variants.rs:44:36 | LL | assert::is_transmutable::(); - | ^^^ At least one value of `Src` isn't a bit-valid value of `Dst` + | ^^^ The size of `Src` is smaller than the size of `Dst` | note: required by a bound in `is_transmutable` --> $DIR/should_pad_variants.rs:13:14 diff --git a/tests/ui/transmutability/structs/repr/should_require_well_defined_layout.stderr b/tests/ui/transmutability/structs/repr/should_require_well_defined_layout.stderr index 7f26bc498176c..4c5062cd3b303 100644 --- a/tests/ui/transmutability/structs/repr/should_require_well_defined_layout.stderr +++ b/tests/ui/transmutability/structs/repr/should_require_well_defined_layout.stderr @@ -24,7 +24,7 @@ error[E0277]: `u128` cannot be safely transmuted into `should_reject_repr_rust:: --> $DIR/should_require_well_defined_layout.rs:29:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `u128` does not have a well-specified layout + | ^^^^^^^^^ `should_reject_repr_rust::unit::repr_rust` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 @@ -68,7 +68,7 @@ error[E0277]: `u128` cannot be safely transmuted into `should_reject_repr_rust:: --> $DIR/should_require_well_defined_layout.rs:35:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `u128` does not have a well-specified layout + | ^^^^^^^^^ `should_reject_repr_rust::tuple::repr_rust` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 @@ -112,7 +112,7 @@ error[E0277]: `u128` cannot be safely transmuted into `should_reject_repr_rust:: --> $DIR/should_require_well_defined_layout.rs:41:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `u128` does not have a well-specified layout + | ^^^^^^^^^ `should_reject_repr_rust::braces::repr_rust` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 @@ -156,7 +156,7 @@ error[E0277]: `u128` cannot be safely transmuted into `aligned::repr_rust` in th --> $DIR/should_require_well_defined_layout.rs:47:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `u128` does not have a well-specified layout + | ^^^^^^^^^ `aligned::repr_rust` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 @@ -200,7 +200,7 @@ error[E0277]: `u128` cannot be safely transmuted into `packed::repr_rust` in the --> $DIR/should_require_well_defined_layout.rs:53:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `u128` does not have a well-specified layout + | ^^^^^^^^^ `packed::repr_rust` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 @@ -244,7 +244,7 @@ error[E0277]: `u128` cannot be safely transmuted into `nested::repr_c` in the de --> $DIR/should_require_well_defined_layout.rs:60:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^ `u128` does not have a well-specified layout + | ^^^^^^ `nested::repr_c` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 diff --git a/tests/ui/transmutability/unions/repr/should_require_well_defined_layout.stderr b/tests/ui/transmutability/unions/repr/should_require_well_defined_layout.stderr index 305ecc71733d3..4293d34f47b23 100644 --- a/tests/ui/transmutability/unions/repr/should_require_well_defined_layout.stderr +++ b/tests/ui/transmutability/unions/repr/should_require_well_defined_layout.stderr @@ -24,7 +24,7 @@ error[E0277]: `u128` cannot be safely transmuted into `should_reject_repr_rust:: --> $DIR/should_require_well_defined_layout.rs:31:43 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ `u128` does not have a well-specified layout + | ^^^^^^^^^ `should_reject_repr_rust::repr_rust` does not have a well-specified layout | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:13:14 diff --git a/tests/ui/transmutability/unions/should_pad_variants.stderr b/tests/ui/transmutability/unions/should_pad_variants.stderr index c82a79680225d..bfbef8b25fcf4 100644 --- a/tests/ui/transmutability/unions/should_pad_variants.stderr +++ b/tests/ui/transmutability/unions/should_pad_variants.stderr @@ -2,7 +2,7 @@ error[E0277]: `Src` cannot be safely transmuted into `Dst` in the defining scope --> $DIR/should_pad_variants.rs:44:36 | LL | assert::is_transmutable::(); - | ^^^ At least one value of `Src` isn't a bit-valid value of `Dst` + | ^^^ The size of `Src` is smaller than the size of `Dst` | note: required by a bound in `is_transmutable` --> $DIR/should_pad_variants.rs:13:14 diff --git a/tests/ui/transmute/transmute-padding-ice.stderr b/tests/ui/transmute/transmute-padding-ice.stderr index 2739fd16645ab..f5480e0b9fb82 100644 --- a/tests/ui/transmute/transmute-padding-ice.stderr +++ b/tests/ui/transmute/transmute-padding-ice.stderr @@ -2,7 +2,7 @@ error[E0277]: `B` cannot be safely transmuted into `A` in the defining scope of --> $DIR/transmute-padding-ice.rs:27:40 | LL | assert::is_maybe_transmutable::(); - | ^ At least one value of `B` isn't a bit-valid value of `A` + | ^ The size of `B` is smaller than the size of `A` | note: required by a bound in `is_maybe_transmutable` --> $DIR/transmute-padding-ice.rs:11:14 From db3275c962eae006a7502f89f2eaf07af2d0f1dd Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Thu, 27 Apr 2023 18:33:52 -0700 Subject: [PATCH 4/8] Safe Transmute: Add alignment tests --- .../transmutability/alignment/align-fail.rs | 23 ++++++++++++++ .../alignment/align-fail.stderr | 30 +++++++++++++++++++ .../transmutability/alignment/align-pass.rs | 23 ++++++++++++++ 3 files changed, 76 insertions(+) create mode 100644 tests/ui/transmutability/alignment/align-fail.rs create mode 100644 tests/ui/transmutability/alignment/align-fail.stderr create mode 100644 tests/ui/transmutability/alignment/align-pass.rs diff --git a/tests/ui/transmutability/alignment/align-fail.rs b/tests/ui/transmutability/alignment/align-fail.rs new file mode 100644 index 0000000000000..7f6090a6e4db5 --- /dev/null +++ b/tests/ui/transmutability/alignment/align-fail.rs @@ -0,0 +1,23 @@ +// check-fail +#![feature(transmutability)] + +mod assert { + use std::mem::{Assume, BikeshedIntrinsicFrom}; + pub struct Context; + + pub fn is_maybe_transmutable() + where + Dst: BikeshedIntrinsicFrom + {} +} + +fn main() { + assert::is_maybe_transmutable::<&'static [u8; 0], &'static [u16; 0]>(); //~ ERROR `&[u8; 0]` cannot be safely transmuted into `&[u16; 0]` +} diff --git a/tests/ui/transmutability/alignment/align-fail.stderr b/tests/ui/transmutability/alignment/align-fail.stderr new file mode 100644 index 0000000000000..8a191dc61776d --- /dev/null +++ b/tests/ui/transmutability/alignment/align-fail.stderr @@ -0,0 +1,30 @@ +error[E0277]: `&[u8; 0]` cannot be safely transmuted into `&[u16; 0]` in the defining scope of `assert::Context` + --> $DIR/align-fail.rs:22:55 + | +LL | ...tatic [u8; 0], &'static [u16; 0]>(); + | ^^^^^^^^^^^^^^^^^ The alignment of `&[u8; 0]` should be stricter than that of `&[u16; 0]`, but it is not + | +note: required by a bound in `is_maybe_transmutable` + --> $DIR/align-fail.rs:10:14 + | +LL | pub fn is_maybe_transmutable() + | --------------------- required by a bound in this function +LL | where +LL | Dst: BikeshedIntrinsicFrom + | |__________^ required by this bound in `is_maybe_transmutable` +help: consider removing the leading `&`-reference + | +LL - assert::is_maybe_transmutable::<&'static [u8; 0], &'static [u16; 0]>(); +LL + assert::is_maybe_transmutable::<&'static [u8; 0], [u16; 0]>(); + | + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/transmutability/alignment/align-pass.rs b/tests/ui/transmutability/alignment/align-pass.rs new file mode 100644 index 0000000000000..62dc672eacb9b --- /dev/null +++ b/tests/ui/transmutability/alignment/align-pass.rs @@ -0,0 +1,23 @@ +// check-pass +#![feature(transmutability)] + +mod assert { + use std::mem::{Assume, BikeshedIntrinsicFrom}; + pub struct Context; + + pub fn is_maybe_transmutable() + where + Dst: BikeshedIntrinsicFrom + {} +} + +fn main() { + assert::is_maybe_transmutable::<&'static [u16; 0], &'static [u8; 0]>(); +} From 62663582375d7dedf42c0a30bfe04c7b53b452d7 Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Fri, 28 Apr 2023 14:06:10 -0700 Subject: [PATCH 5/8] Safe Transmute: Check mutability before creating dst -> src obligation - Only create dst -> src obligation if Dst is mutable - Add some long comments to explain parts of the transmutability code that were unclear to me when reading - Update/add tests --- .../src/traits/error_reporting/mod.rs | 3 ++- .../src/traits/select/confirmation.rs | 17 ++++++++++--- compiler/rustc_transmute/src/layout/mod.rs | 15 ++++++----- compiler/rustc_transmute/src/lib.rs | 2 +- .../src/maybe_transmutable/mod.rs | 22 ++++++++++++++++ .../alignment/align-fail.stderr | 4 +-- .../ui/transmutability/primitives/bool-mut.rs | 17 +++++++++++++ .../primitives/bool-mut.stderr | 18 +++++++++++++ .../primitives/bool.current.stderr | 4 +-- .../primitives/bool.next.stderr | 4 +-- tests/ui/transmutability/primitives/bool.rs | 5 +--- ...ursive-wrapper-types-bit-compatible-mut.rs | 25 +++++++++++++++++++ ...e-wrapper-types-bit-compatible-mut.stderr} | 10 ++++---- .../recursive-wrapper-types-bit-compatible.rs | 6 ++--- ...ecursive-wrapper-types-bit-incompatible.rs | 2 +- .../transmutability/references/u8-to-unit.rs | 8 +++--- .../references/u8-to-unit.stderr | 25 ------------------- .../transmutability/references/unit-to-u8.rs | 4 +-- 18 files changed, 128 insertions(+), 63 deletions(-) create mode 100644 tests/ui/transmutability/primitives/bool-mut.rs create mode 100644 tests/ui/transmutability/primitives/bool-mut.stderr create mode 100644 tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible-mut.rs rename tests/ui/transmutability/references/{recursive-wrapper-types-bit-compatible.stderr => recursive-wrapper-types-bit-compatible-mut.stderr} (59%) delete mode 100644 tests/ui/transmutability/references/u8-to-unit.stderr 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 8224fdf591afb..82ce2c2fa8d52 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -2784,9 +2784,10 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { rustc_transmute::Reason::DstIsTooBig => { format!("The size of `{src}` is smaller than the size of `{dst}`") } + // FIXME(bryangarza): Say exactly what the minimum alignments of src and dst are rustc_transmute::Reason::DstHasStricterAlignment => { format!( - "The alignment of `{src}` should be stricter than that of `{dst}`, but it is not" + "The minimum alignment of `{src}` should be greater than that of `{dst}`, but it is not" ) } rustc_transmute::Reason::DstIsMoreUnique => { diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index e3d982b5c3f8b..6b8d8a947eae1 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -6,6 +6,7 @@ //! //! [rustc dev guide]: //! https://rustc-dev-guide.rust-lang.org/traits/resolution.html#confirmation +use rustc_ast::Mutability; use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_hir::lang_items::LangItem; use rustc_infer::infer::LateBoundRegionConversionTime::HigherRankedType; @@ -295,6 +296,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { Ok(None) => Ok(vec![]), Err(_) => Err(Unimplemented), // FIXME(bryangarza): Add separate `IfAny` case, instead of treating as `IfAll` + // Not possible until the trait solver supports disjunctions of obligations Ok(Some(rustc_transmute::Condition::IfAll(answers))) | Ok(Some(rustc_transmute::Condition::IfAny(answers))) => { let mut nested = vec![]; @@ -311,7 +313,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { let scope = predicate.trait_ref.substs.type_at(2); let assume_const = predicate.trait_ref.substs.const_at(3); let make_obl = |from_ty, to_ty| { - let trait_ref1 = tcx.mk_trait_ref( + let trait_ref1 = ty::TraitRef::new( + tcx, trait_def_id, [ ty::GenericArg::from(to_ty), @@ -329,8 +332,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ) }; - // FIXME(bryangarza): Check src.mutability or dst.mutability to know whether dst -> src obligation is needed - Ok(vec![make_obl(src.ty, dst.ty), make_obl(dst.ty, src.ty)]) + // If Dst is mutable, check bidirectionally. + // For example, transmuting bool -> u8 is OK as long as you can't update that u8 + // to be > 1, because you could later transmute the u8 back to a bool and get UB. + let mut obligations = vec![make_obl(src.ty, dst.ty)]; + if dst.mutability == Mutability::Mut { + obligations.push(make_obl(dst.ty, src.ty)); + } + Ok(obligations) } } } @@ -353,6 +362,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { let dst = predicate.trait_ref.substs.type_at(0); let src = predicate.trait_ref.substs.type_at(1); + debug!(?src, ?dst); let mut transmute_env = rustc_transmute::TransmuteTypeEnv::new(self.infcx); let maybe_transmutable = transmute_env.is_transmutable( obligation.cause.clone(), @@ -361,7 +371,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { assume, ); - debug!(?src, ?dst); let fully_flattened = flatten_answer_tree(self.tcx(), obligation, predicate, maybe_transmutable)?; debug!(?fully_flattened); diff --git a/compiler/rustc_transmute/src/layout/mod.rs b/compiler/rustc_transmute/src/layout/mod.rs index b318447e581c7..76d97e0e6e7a6 100644 --- a/compiler/rustc_transmute/src/layout/mod.rs +++ b/compiler/rustc_transmute/src/layout/mod.rs @@ -31,18 +31,21 @@ impl fmt::Debug for Byte { pub(crate) trait Def: Debug + Hash + Eq + PartialEq + Copy + Clone {} pub trait Ref: Debug + Hash + Eq + PartialEq + Copy + Clone { + fn min_align(&self) -> usize; + + fn is_mutable(&self) -> bool; +} + +impl Def for ! {} +impl Ref for ! { fn min_align(&self) -> usize { - 1 + unreachable!() } - fn is_mutable(&self) -> bool { - false + unreachable!() } } -impl Def for ! {} -impl Ref for ! {} - #[cfg(feature = "rustc")] pub mod rustc { use rustc_middle::mir::Mutability; diff --git a/compiler/rustc_transmute/src/lib.rs b/compiler/rustc_transmute/src/lib.rs index 60adbc1b470c8..baf63e6d3a225 100644 --- a/compiler/rustc_transmute/src/lib.rs +++ b/compiler/rustc_transmute/src/lib.rs @@ -1,4 +1,4 @@ -#![feature(alloc_layout_extra, decl_macro, iterator_try_reduce, never_type, let_chains)] +#![feature(alloc_layout_extra, decl_macro, iterator_try_reduce, never_type)] #![allow(dead_code, unused_variables)] #![deny(rustc::untranslatable_diagnostic)] #![deny(rustc::diagnostic_outside_of_impl)] diff --git a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs index 47f61a808403b..0f4a096cdae19 100644 --- a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs +++ b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs @@ -203,8 +203,29 @@ where if let Some(answer) = cache.get(&(src_state, dst_state)) { answer.clone() } else { + debug!(?src_state, ?dst_state); + debug!(src = ?self.src); + debug!(dst = ?self.dst); + debug!( + src_transitions_len = self.src.transitions.len(), + dst_transitions_len = self.dst.transitions.len() + ); let answer = if dst_state == self.dst.accepting { // truncation: `size_of(Src) >= size_of(Dst)` + // + // Why is truncation OK to do? Because even though the Src is bigger, all we care about + // is whether we have enough data for the Dst to be valid in accordance with what its + // type dictates. + // For example, in a u8 to `()` transmutation, we have enough data available from the u8 + // to transmute it to a `()` (though in this case does `()` really need any data to + // begin with? It doesn't). Same thing with u8 to fieldless struct. + // Now then, why is something like u8 to bool not allowed? That is not because the bool + // is smaller in size, but rather because those 2 bits that we are re-interpreting from + // the u8 could introduce invalid states for the bool type. + // + // So, if it's possible to transmute to a smaller Dst by truncating, and we can guarantee + // that none of the actually-used data can introduce an invalid state for Dst's type, we + // are able to safely transmute, even with truncation. Ok(None) } else if src_state == self.src.accepting { // extension: `size_of(Src) >= size_of(Dst)` @@ -259,6 +280,7 @@ where // ...if `refs_answer` was computed lazily. The below early // returns can be deleted without impacting the correctness of // the algoritm; only its performance. + debug!(?bytes_answer); match bytes_answer { Err(_) if !self.assume.validity => return bytes_answer, Ok(None) if self.assume.validity => return bytes_answer, diff --git a/tests/ui/transmutability/alignment/align-fail.stderr b/tests/ui/transmutability/alignment/align-fail.stderr index 8a191dc61776d..8ace98a3ed737 100644 --- a/tests/ui/transmutability/alignment/align-fail.stderr +++ b/tests/ui/transmutability/alignment/align-fail.stderr @@ -1,8 +1,8 @@ error[E0277]: `&[u8; 0]` cannot be safely transmuted into `&[u16; 0]` in the defining scope of `assert::Context` --> $DIR/align-fail.rs:22:55 | -LL | ...tatic [u8; 0], &'static [u16; 0]>(); - | ^^^^^^^^^^^^^^^^^ The alignment of `&[u8; 0]` should be stricter than that of `&[u16; 0]`, but it is not +LL | ...c [u8; 0], &'static [u16; 0]>(); + | ^^^^^^^^^^^^^^^^^ The minimum alignment of `&[u8; 0]` should be greater than that of `&[u16; 0]`, but it is not | note: required by a bound in `is_maybe_transmutable` --> $DIR/align-fail.rs:10:14 diff --git a/tests/ui/transmutability/primitives/bool-mut.rs b/tests/ui/transmutability/primitives/bool-mut.rs new file mode 100644 index 0000000000000..49dbe90e4b8b3 --- /dev/null +++ b/tests/ui/transmutability/primitives/bool-mut.rs @@ -0,0 +1,17 @@ +// check-fail +//[next] compile-flags: -Ztrait-solver=next + +#![feature(transmutability)] +mod assert { + use std::mem::{Assume, BikeshedIntrinsicFrom}; + pub struct Context; + + pub fn is_transmutable() + where + Dst: BikeshedIntrinsicFrom + {} +} + +fn main() { + assert::is_transmutable::<&'static mut bool, &'static mut u8>() //~ ERROR cannot be safely transmuted +} diff --git a/tests/ui/transmutability/primitives/bool-mut.stderr b/tests/ui/transmutability/primitives/bool-mut.stderr new file mode 100644 index 0000000000000..b36991e1c01c6 --- /dev/null +++ b/tests/ui/transmutability/primitives/bool-mut.stderr @@ -0,0 +1,18 @@ +error[E0277]: `u8` cannot be safely transmuted into `bool` in the defining scope of `assert::Context` + --> $DIR/bool-mut.rs:16:50 + | +LL | assert::is_transmutable::<&'static mut bool, &'static mut u8>() + | ^^^^^^^^^^^^^^^ At least one value of `u8` isn't a bit-valid value of `bool` + | +note: required by a bound in `is_transmutable` + --> $DIR/bool-mut.rs:11:14 + | +LL | pub fn is_transmutable() + | --------------- required by a bound in this function +LL | where +LL | Dst: BikeshedIntrinsicFrom + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `is_transmutable` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/transmutability/primitives/bool.current.stderr b/tests/ui/transmutability/primitives/bool.current.stderr index 47c8438a251c2..4b3eb6c517d91 100644 --- a/tests/ui/transmutability/primitives/bool.current.stderr +++ b/tests/ui/transmutability/primitives/bool.current.stderr @@ -1,11 +1,11 @@ error[E0277]: `u8` cannot be safely transmuted into `bool` in the defining scope of `assert::Context` - --> $DIR/bool.rs:24:35 + --> $DIR/bool.rs:21:35 | LL | assert::is_transmutable::(); | ^^^^ At least one value of `u8` isn't a bit-valid value of `bool` | note: required by a bound in `is_transmutable` - --> $DIR/bool.rs:14:14 + --> $DIR/bool.rs:11:14 | LL | pub fn is_transmutable() | --------------- required by a bound in this function diff --git a/tests/ui/transmutability/primitives/bool.next.stderr b/tests/ui/transmutability/primitives/bool.next.stderr index 47c8438a251c2..4b3eb6c517d91 100644 --- a/tests/ui/transmutability/primitives/bool.next.stderr +++ b/tests/ui/transmutability/primitives/bool.next.stderr @@ -1,11 +1,11 @@ error[E0277]: `u8` cannot be safely transmuted into `bool` in the defining scope of `assert::Context` - --> $DIR/bool.rs:24:35 + --> $DIR/bool.rs:21:35 | LL | assert::is_transmutable::(); | ^^^^ At least one value of `u8` isn't a bit-valid value of `bool` | note: required by a bound in `is_transmutable` - --> $DIR/bool.rs:14:14 + --> $DIR/bool.rs:11:14 | LL | pub fn is_transmutable() | --------------- required by a bound in this function diff --git a/tests/ui/transmutability/primitives/bool.rs b/tests/ui/transmutability/primitives/bool.rs index de77cfc78aa89..654e7b47edecc 100644 --- a/tests/ui/transmutability/primitives/bool.rs +++ b/tests/ui/transmutability/primitives/bool.rs @@ -1,10 +1,7 @@ // revisions: current next //[next] compile-flags: -Ztrait-solver=next -#![crate_type = "lib"] #![feature(transmutability)] -#![allow(dead_code)] -#![allow(incomplete_features)] mod assert { use std::mem::{Assume, BikeshedIntrinsicFrom}; pub struct Context; @@ -20,7 +17,7 @@ mod assert { {} } -fn contrast_with_u8() { +fn main() { assert::is_transmutable::(); //~ ERROR cannot be safely transmuted assert::is_maybe_transmutable::(); assert::is_transmutable::(); diff --git a/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible-mut.rs b/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible-mut.rs new file mode 100644 index 0000000000000..a6e2889d3f23d --- /dev/null +++ b/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible-mut.rs @@ -0,0 +1,25 @@ +// check-fail +#![feature(transmutability)] + +mod assert { + use std::mem::{Assume, BikeshedIntrinsicFrom}; + pub struct Context; + + pub fn is_maybe_transmutable() + where + Dst: BikeshedIntrinsicFrom + {} +} + +fn main() { + #[repr(C)] struct A(bool, &'static A); + #[repr(C)] struct B(u8, &'static B); + assert::is_maybe_transmutable::<&'static A, &'static mut B>(); //~ ERROR cannot be safely transmuted +} diff --git a/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.stderr b/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible-mut.stderr similarity index 59% rename from tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.stderr rename to tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible-mut.stderr index fac0e4f032e00..4b4d6ad0298ee 100644 --- a/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.stderr +++ b/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible-mut.stderr @@ -1,11 +1,11 @@ -error[E0277]: `B` cannot be safely transmuted into `A` in the defining scope of `assert::Context` - --> $DIR/recursive-wrapper-types-bit-compatible.rs:26:49 +error[E0277]: `&A` cannot be safely transmuted into `&mut B` in the defining scope of `assert::Context` + --> $DIR/recursive-wrapper-types-bit-compatible-mut.rs:24:49 | -LL | assert::is_maybe_transmutable::<&'static A, &'static B>(); - | ^^^^^^^^^^ At least one value of `B` isn't a bit-valid value of `A` +LL | assert::is_maybe_transmutable::<&'static A, &'static mut B>(); + | ^^^^^^^^^^^^^^ `&A` is a shared reference, but `&mut B` is a unique reference | note: required by a bound in `is_maybe_transmutable` - --> $DIR/recursive-wrapper-types-bit-compatible.rs:10:14 + --> $DIR/recursive-wrapper-types-bit-compatible-mut.rs:10:14 | LL | pub fn is_maybe_transmutable() | --------------------- required by a bound in this function diff --git a/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.rs b/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.rs index 918147a086211..709d8cdc762e9 100644 --- a/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.rs +++ b/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.rs @@ -1,4 +1,4 @@ -// check-fail +// check-pass #![feature(transmutability)] mod assert { @@ -21,7 +21,5 @@ mod assert { fn main() { #[repr(C)] struct A(bool, &'static A); #[repr(C)] struct B(u8, &'static B); - // FIXME(bryangarza): Make 2 variants of this test, depending on mutability. - // Right now, we are being strict by default and checking A->B and B->A both. - assert::is_maybe_transmutable::<&'static A, &'static B>(); //~ ERROR `B` cannot be safely transmuted into `A` + assert::is_maybe_transmutable::<&'static A, &'static B>(); } diff --git a/tests/ui/transmutability/references/recursive-wrapper-types-bit-incompatible.rs b/tests/ui/transmutability/references/recursive-wrapper-types-bit-incompatible.rs index 6dcb7df9feb1c..e8582d2fd0212 100644 --- a/tests/ui/transmutability/references/recursive-wrapper-types-bit-incompatible.rs +++ b/tests/ui/transmutability/references/recursive-wrapper-types-bit-incompatible.rs @@ -21,5 +21,5 @@ mod assert { fn main() { #[repr(C)] struct A(bool, &'static A); #[repr(C)] struct B(u8, &'static B); - assert::is_maybe_transmutable::<&'static B, &'static A>(); //~ ERROR `B` cannot be safely transmuted into `A` + assert::is_maybe_transmutable::<&'static B, &'static A>(); //~ ERROR cannot be safely transmuted } diff --git a/tests/ui/transmutability/references/u8-to-unit.rs b/tests/ui/transmutability/references/u8-to-unit.rs index b7dd638b952ad..8b37492bd6b1c 100644 --- a/tests/ui/transmutability/references/u8-to-unit.rs +++ b/tests/ui/transmutability/references/u8-to-unit.rs @@ -1,4 +1,4 @@ -// check-fail +// check-pass #![feature(transmutability)] mod assert { @@ -9,10 +9,10 @@ mod assert { where Dst: BikeshedIntrinsicFrom {} @@ -20,5 +20,5 @@ mod assert { fn main() { #[repr(C)] struct Unit; - assert::is_maybe_transmutable::<&'static u8, &'static Unit>(); //~ ERROR `Unit` cannot be safely transmuted into `u8` + assert::is_maybe_transmutable::<&'static u8, &'static Unit>(); } diff --git a/tests/ui/transmutability/references/u8-to-unit.stderr b/tests/ui/transmutability/references/u8-to-unit.stderr deleted file mode 100644 index 81b0b57f0cf4b..0000000000000 --- a/tests/ui/transmutability/references/u8-to-unit.stderr +++ /dev/null @@ -1,25 +0,0 @@ -error[E0277]: `Unit` cannot be safely transmuted into `u8` in the defining scope of `assert::Context` - --> $DIR/u8-to-unit.rs:23:50 - | -LL | assert::is_maybe_transmutable::<&'static u8, &'static Unit>(); - | ^^^^^^^^^^^^^ The size of `Unit` is smaller than the size of `u8` - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/u8-to-unit.rs:10:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/transmutability/references/unit-to-u8.rs b/tests/ui/transmutability/references/unit-to-u8.rs index 73e1db3d2d58b..eff516e9a9691 100644 --- a/tests/ui/transmutability/references/unit-to-u8.rs +++ b/tests/ui/transmutability/references/unit-to-u8.rs @@ -12,7 +12,7 @@ mod assert { alignment: true, lifetimes: true, safety: true, - validity: true, + validity: false, } }> {} @@ -20,5 +20,5 @@ mod assert { fn main() { #[repr(C)] struct Unit; - assert::is_maybe_transmutable::<&'static Unit, &'static u8>(); //~ ERROR `Unit` cannot be safely transmuted into `u8` + assert::is_maybe_transmutable::<&'static Unit, &'static u8>(); //~ ERROR cannot be safely transmuted } From d2164d5c9a17a7c2eefeccf623c278612b2650de Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Wed, 24 May 2023 17:47:13 -0700 Subject: [PATCH 6/8] Safe Transmute: Update definition of Condition type - Change `Condition` to not contain `Answer`s but instead just contain other `Condition`s directly. - Also improve error reporting for `DstHasStricterAlignment` --- .../src/traits/error_reporting/mod.rs | 11 ++++---- .../src/traits/select/confirmation.rs | 26 ++++++++++--------- compiler/rustc_transmute/src/lib.rs | 6 ++--- .../src/maybe_transmutable/mod.rs | 25 ++++++++++-------- .../alignment/align-fail.stderr | 4 +-- 5 files changed, 39 insertions(+), 33 deletions(-) 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 82ce2c2fa8d52..23126f2cb52dd 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -2751,8 +2751,7 @@ 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): Is this enough, or should we resolve all nested - // obligations like we do for `confirm_transmutability_candidate(...)?` + match rustc_transmute::TransmuteTypeEnv::new(self.infcx).is_transmutable( obligation.cause, src_and_dst, @@ -2784,10 +2783,12 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { rustc_transmute::Reason::DstIsTooBig => { format!("The size of `{src}` is smaller than the size of `{dst}`") } - // FIXME(bryangarza): Say exactly what the minimum alignments of src and dst are - rustc_transmute::Reason::DstHasStricterAlignment => { + rustc_transmute::Reason::DstHasStricterAlignment { + src_min_align, + dst_min_align, + } => { format!( - "The minimum alignment of `{src}` should be greater than that of `{dst}`, but it is not" + "The minimum alignment of `{src}` ({src_min_align}) should be greater than that of `{dst}` ({dst_min_align})" ) } rustc_transmute::Reason::DstIsMoreUnique => { diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 6b8d8a947eae1..3f07ad814ff61 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -290,25 +290,23 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { tcx: TyCtxt<'tcx>, obligation: &TraitObligation<'tcx>, predicate: TraitPredicate<'tcx>, - answer: rustc_transmute::Answer>, - ) -> Result>, SelectionError<'tcx>> { + answer: rustc_transmute::Condition>, + ) -> Vec> { match answer { - Ok(None) => Ok(vec![]), - Err(_) => Err(Unimplemented), // FIXME(bryangarza): Add separate `IfAny` case, instead of treating as `IfAll` // Not possible until the trait solver supports disjunctions of obligations - Ok(Some(rustc_transmute::Condition::IfAll(answers))) - | Ok(Some(rustc_transmute::Condition::IfAny(answers))) => { + rustc_transmute::Condition::IfAll(answers) + | rustc_transmute::Condition::IfAny(answers) => { let mut nested = vec![]; for flattened in answers .into_iter() .map(|answer| flatten_answer_tree(tcx, obligation, predicate, answer)) { - nested.extend(flattened?); + nested.extend(flattened); } - Ok(nested) + nested } - Ok(Some(rustc_transmute::Condition::IfTransmutable { src, dst })) => { + 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); @@ -339,7 +337,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { if dst.mutability == Mutability::Mut { obligations.push(make_obl(dst.ty, src.ty)); } - Ok(obligations) + obligations } } } @@ -371,8 +369,12 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { assume, ); - let fully_flattened = - flatten_answer_tree(self.tcx(), obligation, predicate, maybe_transmutable)?; + let fully_flattened = match maybe_transmutable { + Err(_) => Err(Unimplemented)?, + Ok(Some(mt)) => flatten_answer_tree(self.tcx(), obligation, predicate, mt), + Ok(None) => vec![], + }; + debug!(?fully_flattened); Ok(ImplSourceBuiltinData { nested: fully_flattened }) } diff --git a/compiler/rustc_transmute/src/lib.rs b/compiler/rustc_transmute/src/lib.rs index baf63e6d3a225..7a8cbd50d450d 100644 --- a/compiler/rustc_transmute/src/lib.rs +++ b/compiler/rustc_transmute/src/lib.rs @@ -29,10 +29,10 @@ pub enum Condition { IfTransmutable { src: R, dst: R }, /// `Src` is transmutable into `Dst`, if all of the enclosed requirements are met. - IfAll(Vec>), + IfAll(Vec>), /// `Src` is transmutable into `Dst` if any of the enclosed requirements are met. - IfAny(Vec>), + IfAny(Vec>), } /// Answers: Why wasn't the source type transmutable into the destination type? @@ -49,7 +49,7 @@ pub enum Reason { /// `Dst` is larger than `Src`, and the excess bytes were not exclusively uninitialized. DstIsTooBig, /// Src should have a stricter alignment than Dst, but it does not. - DstHasStricterAlignment, + DstHasStricterAlignment { src_min_align: usize, dst_min_align: usize }, /// Can't go from shared pointer to unique pointer DstIsMoreUnique, } diff --git a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs index 0f4a096cdae19..80e3489e99bcd 100644 --- a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs +++ b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs @@ -303,7 +303,10 @@ where } else if !self.assume.alignment && src_ref.min_align() < dst_ref.min_align() { - Err(Reason::DstHasStricterAlignment) + Err(Reason::DstHasStricterAlignment { + src_min_align: src_ref.min_align(), + dst_min_align: dst_ref.min_align(), + }) } else { // ...such that `src` is transmutable into `dst`, if // `src_ref` is transmutability into `dst_ref`. @@ -360,13 +363,13 @@ where Some(Condition::IfAll(lhs)) } // 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)) + (Some(cond), Some(Condition::IfAll(mut conds))) + | (Some(Condition::IfAll(mut conds)), Some(cond)) => { + conds.push(cond); + Some(Condition::IfAll(conds)) } // Otherwise, both lhs and rhs conditions can be combined in a parent IfAll - (lhs, rhs) => Some(Condition::IfAll(vec![Ok(lhs), Ok(rhs)])), + (Some(lhs), Some(rhs)) => Some(Condition::IfAll(vec![lhs, rhs])), }) } @@ -394,13 +397,13 @@ where 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)) + (Some(cond), Some(Condition::IfAny(mut conds))) + | (Some(Condition::IfAny(mut conds)), Some(cond)) => { + conds.push(cond); + Some(Condition::IfAny(conds)) } // Otherwise, both lhs and rhs conditions can be combined in a parent IfAny - (lhs, rhs) => Some(Condition::IfAny(vec![Ok(lhs), Ok(rhs)])), + (Some(lhs), Some(rhs)) => Some(Condition::IfAny(vec![lhs, rhs])), }) } diff --git a/tests/ui/transmutability/alignment/align-fail.stderr b/tests/ui/transmutability/alignment/align-fail.stderr index 8ace98a3ed737..59246fb1b0371 100644 --- a/tests/ui/transmutability/alignment/align-fail.stderr +++ b/tests/ui/transmutability/alignment/align-fail.stderr @@ -1,8 +1,8 @@ error[E0277]: `&[u8; 0]` cannot be safely transmuted into `&[u16; 0]` in the defining scope of `assert::Context` --> $DIR/align-fail.rs:22:55 | -LL | ...c [u8; 0], &'static [u16; 0]>(); - | ^^^^^^^^^^^^^^^^^ The minimum alignment of `&[u8; 0]` should be greater than that of `&[u16; 0]`, but it is not +LL | ...tatic [u8; 0], &'static [u16; 0]>(); + | ^^^^^^^^^^^^^^^^^ The minimum alignment of `&[u8; 0]` (1) should be greater than that of `&[u16; 0]` (2) | note: required by a bound in `is_maybe_transmutable` --> $DIR/align-fail.rs:10:14 From 64a54df86faf1f55148433753296dc2bc2a7e31d Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Wed, 7 Jun 2023 08:17:58 -0700 Subject: [PATCH 7/8] Safe Transmute: Disable coinduction support This patch just removes the `#[rustc_coinductive]` annotation from `BikeshedIntrinsicFrom` and flips the related tests to `check-fail`. Once an FCP for setting the annotation on the trait is approved, it could be enabled again. --- library/core/src/mem/transmutability.rs | 1 - .../recursive-wrapper-types-bit-compatible.rs | 5 ++-- ...ursive-wrapper-types-bit-compatible.stderr | 25 +++++++++++++++++++ .../references/recursive-wrapper-types.rs | 5 ++-- .../references/recursive-wrapper-types.stderr | 25 +++++++++++++++++++ 5 files changed, 56 insertions(+), 5 deletions(-) create mode 100644 tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.stderr create mode 100644 tests/ui/transmutability/references/recursive-wrapper-types.stderr diff --git a/library/core/src/mem/transmutability.rs b/library/core/src/mem/transmutability.rs index d0c30e715d56b..87ae30619c63b 100644 --- a/library/core/src/mem/transmutability.rs +++ b/library/core/src/mem/transmutability.rs @@ -5,7 +5,6 @@ /// notwithstanding whatever safety checks you have asked the compiler to [`Assume`] are satisfied. #[unstable(feature = "transmutability", issue = "99571")] #[lang = "transmute_trait"] -#[rustc_coinductive] pub unsafe trait BikeshedIntrinsicFrom where Src: ?Sized, diff --git a/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.rs b/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.rs index 709d8cdc762e9..3ea80173afaa9 100644 --- a/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.rs +++ b/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.rs @@ -1,4 +1,5 @@ -// check-pass +// check-fail +// FIXME(bryangarza): Change to check-pass when coinduction is supported for BikeshedIntrinsicFrom #![feature(transmutability)] mod assert { @@ -21,5 +22,5 @@ mod assert { fn main() { #[repr(C)] struct A(bool, &'static A); #[repr(C)] struct B(u8, &'static B); - assert::is_maybe_transmutable::<&'static A, &'static B>(); + assert::is_maybe_transmutable::<&'static A, &'static B>(); //~ ERROR overflow evaluating the requirement } diff --git a/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.stderr b/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.stderr new file mode 100644 index 0000000000000..fae332e6af932 --- /dev/null +++ b/tests/ui/transmutability/references/recursive-wrapper-types-bit-compatible.stderr @@ -0,0 +1,25 @@ +error[E0275]: overflow evaluating the requirement `B: BikeshedIntrinsicFrom` + --> $DIR/recursive-wrapper-types-bit-compatible.rs:25:5 + | +LL | assert::is_maybe_transmutable::<&'static A, &'static B>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: required by a bound in `is_maybe_transmutable` + --> $DIR/recursive-wrapper-types-bit-compatible.rs:11:14 + | +LL | pub fn is_maybe_transmutable() + | --------------------- required by a bound in this function +LL | where +LL | Dst: BikeshedIntrinsicFrom + | |__________^ required by this bound in `is_maybe_transmutable` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0275`. diff --git a/tests/ui/transmutability/references/recursive-wrapper-types.rs b/tests/ui/transmutability/references/recursive-wrapper-types.rs index 090c1fea6dbdf..59d1ad84a5d4c 100644 --- a/tests/ui/transmutability/references/recursive-wrapper-types.rs +++ b/tests/ui/transmutability/references/recursive-wrapper-types.rs @@ -1,4 +1,5 @@ -// check-pass +// check-fail +// FIXME(bryangarza): Change to check-pass when coinduction is supported for BikeshedIntrinsicFrom #![feature(transmutability)] mod assert { @@ -21,6 +22,6 @@ mod assert { fn main() { #[repr(C)] struct A(&'static B); #[repr(C)] struct B(&'static A); - assert::is_maybe_transmutable::<&'static A, &'static B>(); + assert::is_maybe_transmutable::<&'static A, &'static B>(); //~ overflow evaluating the requirement assert::is_maybe_transmutable::<&'static B, &'static A>(); } diff --git a/tests/ui/transmutability/references/recursive-wrapper-types.stderr b/tests/ui/transmutability/references/recursive-wrapper-types.stderr new file mode 100644 index 0000000000000..35a60c226437a --- /dev/null +++ b/tests/ui/transmutability/references/recursive-wrapper-types.stderr @@ -0,0 +1,25 @@ +error[E0275]: overflow evaluating the requirement `A: BikeshedIntrinsicFrom` + --> $DIR/recursive-wrapper-types.rs:25:5 + | +LL | assert::is_maybe_transmutable::<&'static A, &'static B>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: required by a bound in `is_maybe_transmutable` + --> $DIR/recursive-wrapper-types.rs:11:14 + | +LL | pub fn is_maybe_transmutable() + | --------------------- required by a bound in this function +LL | where +LL | Dst: BikeshedIntrinsicFrom + | |__________^ required by this bound in `is_maybe_transmutable` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0275`. From f4cf8f65a5e8e110c8c36469d31f16e8571e2c1a Mon Sep 17 00:00:00 2001 From: Bryan Garza <1396101+bryangarza@users.noreply.github.com> Date: Mon, 12 Jun 2023 16:35:23 -0700 Subject: [PATCH 8/8] Safe Transmute: Refactor error handling and Answer type - Create `Answer` type that is not just a type alias of `Result` - Remove a usage of `map_layouts` to make the code easier to read - Don't hide errors related to Unknown Layout when computing transmutability --- .../src/solve/eval_ctxt.rs | 8 +- .../src/traits/error_reporting/mod.rs | 41 +++-- .../src/traits/select/confirmation.rs | 35 ++-- compiler/rustc_transmute/src/layout/tree.rs | 4 +- compiler/rustc_transmute/src/lib.rs | 20 ++- .../src/maybe_transmutable/mod.rs | 152 +++++++++--------- .../src/maybe_transmutable/tests.rs | 12 +- .../unknown_src_field.rs | 2 +- .../unknown_src_field.stderr | 20 ++- 9 files changed, 167 insertions(+), 127 deletions(-) diff --git a/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs b/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs index e8abd5964bbf3..fb28faa2b7c23 100644 --- a/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs +++ b/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs @@ -668,6 +668,7 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { scope: Ty<'tcx>, assume: rustc_transmute::Assume, ) -> Result { + use rustc_transmute::Answer; // FIXME(transmutability): This really should be returning nested goals for `Answer::If*` match rustc_transmute::TransmuteTypeEnv::new(self.infcx).is_transmutable( ObligationCause::dummy(), @@ -675,11 +676,8 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { scope, assume, ) { - 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), + Answer::Yes => Ok(Certainty::Yes), + Answer::No(_) | Answer::If(_) => 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 23126f2cb52dd..36d5f3566c6ff 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -65,6 +65,11 @@ pub struct ImplCandidate<'tcx> { pub similarity: CandidateSimilarity, } +enum GetSafeTransmuteErrorAndReason { + Silent, + Error { err_msg: String, safe_transmute_explanation: String }, +} + pub trait InferCtxtExt<'tcx> { /// Given some node representing a fn-like thing in the HIR map, /// returns a span and `ArgKind` information that describes the @@ -724,11 +729,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, - ) + ) { + GetSafeTransmuteErrorAndReason::Silent => return, + GetSafeTransmuteErrorAndReason::Error { + err_msg, + safe_transmute_explanation, + } => (err_msg, Some(safe_transmute_explanation)), + } } else { (err_msg, None) }; @@ -1292,7 +1303,7 @@ trait InferCtxtPrivExt<'tcx> { obligation: PredicateObligation<'tcx>, trait_ref: ty::PolyTraitRef<'tcx>, span: Span, - ) -> (String, Option); + ) -> GetSafeTransmuteErrorAndReason; fn add_tuple_trait_message( &self, @@ -2738,7 +2749,9 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { obligation: PredicateObligation<'tcx>, trait_ref: ty::PolyTraitRef<'tcx>, span: Span, - ) -> (String, Option) { + ) -> GetSafeTransmuteErrorAndReason { + use rustc_transmute::Answer; + // 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)); @@ -2758,13 +2771,13 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { scope, assume, ) { - Err(reason) => { + 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 safe_transmute_explanation = match reason { rustc_transmute::Reason::SrcIsUnspecified => { format!("`{src}` does not have a well-specified layout") } @@ -2794,11 +2807,21 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> { rustc_transmute::Reason::DstIsMoreUnique => { 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::SrcLayoutUnknown => { + format!("`{src}` has an unknown layout") + } + rustc_transmute::Reason::DstLayoutUnknown => { + format!("`{dst}` has an unknown layout") + } }; - (custom_err_msg, Some(reason_msg)) + GetSafeTransmuteErrorAndReason::Error { err_msg, safe_transmute_explanation } } // Should never get a Yes at this point! We already ran it before, and did not get a Yes. - Ok(None) => span_bug!( + Answer::Yes => 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 3f07ad814ff61..404055e1df6c8 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -285,28 +285,22 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { &mut self, obligation: &TraitObligation<'tcx>, ) -> Result>, SelectionError<'tcx>> { + use rustc_transmute::{Answer, Condition}; #[instrument(level = "debug", skip(tcx, obligation, predicate))] fn flatten_answer_tree<'tcx>( tcx: TyCtxt<'tcx>, obligation: &TraitObligation<'tcx>, predicate: TraitPredicate<'tcx>, - answer: rustc_transmute::Condition>, + cond: Condition>, ) -> Vec> { - match answer { + match cond { // FIXME(bryangarza): Add separate `IfAny` case, instead of treating as `IfAll` // Not possible until the trait solver supports disjunctions of obligations - rustc_transmute::Condition::IfAll(answers) - | rustc_transmute::Condition::IfAny(answers) => { - let mut nested = vec![]; - for flattened in answers - .into_iter() - .map(|answer| flatten_answer_tree(tcx, obligation, predicate, answer)) - { - nested.extend(flattened); - } - nested - } - rustc_transmute::Condition::IfTransmutable { src, dst } => { + Condition::IfAll(conds) | Condition::IfAny(conds) => conds + .into_iter() + .flat_map(|cond| flatten_answer_tree(tcx, obligation, predicate, cond)) + .collect(), + 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); @@ -333,11 +327,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { // If Dst is mutable, check bidirectionally. // For example, transmuting bool -> u8 is OK as long as you can't update that u8 // to be > 1, because you could later transmute the u8 back to a bool and get UB. - let mut obligations = vec![make_obl(src.ty, dst.ty)]; - if dst.mutability == Mutability::Mut { - obligations.push(make_obl(dst.ty, src.ty)); + match dst.mutability { + Mutability::Not => vec![make_obl(src.ty, dst.ty)], + Mutability::Mut => vec![make_obl(src.ty, dst.ty), make_obl(dst.ty, src.ty)], } - obligations } } } @@ -370,9 +363,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ); let fully_flattened = match maybe_transmutable { - Err(_) => Err(Unimplemented)?, - Ok(Some(mt)) => flatten_answer_tree(self.tcx(), obligation, predicate, mt), - Ok(None) => vec![], + Answer::No(_) => Err(Unimplemented)?, + Answer::If(cond) => flatten_answer_tree(self.tcx(), obligation, predicate, cond), + Answer::Yes => vec![], }; debug!(?fully_flattened); diff --git a/compiler/rustc_transmute/src/layout/tree.rs b/compiler/rustc_transmute/src/layout/tree.rs index ed9309b015d64..6b718be7b1564 100644 --- a/compiler/rustc_transmute/src/layout/tree.rs +++ b/compiler/rustc_transmute/src/layout/tree.rs @@ -188,14 +188,14 @@ pub(crate) mod rustc { /// The layout of the type is unspecified. Unspecified, /// This error will be surfaced elsewhere by rustc, so don't surface it. - Unknown, + UnknownLayout, TypeError(ErrorGuaranteed), } impl<'tcx> From> for Err { fn from(err: LayoutError<'tcx>) -> Self { match err { - LayoutError::Unknown(..) => Self::Unknown, + LayoutError::Unknown(..) => Self::UnknownLayout, err => unimplemented!("{:?}", err), } } diff --git a/compiler/rustc_transmute/src/lib.rs b/compiler/rustc_transmute/src/lib.rs index 7a8cbd50d450d..34ad6bd8c6923 100644 --- a/compiler/rustc_transmute/src/lib.rs +++ b/compiler/rustc_transmute/src/lib.rs @@ -19,10 +19,16 @@ pub struct Assume { pub validity: bool, } -/// Either we have an error, or we have an optional Condition that must hold. -pub type Answer = Result>, Reason>; +/// Either we have an error, transmutation is allowed, or we have an optional +/// Condition that must hold. +#[derive(Debug, Hash, Eq, PartialEq, Clone)] +pub enum Answer { + Yes, + No(Reason), + If(Condition), +} -/// A condition which must hold for safe transmutation to be possible +/// 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`. @@ -35,7 +41,7 @@ pub enum Condition { IfAny(Vec>), } -/// Answers: Why wasn't the source type transmutable into the destination type? +/// Answers "why wasn't the source type transmutable into the destination type?" #[derive(Debug, Hash, Eq, PartialEq, PartialOrd, Ord, Clone)] pub enum Reason { /// The layout of the source type is unspecified. @@ -52,6 +58,12 @@ pub enum Reason { DstHasStricterAlignment { src_min_align: usize, dst_min_align: usize }, /// Can't go from shared pointer to unique pointer DstIsMoreUnique, + /// Encountered a type error + TypeError, + /// The layout of src is unknown + SrcLayoutUnknown, + /// The layout of dst is unknown + DstLayoutUnknown, } #[cfg(feature = "rustc")] diff --git a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs index 80e3489e99bcd..b223a90f7514b 100644 --- a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs +++ b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs @@ -33,6 +33,7 @@ where Self { src, dst, scope, assume, context } } + // FIXME(bryangarza): Delete this when all usages are removed pub(crate) fn map_layouts( self, f: F, @@ -67,30 +68,26 @@ mod rustc { /// then computes an answer using those trees. #[instrument(level = "debug", skip(self), fields(src = ?self.src, dst = ?self.dst))] pub fn answer(self) -> Answer< as QueryContext>::Ref> { - let query_or_answer = self.map_layouts(|src, dst, scope, &context| { - // Convert `src` and `dst` from their rustc representations, to `Tree`-based - // representations. If these conversions fail, conclude that the transmutation is - // unacceptable; the layouts of both the source and destination types must be - // well-defined. - let src = Tree::from_ty(src, context); - let dst = Tree::from_ty(dst, context); - - match (src, dst) { - // 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(Err::Unknown), _) - | (_, Err(Err::Unknown)) => Err(Ok(None)), - (Err(Err::Unspecified), _) => Err(Err(Reason::SrcIsUnspecified)), - (_, Err(Err::Unspecified)) => Err(Err(Reason::DstIsUnspecified)), - (Ok(src), Ok(dst)) => Ok((src, dst)), + let Self { src, dst, scope, assume, context } = self; + + // Convert `src` and `dst` from their rustc representations, to `Tree`-based + // representations. If these conversions fail, conclude that the transmutation is + // unacceptable; the layouts of both the source and destination types must be + // well-defined. + let src = Tree::from_ty(src, context); + let dst = Tree::from_ty(dst, context); + + match (src, dst) { + (Err(Err::TypeError(_)), _) | (_, Err(Err::TypeError(_))) => { + Answer::No(Reason::TypeError) + } + (Err(Err::UnknownLayout), _) => Answer::No(Reason::SrcLayoutUnknown), + (_, Err(Err::UnknownLayout)) => Answer::No(Reason::DstLayoutUnknown), + (Err(Err::Unspecified), _) => Answer::No(Reason::SrcIsUnspecified), + (_, Err(Err::Unspecified)) => Answer::No(Reason::DstIsUnspecified), + (Ok(src), Ok(dst)) => { + MaybeTransmutableQuery { src, dst, scope, assume, context }.answer() } - }); - - match query_or_answer { - Ok(query) => query.answer(), - Err(answer) => answer, } } } @@ -108,6 +105,7 @@ where #[instrument(level = "debug", skip(self), fields(src = ?self.src, dst = ?self.dst))] pub(crate) fn answer(self) -> Answer<::Ref> { let assume_visibility = self.assume.safety; + // FIXME(bryangarza): Refactor this code to get rid of `map_layouts` let query_or_answer = self.map_layouts(|src, dst, scope, context| { // Remove all `Def` nodes from `src`, without checking their visibility. let src = src.prune(&|def| true); @@ -128,12 +126,13 @@ 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| Ok(None))?; + let src = Nfa::from_tree(src).map_err(|Uninhabited| Answer::Yes)?; // 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| Err(Reason::DstIsPrivate))?; + let dst = + Nfa::from_tree(dst).map_err(|Uninhabited| Answer::No(Reason::DstIsPrivate))?; Ok((src, dst)) }); @@ -155,6 +154,7 @@ where #[inline(always)] #[instrument(level = "debug", skip(self), fields(src = ?self.src, dst = ?self.dst))] pub(crate) fn answer(self) -> Answer<::Ref> { + // FIXME(bryangarza): Refactor this code to get rid of `map_layouts` let query_or_answer = self .map_layouts(|src, dst, scope, context| Ok((Dfa::from_nfa(src), Dfa::from_nfa(dst)))); @@ -226,13 +226,13 @@ where // So, if it's possible to transmute to a smaller Dst by truncating, and we can guarantee // that none of the actually-used data can introduce an invalid state for Dst's type, we // are able to safely transmute, even with truncation. - Ok(None) + Answer::Yes } 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 { - Err(Reason::DstIsTooBig) + Answer::No(Reason::DstIsTooBig) } } else { let src_quantifier = if self.assume.validity { @@ -265,7 +265,7 @@ where } else { // otherwise, we've exhausted our options. // the DFAs, from this point onwards, are bit-incompatible. - Err(Reason::DstIsBitIncompatible) + Answer::No(Reason::DstIsBitIncompatible) } }, ), @@ -282,8 +282,8 @@ where // the algoritm; only its performance. debug!(?bytes_answer); match bytes_answer { - Err(_) if !self.assume.validity => return bytes_answer, - Ok(None) if self.assume.validity => return bytes_answer, + Answer::No(_) if !self.assume.validity => return bytes_answer, + Answer::Yes if self.assume.validity => return bytes_answer, _ => {} }; @@ -299,11 +299,11 @@ where .into_iter() .map(|(&dst_ref, &dst_state_prime)| { if !src_ref.is_mutable() && dst_ref.is_mutable() { - Err(Reason::DstIsMoreUnique) + Answer::No(Reason::DstIsMoreUnique) } else if !self.assume.alignment && src_ref.min_align() < dst_ref.min_align() { - Err(Reason::DstHasStricterAlignment { + Answer::No(Reason::DstHasStricterAlignment { src_min_align: src_ref.min_align(), dst_min_align: dst_ref.min_align(), }) @@ -311,10 +311,10 @@ where // ...such that `src` is transmutable into `dst`, if // `src_ref` is transmutability into `dst_ref`. and( - Ok(Some(Condition::IfTransmutable { + Answer::If(Condition::IfTransmutable { src: src_ref, dst: dst_ref, - })), + }), self.answer_memo( cache, src_state_prime, @@ -346,65 +346,56 @@ fn and(lhs: Answer, rhs: Answer) -> Answer where R: PartialEq, { - // If both are errors, then we should return the more specific one - if lhs.is_err() && rhs.is_err() { - if lhs == Err(Reason::DstIsBitIncompatible) { - return rhs; - } else { - return lhs; - } - } - Ok(match (lhs?, rhs?) { + match (lhs, rhs) { + // If both are errors, then we should return the more specific one + (Answer::No(Reason::DstIsBitIncompatible), Answer::No(reason)) + | (Answer::No(reason), Answer::No(_)) + // If either is an error, return it + | (Answer::No(reason), _) | (_, Answer::No(reason)) => Answer::No(reason), // If only one side has a condition, pass it along - (None, other) | (other, None) => other, + | (Answer::Yes, other) | (other, Answer::Yes) => other, // If both sides have IfAll conditions, merge them - (Some(Condition::IfAll(mut lhs)), Some(Condition::IfAll(ref mut rhs))) => { + (Answer::If(Condition::IfAll(mut lhs)), Answer::If(Condition::IfAll(ref mut rhs))) => { lhs.append(rhs); - Some(Condition::IfAll(lhs)) + Answer::If(Condition::IfAll(lhs)) } // If only one side is an IfAll, add the other Condition to it - (Some(cond), Some(Condition::IfAll(mut conds))) - | (Some(Condition::IfAll(mut conds)), Some(cond)) => { + (Answer::If(cond), Answer::If(Condition::IfAll(mut conds))) + | (Answer::If(Condition::IfAll(mut conds)), Answer::If(cond)) => { conds.push(cond); - Some(Condition::IfAll(conds)) + Answer::If(Condition::IfAll(conds)) } // Otherwise, both lhs and rhs conditions can be combined in a parent IfAll - (Some(lhs), Some(rhs)) => Some(Condition::IfAll(vec![lhs, rhs])), - }) + (Answer::If(lhs), Answer::If(rhs)) => Answer::If(Condition::IfAll(vec![lhs, rhs])), + } } fn or(lhs: Answer, rhs: Answer) -> Answer where R: PartialEq, { - // If both are errors, then we should return the more specific one - if lhs.is_err() && rhs.is_err() { - if lhs == Err(Reason::DstIsBitIncompatible) { - return rhs; - } else { - return lhs; - } - } - // 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) { + match (lhs, rhs) { + // If both are errors, then we should return the more specific one + (Answer::No(Reason::DstIsBitIncompatible), Answer::No(reason)) + | (Answer::No(reason), Answer::No(_)) => Answer::No(reason), + // Otherwise, errors can be ignored for the rest of the pattern matching + (Answer::No(_), other) | (other, Answer::No(_)) => or(other, Answer::Yes), // If only one side has a condition, pass it along - (None, other) | (other, None) => other, + (Answer::Yes, other) | (other, Answer::Yes) => other, // If both sides have IfAny conditions, merge them - (Some(Condition::IfAny(mut lhs)), Some(Condition::IfAny(ref mut rhs))) => { + (Answer::If(Condition::IfAny(mut lhs)), Answer::If(Condition::IfAny(ref mut rhs))) => { lhs.append(rhs); - Some(Condition::IfAny(lhs)) + Answer::If(Condition::IfAny(lhs)) } // If only one side is an IfAny, add the other Condition to it - (Some(cond), Some(Condition::IfAny(mut conds))) - | (Some(Condition::IfAny(mut conds)), Some(cond)) => { + (Answer::If(cond), Answer::If(Condition::IfAny(mut conds))) + | (Answer::If(Condition::IfAny(mut conds)), Answer::If(cond)) => { conds.push(cond); - Some(Condition::IfAny(conds)) + Answer::If(Condition::IfAny(conds)) } // Otherwise, both lhs and rhs conditions can be combined in a parent IfAny - (Some(lhs), Some(rhs)) => Some(Condition::IfAny(vec![lhs, rhs])), - }) + (Answer::If(lhs), Answer::If(rhs)) => Answer::If(Condition::IfAny(vec![lhs, rhs])), + } } pub enum Quantifier { @@ -421,16 +412,21 @@ impl Quantifier { use std::ops::ControlFlow::{Break, Continue}; let (init, try_fold_f): (_, fn(_, _) -> _) = match self { - Self::ThereExists => (Err(Reason::DstIsBitIncompatible), |accum: Answer, next| { - match or(accum, next) { - Ok(None) => Break(Ok(None)), + Self::ThereExists => { + (Answer::No(Reason::DstIsBitIncompatible), |accum: Answer, next| { + match or(accum, next) { + Answer::Yes => Break(Answer::Yes), + maybe => Continue(maybe), + } + }) + } + Self::ForAll => (Answer::Yes, |accum: Answer, next| { + let answer = and(accum, next); + match answer { + Answer::No(_) => Break(answer), maybe => Continue(maybe), } }), - Self::ForAll => (Ok(None), |accum: Answer, next| match and(accum, next) { - Err(reason) => Break(Err(reason)), - maybe => Continue(maybe), - }), }; let (Continue(result) | Break(result)) = iter.into_iter().try_fold(init, try_fold_f); diff --git a/compiler/rustc_transmute/src/maybe_transmutable/tests.rs b/compiler/rustc_transmute/src/maybe_transmutable/tests.rs index df6a83df23f20..e49bebf571dea 100644 --- a/compiler/rustc_transmute/src/maybe_transmutable/tests.rs +++ b/compiler/rustc_transmute/src/maybe_transmutable/tests.rs @@ -4,6 +4,8 @@ use crate::{layout, Reason}; use itertools::Itertools; mod bool { + use crate::Answer; + use super::*; #[test] @@ -17,7 +19,7 @@ mod bool { UltraMinimal, ) .answer(); - assert_eq!(answer, Ok(None)); + assert_eq!(answer, Answer::Yes); } #[test] @@ -30,7 +32,7 @@ mod bool { UltraMinimal, ) .answer(); - assert_eq!(answer, Ok(None)); + assert_eq!(answer, Answer::Yes); } #[test] @@ -65,7 +67,7 @@ mod bool { if src_set.is_subset(&dst_set) { assert_eq!( - Ok(None), + Answer::Yes, MaybeTransmutableQuery::new( src_layout.clone(), dst_layout.clone(), @@ -80,7 +82,7 @@ mod bool { ); } else if !src_set.is_disjoint(&dst_set) { assert_eq!( - Ok(None), + Answer::Yes, MaybeTransmutableQuery::new( src_layout.clone(), dst_layout.clone(), @@ -95,7 +97,7 @@ mod bool { ); } else { assert_eq!( - Err(Reason::DstIsBitIncompatible), + Answer::No(Reason::DstIsBitIncompatible), MaybeTransmutableQuery::new( src_layout.clone(), dst_layout.clone(), diff --git a/tests/ui/transmutability/malformed-program-gracefulness/unknown_src_field.rs b/tests/ui/transmutability/malformed-program-gracefulness/unknown_src_field.rs index ebe34e13432b5..8d19cabc0f9f0 100644 --- a/tests/ui/transmutability/malformed-program-gracefulness/unknown_src_field.rs +++ b/tests/ui/transmutability/malformed-program-gracefulness/unknown_src_field.rs @@ -18,5 +18,5 @@ fn should_gracefully_handle_unknown_dst_field() { struct Context; #[repr(C)] struct Src; #[repr(C)] struct Dst(Missing); //~ cannot find type - assert::is_transmutable::(); + assert::is_transmutable::(); //~ ERROR cannot be safely transmuted } diff --git a/tests/ui/transmutability/malformed-program-gracefulness/unknown_src_field.stderr b/tests/ui/transmutability/malformed-program-gracefulness/unknown_src_field.stderr index 475e6f429f3c9..c2df398b8ff91 100644 --- a/tests/ui/transmutability/malformed-program-gracefulness/unknown_src_field.stderr +++ b/tests/ui/transmutability/malformed-program-gracefulness/unknown_src_field.stderr @@ -4,6 +4,22 @@ error[E0412]: cannot find type `Missing` in this scope LL | #[repr(C)] struct Dst(Missing); | ^^^^^^^ not found in this scope -error: aborting due to previous error +error[E0277]: `Src` cannot be safely transmuted into `Dst` in the defining scope of `should_gracefully_handle_unknown_dst_field::Context` + --> $DIR/unknown_src_field.rs:21:36 + | +LL | assert::is_transmutable::(); + | ^^^ `Dst` has an unknown layout + | +note: required by a bound in `is_transmutable` + --> $DIR/unknown_src_field.rs:13:14 + | +LL | pub fn is_transmutable() + | --------------- required by a bound in this function +LL | where +LL | Dst: BikeshedIntrinsicFrom + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `is_transmutable` + +error: aborting due to 2 previous errors -For more information about this error, try `rustc --explain E0412`. +Some errors have detailed explanations: E0277, E0412. +For more information about an error, try `rustc --explain E0277`.