From 9453d2cfebfbfb58753c8afef1df3686ae8c6197 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 27 Sep 2024 14:24:31 -0400 Subject: [PATCH] Fix transmute goal --- .../rustc_next_trait_solver/src/solve/mod.rs | 31 +++ .../src/solve/trait_goals.rs | 7 +- .../traits/fulfillment_errors.rs | 229 ++++++++++-------- .../src/traits/engine.rs | 11 + .../src/traits/select/confirmation.rs | 13 +- .../src/traits/structural_normalize.rs | 2 + compiler/rustc_transmute/src/lib.rs | 7 +- .../dont-assume-err-is-yes-issue-126377.rs | 19 ++ ...dont-assume-err-is-yes-issue-126377.stderr | 34 +++ 9 files changed, 236 insertions(+), 117 deletions(-) create mode 100644 tests/ui/transmutability/dont-assume-err-is-yes-issue-126377.rs create mode 100644 tests/ui/transmutability/dont-assume-err-is-yes-issue-126377.stderr diff --git a/compiler/rustc_next_trait_solver/src/solve/mod.rs b/compiler/rustc_next_trait_solver/src/solve/mod.rs index ff91fa13fd06..a475a58e483b 100644 --- a/compiler/rustc_next_trait_solver/src/solve/mod.rs +++ b/compiler/rustc_next_trait_solver/src/solve/mod.rs @@ -295,6 +295,37 @@ where Ok(ty) } } + + /// Normalize a const for when it is structurally matched on, or more likely + /// when it needs `.try_to_*` called on it (e.g. to turn it into a usize). + /// + /// This function is necessary in nearly all cases before matching on a const. + /// Not doing so is likely to be incomplete and therefore unsound during + /// coherence. + #[instrument(level = "trace", skip(self, param_env), ret)] + fn structurally_normalize_const( + &mut self, + param_env: I::ParamEnv, + ct: I::Const, + ) -> Result { + if let ty::ConstKind::Unevaluated(..) = ct.kind() { + let normalized_ct = self.next_const_infer(); + let alias_relate_goal = Goal::new( + self.cx(), + param_env, + ty::PredicateKind::AliasRelate( + ct.into(), + normalized_ct.into(), + ty::AliasRelationDirection::Equate, + ), + ); + self.add_goal(GoalSource::Misc, alias_relate_goal); + self.try_evaluate_added_goals()?; + Ok(self.resolve_vars_if_possible(normalized_ct)) + } else { + Ok(ct) + } + } } fn response_no_constraints_raw( diff --git a/compiler/rustc_next_trait_solver/src/solve/trait_goals.rs b/compiler/rustc_next_trait_solver/src/solve/trait_goals.rs index 2cbed0bceb2d..a8d6536baadd 100644 --- a/compiler/rustc_next_trait_solver/src/solve/trait_goals.rs +++ b/compiler/rustc_next_trait_solver/src/solve/trait_goals.rs @@ -627,11 +627,16 @@ where } ecx.probe_builtin_trait_candidate(BuiltinImplSource::Misc).enter(|ecx| { + let assume = ecx.structurally_normalize_const( + goal.param_env, + goal.predicate.trait_ref.args.const_at(2), + )?; + let certainty = ecx.is_transmutable( goal.param_env, goal.predicate.trait_ref.args.type_at(0), goal.predicate.trait_ref.args.type_at(1), - goal.predicate.trait_ref.args.const_at(2), + assume, )?; ecx.evaluate_added_goals_and_make_canonical_response(certainty) }) diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs index 5076152dbff8..d8279c6a77a4 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs @@ -2247,124 +2247,143 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { span: Span, ) -> GetSafeTransmuteErrorAndReason { use rustc_transmute::Answer; + self.probe(|_| { + // We don't assemble a transmutability candidate for types that are generic + // and we should have ambiguity for types that still have non-region infer. + if obligation.predicate.has_non_region_param() || obligation.has_non_region_infer() { + return GetSafeTransmuteErrorAndReason::Default; + } - // We don't assemble a transmutability candidate for types that are generic - // and we should have ambiguity for types that still have non-region infer. - if obligation.predicate.has_non_region_param() || obligation.has_non_region_infer() { - return GetSafeTransmuteErrorAndReason::Default; - } + // Erase regions because layout code doesn't particularly care about regions. + let trait_ref = + self.tcx.erase_regions(self.tcx.instantiate_bound_regions_with_erased(trait_ref)); - // Erase regions because layout code doesn't particularly care about regions. - let trait_ref = - self.tcx.erase_regions(self.tcx.instantiate_bound_regions_with_erased(trait_ref)); + let src_and_dst = rustc_transmute::Types { + dst: trait_ref.args.type_at(0), + src: trait_ref.args.type_at(1), + }; - let src_and_dst = rustc_transmute::Types { - dst: trait_ref.args.type_at(0), - src: trait_ref.args.type_at(1), - }; - let Some(assume) = rustc_transmute::Assume::from_const( - self.infcx.tcx, - obligation.param_env, - trait_ref.args.const_at(2), - ) else { - self.dcx().span_delayed_bug( - span, - "Unable to construct rustc_transmute::Assume where it was previously possible", - ); - return GetSafeTransmuteErrorAndReason::Silent; - }; + let ocx = ObligationCtxt::new(self); + let Ok(assume) = ocx.structurally_normalize_const( + &obligation.cause, + obligation.param_env, + trait_ref.args.const_at(2), + ) else { + self.dcx().span_delayed_bug( + span, + "Unable to construct rustc_transmute::Assume where it was previously possible", + ); + return GetSafeTransmuteErrorAndReason::Silent; + }; - let dst = trait_ref.args.type_at(0); - let src = trait_ref.args.type_at(1); + let Some(assume) = + rustc_transmute::Assume::from_const(self.infcx.tcx, obligation.param_env, assume) + else { + self.dcx().span_delayed_bug( + span, + "Unable to construct rustc_transmute::Assume where it was previously possible", + ); + return GetSafeTransmuteErrorAndReason::Silent; + }; - let err_msg = format!("`{src}` cannot be safely transmuted into `{dst}`"); + let dst = trait_ref.args.type_at(0); + let src = trait_ref.args.type_at(1); + let err_msg = format!("`{src}` cannot be safely transmuted into `{dst}`"); - match rustc_transmute::TransmuteTypeEnv::new(self.infcx).is_transmutable( - obligation.cause, - src_and_dst, - assume, - ) { - Answer::No(reason) => { - let safe_transmute_explanation = match reason { - rustc_transmute::Reason::SrcIsNotYetSupported => { - format!("analyzing the transmutability of `{src}` is not yet supported") - } + match rustc_transmute::TransmuteTypeEnv::new(self.infcx).is_transmutable( + obligation.cause, + src_and_dst, + assume, + ) { + Answer::No(reason) => { + let safe_transmute_explanation = match reason { + rustc_transmute::Reason::SrcIsNotYetSupported => { + format!("analyzing the transmutability of `{src}` is not yet supported") + } - rustc_transmute::Reason::DstIsNotYetSupported => { - format!("analyzing the transmutability of `{dst}` is not yet supported") - } + rustc_transmute::Reason::DstIsNotYetSupported => { + format!("analyzing the transmutability of `{dst}` is not yet supported") + } - rustc_transmute::Reason::DstIsBitIncompatible => { - format!("at least one value of `{src}` isn't a bit-valid value of `{dst}`") - } + rustc_transmute::Reason::DstIsBitIncompatible => { + format!( + "at least one value of `{src}` isn't a bit-valid value of `{dst}`" + ) + } - rustc_transmute::Reason::DstUninhabited => { - format!("`{dst}` is uninhabited") - } + rustc_transmute::Reason::DstUninhabited => { + format!("`{dst}` is uninhabited") + } - rustc_transmute::Reason::DstMayHaveSafetyInvariants => { - format!("`{dst}` may carry safety invariants") + rustc_transmute::Reason::DstMayHaveSafetyInvariants => { + format!("`{dst}` may carry safety invariants") + } + rustc_transmute::Reason::DstIsTooBig => { + format!("the size of `{src}` is smaller than the size of `{dst}`") + } + rustc_transmute::Reason::DstRefIsTooBig { src, dst } => { + let src_size = src.size; + let dst_size = dst.size; + format!( + "the referent size of `{src}` ({src_size} bytes) \ + is smaller than that of `{dst}` ({dst_size} bytes)" + ) + } + rustc_transmute::Reason::SrcSizeOverflow => { + format!( + "values of the type `{src}` are too big for the target architecture" + ) + } + rustc_transmute::Reason::DstSizeOverflow => { + format!( + "values of the type `{dst}` are too big for the target architecture" + ) + } + rustc_transmute::Reason::DstHasStricterAlignment { + src_min_align, + dst_min_align, + } => { + format!( + "the minimum alignment of `{src}` ({src_min_align}) should \ + be greater than that of `{dst}` ({dst_min_align})" + ) + } + 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") + } + }; + GetSafeTransmuteErrorAndReason::Error { + err_msg, + safe_transmute_explanation: Some(safe_transmute_explanation), } - rustc_transmute::Reason::DstIsTooBig => { - format!("the size of `{src}` is smaller than the size of `{dst}`") - } - rustc_transmute::Reason::DstRefIsTooBig { src, dst } => { - let src_size = src.size; - let dst_size = dst.size; - format!( - "the referent size of `{src}` ({src_size} bytes) is smaller than that of `{dst}` ({dst_size} bytes)" - ) - } - rustc_transmute::Reason::SrcSizeOverflow => { - format!( - "values of the type `{src}` are too big for the target architecture" - ) - } - rustc_transmute::Reason::DstSizeOverflow => { - format!( - "values of the type `{dst}` are too big for the target architecture" - ) - } - rustc_transmute::Reason::DstHasStricterAlignment { - src_min_align, - dst_min_align, - } => { - format!( - "the minimum alignment of `{src}` ({src_min_align}) should be greater than that of `{dst}` ({dst_min_align})" - ) - } - 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") - } - }; - GetSafeTransmuteErrorAndReason::Error { - err_msg, - safe_transmute_explanation: Some(safe_transmute_explanation), } + // Should never get a Yes at this point! We already ran it before, and did not get a Yes. + Answer::Yes => span_bug!( + span, + "Inconsistent rustc_transmute::is_transmutable(...) result, got Yes", + ), + // Reached when a different obligation (namely `Freeze`) causes the + // transmutability analysis to fail. In this case, silence the + // transmutability error message in favor of that more specific + // error. + Answer::If(_) => GetSafeTransmuteErrorAndReason::Error { + err_msg, + safe_transmute_explanation: None, + }, } - // Should never get a Yes at this point! We already ran it before, and did not get a Yes. - Answer::Yes => span_bug!( - span, - "Inconsistent rustc_transmute::is_transmutable(...) result, got Yes", - ), - // Reached when a different obligation (namely `Freeze`) causes the - // transmutability analysis to fail. In this case, silence the - // transmutability error message in favor of that more specific - // error. - Answer::If(_) => { - GetSafeTransmuteErrorAndReason::Error { err_msg, safe_transmute_explanation: None } - } - } + }) } /// For effects predicates such as `::Effects: Compat`, pretend that the diff --git a/compiler/rustc_trait_selection/src/traits/engine.rs b/compiler/rustc_trait_selection/src/traits/engine.rs index a45ec8b3c449..5e270b62b008 100644 --- a/compiler/rustc_trait_selection/src/traits/engine.rs +++ b/compiler/rustc_trait_selection/src/traits/engine.rs @@ -329,4 +329,15 @@ where .at(cause, param_env) .structurally_normalize(value, &mut **self.engine.borrow_mut()) } + + pub fn structurally_normalize_const( + &self, + cause: &ObligationCause<'tcx>, + param_env: ty::ParamEnv<'tcx>, + value: ty::Const<'tcx>, + ) -> Result, Vec> { + self.infcx + .at(cause, param_env) + .structurally_normalize_const(value, &mut **self.engine.borrow_mut()) + } } diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 0ba3b4e6e551..f78cd6c1aa0a 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -405,11 +405,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { let predicate = obligation.predicate.skip_binder(); - let Some(assume) = rustc_transmute::Assume::from_const( - self.infcx.tcx, - obligation.param_env, - predicate.trait_ref.args.const_at(2), - ) else { + let mut assume = predicate.trait_ref.args.const_at(2); + // FIXME(min_generic_const_exprs): We should shallowly normalize this. + if self.tcx().features().generic_const_exprs { + assume = assume.normalize_internal(self.tcx(), obligation.param_env); + } + let Some(assume) = + rustc_transmute::Assume::from_const(self.infcx.tcx, obligation.param_env, assume) + else { return Err(Unimplemented); }; diff --git a/compiler/rustc_trait_selection/src/traits/structural_normalize.rs b/compiler/rustc_trait_selection/src/traits/structural_normalize.rs index 3814f8112e92..f8d98eb856e8 100644 --- a/compiler/rustc_trait_selection/src/traits/structural_normalize.rs +++ b/compiler/rustc_trait_selection/src/traits/structural_normalize.rs @@ -82,6 +82,8 @@ impl<'tcx> At<'_, 'tcx> { } Ok(self.infcx.resolve_vars_if_possible(new_infer_ct)) + } else if self.infcx.tcx.features().generic_const_exprs { + Ok(ct.normalize_internal(self.infcx.tcx, self.param_env)) } else { Ok(self.normalize(ct).into_value_registering_obligations(self.infcx, fulfill_cx)) } diff --git a/compiler/rustc_transmute/src/lib.rs b/compiler/rustc_transmute/src/lib.rs index 7796ab9d6919..f7651e49cdd1 100644 --- a/compiler/rustc_transmute/src/lib.rs +++ b/compiler/rustc_transmute/src/lib.rs @@ -134,12 +134,7 @@ mod rustc { use rustc_span::symbol::sym; let Some((cv, ty)) = c.try_to_valtree() else { - return Some(Self { - alignment: true, - lifetimes: true, - safety: true, - validity: true, - }); + return None; }; let adt_def = ty.ty_adt_def()?; diff --git a/tests/ui/transmutability/dont-assume-err-is-yes-issue-126377.rs b/tests/ui/transmutability/dont-assume-err-is-yes-issue-126377.rs new file mode 100644 index 000000000000..eeb0777c8566 --- /dev/null +++ b/tests/ui/transmutability/dont-assume-err-is-yes-issue-126377.rs @@ -0,0 +1,19 @@ +#![feature(transmutability)] +#![feature(generic_const_exprs)] +//~^ WARN the feature `generic_const_exprs` is incomplete + +use std::mem::{Assume, TransmuteFrom}; + +pub fn is_transmutable() +where + (): TransmuteFrom<(), { Assume::SAFETY }>, +{ +} + +fn foo() { + is_transmutable::<{}>(); + //~^ ERROR the trait bound `(): TransmuteFrom<(), { Assume::SAFETY }>` is not satisfied + //~| ERROR mismatched types +} + +fn main() {} diff --git a/tests/ui/transmutability/dont-assume-err-is-yes-issue-126377.stderr b/tests/ui/transmutability/dont-assume-err-is-yes-issue-126377.stderr new file mode 100644 index 000000000000..6cb6a85c78a6 --- /dev/null +++ b/tests/ui/transmutability/dont-assume-err-is-yes-issue-126377.stderr @@ -0,0 +1,34 @@ +warning: the feature `generic_const_exprs` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/dont-assume-err-is-yes-issue-126377.rs:2:12 + | +LL | #![feature(generic_const_exprs)] + | ^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #76560 for more information + = note: `#[warn(incomplete_features)]` on by default + +error[E0308]: mismatched types + --> $DIR/dont-assume-err-is-yes-issue-126377.rs:14:23 + | +LL | is_transmutable::<{}>(); + | ^^ expected `bool`, found `()` + +error[E0277]: the trait bound `(): TransmuteFrom<(), { Assume::SAFETY }>` is not satisfied + --> $DIR/dont-assume-err-is-yes-issue-126377.rs:14:23 + | +LL | is_transmutable::<{}>(); + | ^^ the trait `TransmuteFrom<(), { Assume::SAFETY }>` is not implemented for `()` + | +note: required by a bound in `is_transmutable` + --> $DIR/dont-assume-err-is-yes-issue-126377.rs:9:9 + | +LL | pub fn is_transmutable() + | --------------- required by a bound in this function +LL | where +LL | (): TransmuteFrom<(), { Assume::SAFETY }>, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `is_transmutable` + +error: aborting due to 2 previous errors; 1 warning emitted + +Some errors have detailed explanations: E0277, E0308. +For more information about an error, try `rustc --explain E0277`.