Skip to content

Commit 36febe1

Browse files
bryangarzabryangarza
committed
Improve safe transmute error reporting
This patch updates the error reporting when Safe Transmute is not possible between 2 types by including the reason. Also, fix some small bugs that occur when computing the `Answer` for transmutability.
1 parent 59a05ad commit 36febe1

29 files changed

+495
-593
lines changed

‎compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

Lines changed: 104 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
663663
return;
664664
}
665665
let trait_ref = trait_predicate.to_poly_trait_ref();
666+
666667
let (post_message, pre_message, type_def) = self
667668
.get_parent_trait_ref(obligation.cause.code())
668669
.map(|(t, s)| {
@@ -702,33 +703,45 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
702703
(message, note, append_const_msg)
703704
};
704705

705-
let mut err = struct_span_err!(
706-
self.tcx.sess,
707-
span,
708-
E0277,
709-
"{}",
710-
message
711-
.and_then(|cannot_do_this| {
712-
match (predicate_is_const, append_const_msg) {
713-
// do nothing if predicate is not const
714-
(false, _) => Some(cannot_do_this),
715-
// suggested using default post message
716-
(true, Some(None)) => {
717-
Some(format!("{cannot_do_this} in const contexts"))
718-
}
719-
// overridden post message
720-
(true, Some(Some(post_message))) => {
721-
Some(format!("{cannot_do_this}{post_message}"))
722-
}
723-
// fallback to generic message
724-
(true, None) => None,
706+
let err_msg = message
707+
.and_then(|cannot_do_this| {
708+
match (predicate_is_const, append_const_msg) {
709+
// do nothing if predicate is not const
710+
(false, _) => Some(cannot_do_this),
711+
// suggested using default post message
712+
(true, Some(None)) => {
713+
Some(format!("{cannot_do_this} in const contexts"))
714+
}
715+
// overridden post message
716+
(true, Some(Some(post_message))) => {
717+
Some(format!("{cannot_do_this}{post_message}"))
725718
}
726-
})
727-
.unwrap_or_else(|| format!(
719+
// fallback to generic message
720+
(true, None) => None,
721+
}
722+
})
723+
.unwrap_or_else(|| {
724+
format!(
728725
"the trait bound `{}` is not satisfied{}",
729726
trait_predicate, post_message,
730-
))
731-
);
727+
)
728+
});
729+
730+
let (err_msg, safe_transmute_explanation) = if Some(trait_ref.def_id())
731+
== self.tcx.lang_items().transmute_trait()
732+
{
733+
// Recompute the safe transmute reason and use that for the error reporting
734+
self.get_safe_transmute_error_and_reason(
735+
trait_predicate,
736+
obligation.clone(),
737+
trait_ref,
738+
span,
739+
)
740+
} else {
741+
(err_msg, None)
742+
};
743+
744+
let mut err = struct_span_err!(self.tcx.sess, span, E0277, "{}", err_msg);
732745

733746
if is_try_conversion && let Some(ret_span) = self.return_type_span(&obligation) {
734747
err.span_label(
@@ -818,6 +831,8 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
818831
// at the type param with a label to suggest constraining it.
819832
err.help(&explanation);
820833
}
834+
} else if let Some(custom_explanation) = safe_transmute_explanation {
835+
err.span_label(span, custom_explanation);
821836
} else {
822837
err.span_label(span, explanation);
823838
}
@@ -1601,6 +1616,14 @@ trait InferCtxtPrivExt<'tcx> {
16011616
obligated_types: &mut Vec<Ty<'tcx>>,
16021617
cause_code: &ObligationCauseCode<'tcx>,
16031618
) -> bool;
1619+
1620+
fn get_safe_transmute_error_and_reason(
1621+
&self,
1622+
trait_predicate: ty::Binder<'tcx, ty::TraitPredicate<'tcx>>,
1623+
obligation: Obligation<'tcx, ty::Predicate<'tcx>>,
1624+
trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>,
1625+
span: Span,
1626+
) -> (String, Option<String>);
16041627
}
16051628

16061629
impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
@@ -2879,6 +2902,63 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
28792902
}
28802903
false
28812904
}
2905+
2906+
fn get_safe_transmute_error_and_reason(
2907+
&self,
2908+
trait_predicate: ty::Binder<'tcx, ty::TraitPredicate<'tcx>>,
2909+
obligation: Obligation<'tcx, ty::Predicate<'tcx>>,
2910+
trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>,
2911+
span: Span,
2912+
) -> (String, Option<String>) {
2913+
let src_and_dst = trait_predicate.map_bound(|p| rustc_transmute::Types {
2914+
dst: p.trait_ref.substs.type_at(0),
2915+
src: p.trait_ref.substs.type_at(1),
2916+
});
2917+
let scope = trait_ref.skip_binder().substs.type_at(2);
2918+
let Some(assume) =
2919+
rustc_transmute::Assume::from_const(self.infcx.tcx, obligation.param_env, trait_ref.skip_binder().substs.const_at(3)) else {
2920+
span_bug!(span, "Unable to construct rustc_transmute::Assume where it was previously possible");
2921+
};
2922+
match rustc_transmute::TransmuteTypeEnv::new(self.infcx).is_transmutable(
2923+
obligation.cause,
2924+
src_and_dst,
2925+
scope,
2926+
assume,
2927+
) {
2928+
rustc_transmute::Answer::No(reason) => {
2929+
let dst = trait_ref.skip_binder().substs.type_at(0);
2930+
let src = trait_ref.skip_binder().substs.type_at(1);
2931+
let custom_err_msg = format!("`{src}` cannot be safely transmuted into `{dst}` in the defining scope of `{scope}`").to_string();
2932+
let reason_msg = match reason {
2933+
rustc_transmute::Reason::SrcIsUnspecified => {
2934+
format!("`{src}` does not have a well-specified layout").to_string()
2935+
}
2936+
rustc_transmute::Reason::DstIsUnspecified => {
2937+
format!("`{dst}` does not have a well-specified layout").to_string()
2938+
}
2939+
rustc_transmute::Reason::DstIsBitIncompatible => {
2940+
format!("At least one value of `{src}` isn't a bit-valid value of `{dst}`")
2941+
.to_string()
2942+
}
2943+
rustc_transmute::Reason::DstIsPrivate => format!(
2944+
"`{dst}` is or contains a type or field that is not visible in that scope"
2945+
)
2946+
.to_string(),
2947+
// FIXME(bryangarza): Include the number of bytes of src and dst
2948+
rustc_transmute::Reason::DstIsTooBig => {
2949+
format!("The size of `{src}` is smaller than the size of `{dst}`")
2950+
}
2951+
};
2952+
(custom_err_msg, Some(reason_msg))
2953+
}
2954+
// Should never get a Yes at this point! We already ran it before, and did not get a Yes.
2955+
rustc_transmute::Answer::Yes => span_bug!(
2956+
span,
2957+
"Inconsistent rustc_transmute::is_transmutable(...) result, got Yes",
2958+
),
2959+
_ => span_bug!(span, "Unsupported rustc_transmute::Reason variant"),
2960+
}
2961+
}
28822962
}
28832963

28842964
/// Crude way of getting back an `Expr` from a `Span`.

‎compiler/rustc_transmute/src/layout/tree.rs

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -167,31 +167,31 @@ where
167167
}
168168
}
169169

170-
#[derive(Debug, Copy, Clone)]
171-
pub(crate) enum Err {
172-
/// The layout of the type is unspecified.
173-
Unspecified,
174-
/// This error will be surfaced elsewhere by rustc, so don't surface it.
175-
Unknown,
176-
}
177-
178170
#[cfg(feature = "rustc")]
179171
pub(crate) mod rustc {
180-
use super::{Err, Tree};
172+
use super::Tree;
181173
use crate::layout::rustc::{Def, Ref};
182174

183-
use rustc_middle::ty;
184175
use rustc_middle::ty::layout::LayoutError;
185176
use rustc_middle::ty::util::Discr;
186177
use rustc_middle::ty::AdtDef;
187178
use rustc_middle::ty::ParamEnv;
188179
use rustc_middle::ty::SubstsRef;
189-
use rustc_middle::ty::Ty;
190-
use rustc_middle::ty::TyCtxt;
191180
use rustc_middle::ty::VariantDef;
181+
use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt};
182+
use rustc_span::ErrorGuaranteed;
192183
use rustc_target::abi::Align;
193184
use std::alloc;
194185

186+
#[derive(Debug, Copy, Clone)]
187+
pub(crate) enum Err {
188+
/// The layout of the type is unspecified.
189+
Unspecified,
190+
/// This error will be surfaced elsewhere by rustc, so don't surface it.
191+
Unknown,
192+
TypeError(ErrorGuaranteed),
193+
}
194+
195195
impl<'tcx> From<LayoutError<'tcx>> for Err {
196196
fn from(err: LayoutError<'tcx>) -> Self {
197197
match err {
@@ -261,6 +261,10 @@ pub(crate) mod rustc {
261261
use rustc_middle::ty::UintTy::*;
262262
use rustc_target::abi::HasDataLayout;
263263

264+
if let Err(e) = ty.error_reported() {
265+
return Err(Err::TypeError(e));
266+
}
267+
264268
let target = tcx.data_layout();
265269

266270
match ty.kind() {

‎compiler/rustc_transmute/src/maybe_transmutable/mod.rs

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ where
5656
#[cfg(feature = "rustc")]
5757
mod rustc {
5858
use super::*;
59-
use crate::layout::tree::Err;
59+
use crate::layout::tree::rustc::Err;
6060

6161
use rustc_middle::ty::Ty;
6262
use rustc_middle::ty::TyCtxt;
@@ -71,19 +71,20 @@ mod rustc {
7171
// representations. If these conversions fail, conclude that the transmutation is
7272
// unacceptable; the layouts of both the source and destination types must be
7373
// well-defined.
74-
let src = Tree::from_ty(src, context).map_err(|err| match err {
75-
// Answer `Yes` here, because "Unknown Type" will already be reported by
76-
// rustc. No need to spam the user with more errors.
77-
Err::Unknown => Answer::Yes,
78-
Err::Unspecified => Answer::No(Reason::SrcIsUnspecified),
79-
})?;
74+
let src = Tree::from_ty(src, context);
75+
let dst = Tree::from_ty(dst, context);
8076

81-
let dst = Tree::from_ty(dst, context).map_err(|err| match err {
82-
Err::Unknown => Answer::Yes,
83-
Err::Unspecified => Answer::No(Reason::DstIsUnspecified),
84-
})?;
85-
86-
Ok((src, dst))
77+
match (src, dst) {
78+
// Answer `Yes` here, because 'unknown layout' and type errors will already
79+
// be reported by rustc. No need to spam the user with more errors.
80+
(Err(Err::TypeError(_)), _) => Err(Answer::Yes),
81+
(_, Err(Err::TypeError(_))) => Err(Answer::Yes),
82+
(Err(Err::Unknown), _) => Err(Answer::Yes),
83+
(_, Err(Err::Unknown)) => Err(Answer::Yes),
84+
(Err(Err::Unspecified), _) => Err(Answer::No(Reason::SrcIsUnspecified)),
85+
(_, Err(Err::Unspecified)) => Err(Answer::No(Reason::DstIsUnspecified)),
86+
(Ok(src), Ok(dst)) => Ok((src, dst)),
87+
}
8788
});
8889

8990
match query_or_answer {

‎compiler/rustc_transmute/src/maybe_transmutable/tests.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use super::query_context::test::{Def, UltraMinimal};
22
use crate::maybe_transmutable::MaybeTransmutableQuery;
3-
use crate::{layout, Answer, Reason, Set};
3+
use crate::{layout, Answer, Reason};
44
use itertools::Itertools;
55

66
mod bool {
@@ -48,9 +48,9 @@ mod bool {
4848

4949
let into_set = |alts: Vec<_>| {
5050
#[cfg(feature = "rustc")]
51-
let mut set = Set::default();
51+
let mut set = crate::Set::default();
5252
#[cfg(not(feature = "rustc"))]
53-
let mut set = Set::new();
53+
let mut set = std::collections::HashSet::new();
5454
set.extend(alts);
5555
set
5656
};

‎library/core/src/mem/transmutability.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,6 @@
55
/// notwithstanding whatever safety checks you have asked the compiler to [`Assume`] are satisfied.
66
#[unstable(feature = "transmutability", issue = "99571")]
77
#[lang = "transmute_trait"]
8-
#[rustc_on_unimplemented(
9-
message = "`{Src}` cannot be safely transmuted into `{Self}` in the defining scope of `{Context}`.",
10-
label = "`{Src}` cannot be safely transmuted into `{Self}` in the defining scope of `{Context}`."
11-
)]
128
pub unsafe trait BikeshedIntrinsicFrom<Src, Context, const ASSUME: Assume = { Assume::NOTHING }>
139
where
1410
Src: ?Sized,

‎tests/ui/transmutability/arrays/should_require_well_defined_layout.stderr

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
1-
error[E0277]: `[String; 0]` cannot be safely transmuted into `()` in the defining scope of `assert::Context`.
1+
error[E0277]: `[String; 0]` cannot be safely transmuted into `()` in the defining scope of `assert::Context`
22
--> $DIR/should_require_well_defined_layout.rs:26:52
33
|
44
LL | assert::is_maybe_transmutable::<repr_rust, ()>();
5-
| ^^ `[String; 0]` cannot be safely transmuted into `()` in the defining scope of `assert::Context`.
5+
| ^^ `[String; 0]` does not have a well-specified layout
66
|
7-
= help: the trait `BikeshedIntrinsicFrom<[String; 0], assert::Context, Assume { alignment: true, lifetimes: true, safety: true, validity: true }>` is not implemented for `()`
87
note: required by a bound in `is_maybe_transmutable`
98
--> $DIR/should_require_well_defined_layout.rs:13:14
109
|
@@ -20,13 +19,12 @@ LL | | .and(Assume::VALIDITY)
2019
LL | | }>
2120
| |__________^ required by this bound in `is_maybe_transmutable`
2221

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

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

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

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

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

0 commit comments

Comments
 (0)