Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit c6223e1

Browse files
matthiaskrgrmatthiaskrgr
authoredApr 14, 2023
Rollup merge of #109800 - bryangarza:safe-transmute-improved-errors, r=compiler-errors
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.
2 parents ec08676 + 36febe1 commit c6223e1

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
@@ -673,6 +673,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
673673
return;
674674
}
675675
let trait_ref = trait_predicate.to_poly_trait_ref();
676+
676677
let (post_message, pre_message, type_def) = self
677678
.get_parent_trait_ref(obligation.cause.code())
678679
.map(|(t, s)| {
@@ -712,33 +713,45 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
712713
(message, note, append_const_msg)
713714
};
714715

715-
let mut err = struct_span_err!(
716-
self.tcx.sess,
717-
span,
718-
E0277,
719-
"{}",
720-
message
721-
.and_then(|cannot_do_this| {
722-
match (predicate_is_const, append_const_msg) {
723-
// do nothing if predicate is not const
724-
(false, _) => Some(cannot_do_this),
725-
// suggested using default post message
726-
(true, Some(None)) => {
727-
Some(format!("{cannot_do_this} in const contexts"))
728-
}
729-
// overridden post message
730-
(true, Some(Some(post_message))) => {
731-
Some(format!("{cannot_do_this}{post_message}"))
732-
}
733-
// fallback to generic message
734-
(true, None) => None,
716+
let err_msg = message
717+
.and_then(|cannot_do_this| {
718+
match (predicate_is_const, append_const_msg) {
719+
// do nothing if predicate is not const
720+
(false, _) => Some(cannot_do_this),
721+
// suggested using default post message
722+
(true, Some(None)) => {
723+
Some(format!("{cannot_do_this} in const contexts"))
724+
}
725+
// overridden post message
726+
(true, Some(Some(post_message))) => {
727+
Some(format!("{cannot_do_this}{post_message}"))
735728
}
736-
})
737-
.unwrap_or_else(|| format!(
729+
// fallback to generic message
730+
(true, None) => None,
731+
}
732+
})
733+
.unwrap_or_else(|| {
734+
format!(
738735
"the trait bound `{}` is not satisfied{}",
739736
trait_predicate, post_message,
740-
))
741-
);
737+
)
738+
});
739+
740+
let (err_msg, safe_transmute_explanation) = if Some(trait_ref.def_id())
741+
== self.tcx.lang_items().transmute_trait()
742+
{
743+
// Recompute the safe transmute reason and use that for the error reporting
744+
self.get_safe_transmute_error_and_reason(
745+
trait_predicate,
746+
obligation.clone(),
747+
trait_ref,
748+
span,
749+
)
750+
} else {
751+
(err_msg, None)
752+
};
753+
754+
let mut err = struct_span_err!(self.tcx.sess, span, E0277, "{}", err_msg);
742755

743756
if is_try_conversion && let Some(ret_span) = self.return_type_span(&obligation) {
744757
err.span_label(
@@ -828,6 +841,8 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
828841
// at the type param with a label to suggest constraining it.
829842
err.help(&explanation);
830843
}
844+
} else if let Some(custom_explanation) = safe_transmute_explanation {
845+
err.span_label(span, custom_explanation);
831846
} else {
832847
err.span_label(span, explanation);
833848
}
@@ -1611,6 +1626,14 @@ trait InferCtxtPrivExt<'tcx> {
16111626
obligated_types: &mut Vec<Ty<'tcx>>,
16121627
cause_code: &ObligationCauseCode<'tcx>,
16131628
) -> bool;
1629+
1630+
fn get_safe_transmute_error_and_reason(
1631+
&self,
1632+
trait_predicate: ty::Binder<'tcx, ty::TraitPredicate<'tcx>>,
1633+
obligation: Obligation<'tcx, ty::Predicate<'tcx>>,
1634+
trait_ref: ty::Binder<'tcx, ty::TraitRef<'tcx>>,
1635+
span: Span,
1636+
) -> (String, Option<String>);
16141637
}
16151638

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

29002980
/// 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)