Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve safe transmute error reporting #109800

Merged
merged 1 commit into from
Apr 14, 2023
Merged

Improve safe transmute error reporting #109800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 104 additions & 24 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
return;
}
let trait_ref = trait_predicate.to_poly_trait_ref();

let (post_message, pre_message, type_def) = self
.get_parent_trait_ref(obligation.cause.code())
.map(|(t, s)| {
Expand Down Expand Up @@ -702,33 +703,45 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
(message, note, append_const_msg)
};

let mut err = struct_span_err!(
self.tcx.sess,
span,
E0277,
"{}",
message
.and_then(|cannot_do_this| {
match (predicate_is_const, append_const_msg) {
// do nothing if predicate is not const
(false, _) => Some(cannot_do_this),
// suggested using default post message
(true, Some(None)) => {
Some(format!("{cannot_do_this} in const contexts"))
}
// overridden post message
(true, Some(Some(post_message))) => {
Some(format!("{cannot_do_this}{post_message}"))
}
// fallback to generic message
(true, None) => None,
let err_msg = message
.and_then(|cannot_do_this| {
match (predicate_is_const, append_const_msg) {
// do nothing if predicate is not const
(false, _) => Some(cannot_do_this),
// suggested using default post message
(true, Some(None)) => {
Some(format!("{cannot_do_this} in const contexts"))
}
// overridden post message
(true, Some(Some(post_message))) => {
Some(format!("{cannot_do_this}{post_message}"))
}
})
.unwrap_or_else(|| format!(
// fallback to generic message
(true, None) => None,
}
})
.unwrap_or_else(|| {
format!(
"the trait bound `{}` is not satisfied{}",
trait_predicate, post_message,
))
);
)
});

let (err_msg, safe_transmute_explanation) = if Some(trait_ref.def_id())
== 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(
trait_predicate,
obligation.clone(),
trait_ref,
span,
)
} else {
(err_msg, None)
};

let mut err = struct_span_err!(self.tcx.sess, span, E0277, "{}", err_msg);

if is_try_conversion && let Some(ret_span) = self.return_type_span(&obligation) {
err.span_label(
Expand Down Expand Up @@ -818,6 +831,8 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
// at the type param with a label to suggest constraining it.
err.help(&explanation);
}
} else if let Some(custom_explanation) = safe_transmute_explanation {
err.span_label(span, custom_explanation);
} else {
err.span_label(span, explanation);
}
Expand Down Expand Up @@ -1601,6 +1616,14 @@ trait InferCtxtPrivExt<'tcx> {
obligated_types: &mut Vec<Ty<'tcx>>,
cause_code: &ObligationCauseCode<'tcx>,
) -> bool;

fn get_safe_transmute_error_and_reason(
&self,
trait_predicate: ty::Binder<'tcx, ty::TraitPredicate<'tcx>>,
obligation: Obligation<'tcx, ty::Predicate<'tcx>>,
trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>,
span: Span,
) -> (String, Option<String>);
}

impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
Expand Down Expand Up @@ -2879,6 +2902,63 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
}
false
}

fn get_safe_transmute_error_and_reason(
&self,
trait_predicate: ty::Binder<'tcx, ty::TraitPredicate<'tcx>>,
obligation: Obligation<'tcx, ty::Predicate<'tcx>>,
trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>,
span: Span,
) -> (String, Option<String>) {
let src_and_dst = trait_predicate.map_bound(|p| rustc_transmute::Types {
dst: p.trait_ref.substs.type_at(0),
src: p.trait_ref.substs.type_at(1),
});
let scope = trait_ref.skip_binder().substs.type_at(2);
let Some(assume) =
rustc_transmute::Assume::from_const(self.infcx.tcx, obligation.param_env, trait_ref.skip_binder().substs.const_at(3)) else {
span_bug!(span, "Unable to construct rustc_transmute::Assume where it was previously possible");
};
match rustc_transmute::TransmuteTypeEnv::new(self.infcx).is_transmutable(
obligation.cause,
src_and_dst,
scope,
assume,
) {
rustc_transmute::Answer::No(reason) => {
let dst = trait_ref.skip_binder().substs.type_at(0);
let src = trait_ref.skip_binder().substs.type_at(1);
let custom_err_msg = format!("`{src}` cannot be safely transmuted into `{dst}` in the defining scope of `{scope}`").to_string();
let reason_msg = match reason {
rustc_transmute::Reason::SrcIsUnspecified => {
format!("`{src}` does not have a well-specified layout").to_string()
}
rustc_transmute::Reason::DstIsUnspecified => {
format!("`{dst}` does not have a well-specified layout").to_string()
}
rustc_transmute::Reason::DstIsBitIncompatible => {
format!("At least one value of `{src}` isn't a bit-valid value of `{dst}`")
.to_string()
}
rustc_transmute::Reason::DstIsPrivate => format!(
"`{dst}` is or contains a type or field that is not visible in that scope"
)
.to_string(),
// 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}`")
}
};
(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!(
span,
"Inconsistent rustc_transmute::is_transmutable(...) result, got Yes",
),
_ => span_bug!(span, "Unsupported rustc_transmute::Reason variant"),
}
}
}

/// Crude way of getting back an `Expr` from a `Span`.
Expand Down
28 changes: 16 additions & 12 deletions compiler/rustc_transmute/src/layout/tree.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,31 +167,31 @@ where
}
}

#[derive(Debug, Copy, Clone)]
pub(crate) enum Err {
/// The layout of the type is unspecified.
Unspecified,
/// This error will be surfaced elsewhere by rustc, so don't surface it.
Unknown,
}

#[cfg(feature = "rustc")]
pub(crate) mod rustc {
use super::{Err, Tree};
use super::Tree;
use crate::layout::rustc::{Def, Ref};

use rustc_middle::ty;
use rustc_middle::ty::layout::LayoutError;
use rustc_middle::ty::util::Discr;
use rustc_middle::ty::AdtDef;
use rustc_middle::ty::ParamEnv;
use rustc_middle::ty::SubstsRef;
use rustc_middle::ty::Ty;
use rustc_middle::ty::TyCtxt;
use rustc_middle::ty::VariantDef;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt};
use rustc_span::ErrorGuaranteed;
use rustc_target::abi::Align;
use std::alloc;

#[derive(Debug, Copy, Clone)]
pub(crate) enum Err {
/// The layout of the type is unspecified.
Unspecified,
/// This error will be surfaced elsewhere by rustc, so don't surface it.
Unknown,
TypeError(ErrorGuaranteed),
}

impl<'tcx> From<LayoutError<'tcx>> for Err {
fn from(err: LayoutError<'tcx>) -> Self {
match err {
Expand Down Expand Up @@ -261,6 +261,10 @@ 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));
}

let target = tcx.data_layout();

match ty.kind() {
Expand Down
27 changes: 14 additions & 13 deletions compiler/rustc_transmute/src/maybe_transmutable/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ where
#[cfg(feature = "rustc")]
mod rustc {
use super::*;
use crate::layout::tree::Err;
use crate::layout::tree::rustc::Err;

use rustc_middle::ty::Ty;
use rustc_middle::ty::TyCtxt;
Expand All @@ -71,19 +71,20 @@ mod rustc {
// 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).map_err(|err| match err {
// Answer `Yes` here, because "Unknown Type" will already be reported by
// rustc. No need to spam the user with more errors.
Err::Unknown => Answer::Yes,
Err::Unspecified => Answer::No(Reason::SrcIsUnspecified),
})?;
let src = Tree::from_ty(src, context);
let dst = Tree::from_ty(dst, context);

let dst = Tree::from_ty(dst, context).map_err(|err| match err {
Err::Unknown => Answer::Yes,
Err::Unspecified => Answer::No(Reason::DstIsUnspecified),
})?;

Ok((src, dst))
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)),
(Ok(src), Ok(dst)) => Ok((src, dst)),
}
});

match query_or_answer {
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_transmute/src/maybe_transmutable/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::query_context::test::{Def, UltraMinimal};
use crate::maybe_transmutable::MaybeTransmutableQuery;
use crate::{layout, Answer, Reason, Set};
use crate::{layout, Answer, Reason};
use itertools::Itertools;

mod bool {
Expand Down Expand Up @@ -48,9 +48,9 @@ mod bool {

let into_set = |alts: Vec<_>| {
#[cfg(feature = "rustc")]
let mut set = Set::default();
let mut set = crate::Set::default();
#[cfg(not(feature = "rustc"))]
let mut set = Set::new();
let mut set = std::collections::HashSet::new();
set.extend(alts);
set
};
Expand Down
4 changes: 0 additions & 4 deletions library/core/src/mem/transmutability.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@
/// notwithstanding whatever safety checks you have asked the compiler to [`Assume`] are satisfied.
#[unstable(feature = "transmutability", issue = "99571")]
#[lang = "transmute_trait"]
#[rustc_on_unimplemented(
message = "`{Src}` cannot be safely transmuted into `{Self}` in the defining scope of `{Context}`.",
label = "`{Src}` cannot be safely transmuted into `{Self}` in the defining scope of `{Context}`."
)]
pub unsafe trait BikeshedIntrinsicFrom<Src, Context, const ASSUME: Assume = { Assume::NOTHING }>
where
Src: ?Sized,
Expand Down
30 changes: 12 additions & 18 deletions tests/ui/transmutability/arrays/should_require_well_defined_layout.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
error[E0277]: `[String; 0]` cannot be safely transmuted into `()` in the defining scope of `assert::Context`.
error[E0277]: `[String; 0]` cannot be safely transmuted into `()` in the defining scope of `assert::Context`
--> $DIR/should_require_well_defined_layout.rs:26:52
|
LL | assert::is_maybe_transmutable::<repr_rust, ()>();
| ^^ `[String; 0]` cannot be safely transmuted into `()` in the defining scope of `assert::Context`.
| ^^ `[String; 0]` does not have a well-specified layout
|
= help: the trait `BikeshedIntrinsicFrom<[String; 0], assert::Context, Assume { alignment: true, lifetimes: true, safety: true, validity: true }>` is not implemented for `()`
note: required by a bound in `is_maybe_transmutable`
--> $DIR/should_require_well_defined_layout.rs:13:14
|
Expand All @@ -20,13 +19,12 @@ LL | | .and(Assume::VALIDITY)
LL | | }>
| |__________^ required by this bound in `is_maybe_transmutable`

error[E0277]: `u128` cannot be safely transmuted into `[String; 0]` in the defining scope of `assert::Context`.
error[E0277]: `u128` cannot be safely transmuted into `[String; 0]` in the defining scope of `assert::Context`
--> $DIR/should_require_well_defined_layout.rs:27:47
|
LL | assert::is_maybe_transmutable::<u128, repr_rust>();
| ^^^^^^^^^ `u128` cannot be safely transmuted into `[String; 0]` in the defining scope of `assert::Context`.
| ^^^^^^^^^ `[String; 0]` does not have a well-specified layout
|
= help: the trait `BikeshedIntrinsicFrom<u128, assert::Context, Assume { alignment: true, lifetimes: true, safety: true, validity: true }>` is not implemented for `[String; 0]`
note: required by a bound in `is_maybe_transmutable`
--> $DIR/should_require_well_defined_layout.rs:13:14
|
Expand All @@ -42,13 +40,12 @@ LL | | .and(Assume::VALIDITY)
LL | | }>
| |__________^ required by this bound in `is_maybe_transmutable`

error[E0277]: `[String; 1]` cannot be safely transmuted into `()` in the defining scope of `assert::Context`.
error[E0277]: `[String; 1]` cannot be safely transmuted into `()` in the defining scope of `assert::Context`
--> $DIR/should_require_well_defined_layout.rs:32:52
|
LL | assert::is_maybe_transmutable::<repr_rust, ()>();
| ^^ `[String; 1]` cannot be safely transmuted into `()` in the defining scope of `assert::Context`.
| ^^ `[String; 1]` does not have a well-specified layout
|
= help: the trait `BikeshedIntrinsicFrom<[String; 1], assert::Context, Assume { alignment: true, lifetimes: true, safety: true, validity: true }>` is not implemented for `()`
note: required by a bound in `is_maybe_transmutable`
--> $DIR/should_require_well_defined_layout.rs:13:14
|
Expand All @@ -64,13 +61,12 @@ LL | | .and(Assume::VALIDITY)
LL | | }>
| |__________^ required by this bound in `is_maybe_transmutable`

error[E0277]: `u128` cannot be safely transmuted into `[String; 1]` in the defining scope of `assert::Context`.
error[E0277]: `u128` cannot be safely transmuted into `[String; 1]` in the defining scope of `assert::Context`
--> $DIR/should_require_well_defined_layout.rs:33:47
|
LL | assert::is_maybe_transmutable::<u128, repr_rust>();
| ^^^^^^^^^ `u128` cannot be safely transmuted into `[String; 1]` in the defining scope of `assert::Context`.
| ^^^^^^^^^ `[String; 1]` does not have a well-specified layout
|
= help: the trait `BikeshedIntrinsicFrom<u128, assert::Context, Assume { alignment: true, lifetimes: true, safety: true, validity: true }>` is not implemented for `[String; 1]`
note: required by a bound in `is_maybe_transmutable`
--> $DIR/should_require_well_defined_layout.rs:13:14
|
Expand All @@ -86,13 +82,12 @@ LL | | .and(Assume::VALIDITY)
LL | | }>
| |__________^ required by this bound in `is_maybe_transmutable`

error[E0277]: `[String; 2]` cannot be safely transmuted into `()` in the defining scope of `assert::Context`.
error[E0277]: `[String; 2]` cannot be safely transmuted into `()` in the defining scope of `assert::Context`
--> $DIR/should_require_well_defined_layout.rs:38:52
|
LL | assert::is_maybe_transmutable::<repr_rust, ()>();
| ^^ `[String; 2]` cannot be safely transmuted into `()` in the defining scope of `assert::Context`.
| ^^ `[String; 2]` does not have a well-specified layout
|
= help: the trait `BikeshedIntrinsicFrom<[String; 2], assert::Context, Assume { alignment: true, lifetimes: true, safety: true, validity: true }>` is not implemented for `()`
note: required by a bound in `is_maybe_transmutable`
--> $DIR/should_require_well_defined_layout.rs:13:14
|
Expand All @@ -108,13 +103,12 @@ LL | | .and(Assume::VALIDITY)
LL | | }>
| |__________^ required by this bound in `is_maybe_transmutable`

error[E0277]: `u128` cannot be safely transmuted into `[String; 2]` in the defining scope of `assert::Context`.
error[E0277]: `u128` cannot be safely transmuted into `[String; 2]` in the defining scope of `assert::Context`
--> $DIR/should_require_well_defined_layout.rs:39:47
|
LL | assert::is_maybe_transmutable::<u128, repr_rust>();
| ^^^^^^^^^ `u128` cannot be safely transmuted into `[String; 2]` in the defining scope of `assert::Context`.
| ^^^^^^^^^ `[String; 2]` does not have a well-specified layout
|
= help: the trait `BikeshedIntrinsicFrom<u128, assert::Context, Assume { alignment: true, lifetimes: true, safety: true, validity: true }>` is not implemented for `[String; 2]`
note: required by a bound in `is_maybe_transmutable`
--> $DIR/should_require_well_defined_layout.rs:13:14
|
Expand Down
Loading