Skip to content

Commit

Permalink
Safe Transmute: Fix ICE (Inconsistent is_transmutable result)
Browse files Browse the repository at this point in the history
This patch updates the code that constructs the `Assume` type to return an
error instead of silently falling back to a default `Assume`. This fixes an ICE
where error reporting would get a different `is_transmutable` result that is
inconsistent with the original one computed during trait confirmation.

Fixes rust-lang#110969
  • Loading branch information
bryangarza committed Jul 20, 2023
1 parent 39f42ad commit 98ddc0f
Show file tree
Hide file tree
Showing 12 changed files with 147 additions and 45 deletions.
10 changes: 6 additions & 4 deletions compiler/rustc_trait_selection/src/solve/trait_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -599,10 +599,12 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
// which will ICE for region vars.
let args = ecx.tcx().erase_regions(goal.predicate.trait_ref.args);

let Some(assume) =
rustc_transmute::Assume::from_const(ecx.tcx(), goal.param_env, args.const_at(3))
else {
return Err(NoSolution);
let maybe_assume =
rustc_transmute::Assume::from_const(ecx.tcx(), goal.param_env, args.const_at(3));
let assume = match maybe_assume {
Some(Ok(assume)) => assume,
Some(Err(_guar)) => return Err(NoSolution),
None => return Err(NoSolution),
};

let certainty = ecx.is_transmutable(
Expand Down
27 changes: 17 additions & 10 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,9 @@ pub struct ImplCandidate<'tcx> {
}

enum GetSafeTransmuteErrorAndReason {
Silent,
ErrorGuaranteed(ErrorGuaranteed),
// Unable to compute Safe Transmute result because of ambiguity
Ambiguous,
Error { err_msg: String, safe_transmute_explanation: String },
}

Expand Down Expand Up @@ -756,7 +758,10 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
trait_ref,
span,
) {
GetSafeTransmuteErrorAndReason::Silent => return,
GetSafeTransmuteErrorAndReason::ErrorGuaranteed(_guar) => return,
// Unable to compute whether Safe Transmute is possible (for example, due to an unevaluated const).
// The same thing occurred during trait selection/confirmation, so there is no error to report here.
GetSafeTransmuteErrorAndReason::Ambiguous => return,
GetSafeTransmuteErrorAndReason::Error {
err_msg,
safe_transmute_explanation,
Expand Down Expand Up @@ -2884,15 +2889,17 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
src: trait_ref.args.type_at(1),
};
let scope = trait_ref.args.type_at(2);
let Some(assume) = rustc_transmute::Assume::from_const(

let maybe_assume = rustc_transmute::Assume::from_const(
self.infcx.tcx,
obligation.param_env,
trait_ref.args.const_at(3),
) else {
span_bug!(
span,
"Unable to construct rustc_transmute::Assume where it was previously possible"
);
);

let assume = match maybe_assume {
Some(Ok(assume)) => assume,
Some(Err(guar)) => return GetSafeTransmuteErrorAndReason::ErrorGuaranteed(guar),
None => return GetSafeTransmuteErrorAndReason::Ambiguous,
};

match rustc_transmute::TransmuteTypeEnv::new(self.infcx).is_transmutable(
Expand Down Expand Up @@ -2938,8 +2945,8 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
format!("`{src}` is a shared reference, but `{dst}` is a unique reference")
}
// Already reported by rustc
rustc_transmute::Reason::TypeError => {
return GetSafeTransmuteErrorAndReason::Silent;
rustc_transmute::Reason::TypeError(guar) => {
return GetSafeTransmuteErrorAndReason::ErrorGuaranteed(guar);
}
rustc_transmute::Reason::SrcLayoutUnknown => {
format!("`{src}` has an unknown layout")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
let predicate =
self.tcx().erase_regions(self.tcx().erase_late_bound_regions(obligation.predicate));

let Some(assume) = rustc_transmute::Assume::from_const(
let Some(Ok(assume)) = rustc_transmute::Assume::from_const(
self.infcx.tcx,
obligation.param_env,
predicate.trait_ref.args.const_at(3),
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_transmute/src/layout/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,8 @@ pub(crate) mod rustc {
use rustc_middle::ty::UintTy::*;
use rustc_target::abi::HasDataLayout;

if let Err(e) = ty.error_reported() {
return Err(Err::TypeError(e));
if let Err(guar) = ty.error_reported() {
return Err(Err::TypeError(guar));
}

let target = tcx.data_layout();
Expand Down
27 changes: 8 additions & 19 deletions compiler/rustc_transmute/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ pub enum Reason {
/// Can't go from shared pointer to unique pointer
DstIsMoreUnique,
/// Encountered a type error
TypeError,
TypeError(ErrorGuaranteed),
/// The layout of src is unknown
SrcLayoutUnknown,
/// The layout of dst is unknown
Expand All @@ -79,6 +79,7 @@ mod rustc {
use rustc_middle::ty::Ty;
use rustc_middle::ty::TyCtxt;
use rustc_middle::ty::ValTree;
use rustc_span::ErrorGuaranteed;

/// The source and destination types of a transmutation.
#[derive(TypeVisitable, Debug, Clone, Copy)]
Expand Down Expand Up @@ -123,20 +124,14 @@ mod rustc {
tcx: TyCtxt<'tcx>,
param_env: ParamEnv<'tcx>,
c: Const<'tcx>,
) -> Option<Self> {
) -> Option<Result<Self, ErrorGuaranteed>> {
use rustc_middle::ty::ScalarInt;
use rustc_middle::ty::TypeVisitableExt;
use rustc_span::symbol::sym;

let c = c.eval(tcx, param_env);

if let Err(err) = c.error_reported() {
return Some(Self {
alignment: true,
lifetimes: true,
safety: true,
validity: true,
});
return Some(Err(err));
}

let adt_def = c.ty().ty_adt_def()?;
Expand All @@ -151,14 +146,7 @@ mod rustc {
let variant = adt_def.non_enum_variant();
let fields = match c.try_to_valtree() {
Some(ValTree::Branch(branch)) => branch,
_ => {
return Some(Self {
alignment: true,
lifetimes: true,
safety: true,
validity: true,
});
}
_ => return None,
};

let get_field = |name| {
Expand All @@ -171,15 +159,16 @@ mod rustc {
fields[field_idx].unwrap_leaf() == ScalarInt::TRUE
};

Some(Self {
Some(Ok(Self {
alignment: get_field(sym::alignment),
lifetimes: get_field(sym::lifetimes),
safety: get_field(sym::safety),
validity: get_field(sym::validity),
})
}))
}
}
}

#[cfg(feature = "rustc")]
pub use rustc::*;
use rustc_span::ErrorGuaranteed;
4 changes: 2 additions & 2 deletions compiler/rustc_transmute/src/maybe_transmutable/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ mod rustc {
let dst = Tree::from_ty(dst, context);

match (src, dst) {
(Err(Err::TypeError(_)), _) | (_, Err(Err::TypeError(_))) => {
Answer::No(Reason::TypeError)
(Err(Err::TypeError(guar)), _) | (_, Err(Err::TypeError(guar))) => {
Answer::No(Reason::TypeError(guar))
}
(Err(Err::UnknownLayout), _) => Answer::No(Reason::SrcLayoutUnknown),
(_, Err(Err::UnknownLayout)) => Answer::No(Reason::DstLayoutUnknown),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
error: expected parameter name, found `,`
--> $DIR/issue-110892.rs:27:9
--> $DIR/issue-110892.rs:29:9
|
LL | ,
| ^ expected parameter name

error: expected parameter name, found `,`
--> $DIR/issue-110892.rs:28:9
--> $DIR/issue-110892.rs:30:9
|
LL | ,
| ^ expected parameter name

error: expected parameter name, found `,`
--> $DIR/issue-110892.rs:29:9
--> $DIR/issue-110892.rs:31:9
|
LL | ,
| ^ expected parameter name

error: expected parameter name, found `,`
--> $DIR/issue-110892.rs:30:9
--> $DIR/issue-110892.rs:32:9
|
LL | ,
| ^ expected parameter name

error[E0308]: mismatched types
--> $DIR/issue-110892.rs:31:10
--> $DIR/issue-110892.rs:33:10
|
LL | const fn from_options(
| ------------ implicitly returns `()` as its body has no tail or `return` expression
Expand Down
42 changes: 42 additions & 0 deletions tests/ui/transmutability/issue-110892.next.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
error: expected parameter name, found `,`
--> $DIR/issue-110892.rs:29:9
|
LL | ,
| ^ expected parameter name

error: expected parameter name, found `,`
--> $DIR/issue-110892.rs:30:9
|
LL | ,
| ^ expected parameter name

error: expected parameter name, found `,`
--> $DIR/issue-110892.rs:31:9
|
LL | ,
| ^ expected parameter name

error: expected parameter name, found `,`
--> $DIR/issue-110892.rs:32:9
|
LL | ,
| ^ expected parameter name

error[E0284]: type annotations needed: cannot satisfy `the constant `{ from_options(ASSUME_ALIGNMENT, ASSUME_LIFETIMES, ASSUME_SAFETY, ASSUME_VALIDITY) }` can be evaluated`
--> $DIR/issue-110892.rs:23:13
|
LL | { from_options(ASSUME_ALIGNMENT, ASSUME_LIFETIMES, ASSUME_SAFETY, ASSUME_VALIDITY) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot satisfy `the constant `{ from_options(ASSUME_ALIGNMENT, ASSUME_LIFETIMES, ASSUME_SAFETY, ASSUME_VALIDITY) }` can be evaluated`
|
note: required by a bound in `is_transmutable`
--> $DIR/issue-110892.rs:23:13
|
LL | pub fn is_transmutable<
| --------------- required by a bound in this function
...
LL | { from_options(ASSUME_ALIGNMENT, ASSUME_LIFETIMES, ASSUME_SAFETY, ASSUME_VALIDITY) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `is_transmutable`

error: aborting due to 5 previous errors

For more information about this error, try `rustc --explain E0284`.
6 changes: 4 additions & 2 deletions tests/ui/transmutability/issue-110892.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// check-fail
// revisions: current next
//[next] compile-flags: -Ztrait-solver=next
#![feature(generic_const_exprs, transmutability)]
#![allow(incomplete_features)]

Expand All @@ -18,7 +20,7 @@ mod assert {
Dst: BikeshedIntrinsicFrom<
Src,
Context,
{ from_options(ASSUME_ALIGNMENT, ASSUME_LIFETIMES, ASSUME_SAFETY, ASSUME_VALIDITY) }
{ from_options(ASSUME_ALIGNMENT, ASSUME_LIFETIMES, ASSUME_SAFETY, ASSUME_VALIDITY) } //[next]~ ERROR type annotations needed
>,
{}

Expand All @@ -28,7 +30,7 @@ mod assert {
, //~ ERROR expected parameter name, found `,`
, //~ ERROR expected parameter name, found `,`
, //~ ERROR expected parameter name, found `,`
) -> Assume {} //~ ERROR mismatched types
) -> Assume {} //[current]~ ERROR mismatched types
}

fn main() {
Expand Down
15 changes: 15 additions & 0 deletions tests/ui/transmutability/issue-110969.current.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error[E0308]: mismatched types
--> $DIR/issue-110969.rs:26:74
|
LL | const FALSE: bool = assert::is_transmutable::<Src, Dst, Context, {}>();
| ^^ expected `Assume`, found `()`

error[E0308]: mismatched types
--> $DIR/issue-110969.rs:26:29
|
LL | const FALSE: bool = assert::is_transmutable::<Src, Dst, Context, {}>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `bool`, found `()`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0308`.
15 changes: 15 additions & 0 deletions tests/ui/transmutability/issue-110969.next.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error[E0308]: mismatched types
--> $DIR/issue-110969.rs:26:74
|
LL | const FALSE: bool = assert::is_transmutable::<Src, Dst, Context, {}>();
| ^^ expected `Assume`, found `()`

error[E0308]: mismatched types
--> $DIR/issue-110969.rs:26:29
|
LL | const FALSE: bool = assert::is_transmutable::<Src, Dst, Context, {}>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `bool`, found `()`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0308`.
30 changes: 30 additions & 0 deletions tests/ui/transmutability/issue-110969.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// check-fail
// revisions: current next
//[next] compile-flags: -Ztrait-solver=next
#![feature(adt_const_params, generic_const_exprs, transmutability)]
#![allow(incomplete_features)]

mod assert {
use std::mem::BikeshedIntrinsicFrom;

pub fn is_transmutable<Src, Dst, Context, const ASSUME: std::mem::Assume>()
where
Dst: BikeshedIntrinsicFrom<Src, Context, ASSUME>,
{
}
}

fn main() {
struct Context;
#[repr(C)]
struct Src;
#[repr(C)]
struct Dst;

trait Trait {
// The `{}` should not cause an ICE
const FALSE: bool = assert::is_transmutable::<Src, Dst, Context, {}>();
//~^ ERROR mismatched types
//~^^ ERROR mismatched types
}
}

0 comments on commit 98ddc0f

Please sign in to comment.