bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #137289 - compiler-errors:coerce-unsized-errors, r=oli-obk

Browse source 
Consolidate and improve error messaging for `CoerceUnsized` and `DispatchFromDyn`

Firstly, this PR consolidates and reworks the error diagnostics for `CoercePointee` and `DispatchFromDyn`. There was a ton of duplication for no reason -- this reworks both the errors and also the error codes, since they can be shared between both traits since they report the same thing.

Secondly, when encountering a struct with multiple fields that must be coerced, point out the field spans, rather than mentioning the fields by name. This makes the error message clearer, but also means that we don't mention the `__S` dummy parameter for `derive(CoercePointee)`.

Thirdly, emit a custom error message when we encounter a trait error that comes from the recursive field `CoerceUnsized`/`DispatchFromDyn` trait check. **Note:** This is the only one I'm not too satisfied with -- I think it could use some more refinement, but ideally it explains that the field must be an unsize-able pointer... Feedback welcome.

Finally, don't emit `DispatchFromDyn` validity errors if we detect `CoerceUnsized` validity errors from an impl of the same ADT.

This is best reviewed per commit.

r? `@oli-obk` perhaps?

cc `@dingxiangfei2009` -- sorry for making my own attempt at this PR, but I wanted to see if I could implement a fix for #136796 in a less complicated way, since communicating over github review comments can be a bit slow. I'll leave comments inline to explain my thinking about the diagnostics changes.
This commit is contained in:
Michael Goulet 2025-02-24 19:21:45 -05:00 • committed by GitHub
commit 0bb00e2085
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
22 changed files with 313 additions and 315 deletions
Download patch file Download diff file Expand all files Collapse all files

16
compiler/rustc_hir_analysis/messages.ftl
View file

@ -85,6 +85,10 @@ hir_analysis_cmse_output_stack_spill =
.note1 = functions with the `{$abi}` ABI must pass their result via the available return registers
.note2 = the result must either be a (transparently wrapped) i64, u64 or f64, or be at most 4 bytes in size
hir_analysis_coerce_multi = implementing `{$trait_name}` does not allow multiple fields to be coerced
.note = the trait `{$trait_name}` may only be implemented when a single field is being coerced
.label = these fields must be coerced for `{$trait_name}` to be valid
hir_analysis_coerce_pointee_no_field = `CoercePointee` can only be derived on `struct`s with at least one field
hir_analysis_coerce_pointee_no_user_validity_assertion = asserting applicability of `derive(CoercePointee)` on a target data is forbidden
@ -95,12 +99,12 @@ hir_analysis_coerce_pointee_not_struct = `derive(CoercePointee)` is only applica
hir_analysis_coerce_pointee_not_transparent = `derive(CoercePointee)` is only applicable to `struct` with `repr(transparent)` layout
hir_analysis_coerce_unsized_field_validity = for `{$ty}` to have a valid implementation of `{$trait_name}`, it must be possible to coerce the field of type `{$field_ty}`
.label = `{$field_ty}` must be a pointer, reference, or smart pointer that is allowed to be unsized
hir_analysis_coerce_unsized_may = the trait `{$trait_name}` may only be implemented for a coercion between structures
hir_analysis_coerce_unsized_multi = implementing the trait `CoerceUnsized` requires multiple coercions
.note = `CoerceUnsized` may only be implemented for a coercion between structures with one field being coerced
.coercions_note = currently, {$number} fields need coercions: {$coercions}
.label = requires multiple coercions
hir_analysis_coerce_zero = implementing `{$trait_name}` requires a field to be coerced
hir_analysis_coercion_between_struct_same_note = expected coercion between the same definition; expected `{$source_path}`, found `{$target_path}`
@ -139,10 +143,6 @@ hir_analysis_cross_crate_traits = cross-crate traits with a default impl, like `
hir_analysis_cross_crate_traits_defined = cross-crate traits with a default impl, like `{$traits}`, can only be implemented for a struct/enum type defined in the current crate
.label = can't implement cross-crate trait for type in another crate
hir_analysis_dispatch_from_dyn_multi = implementing the `DispatchFromDyn` trait requires multiple coercions
.note = the trait `DispatchFromDyn` may only be implemented for a coercion between structures with a single field being coerced
.coercions_note = currently, {$number} fields need coercions: {$coercions}
hir_analysis_dispatch_from_dyn_repr = structs implementing `DispatchFromDyn` may not have `#[repr(packed)]` or `#[repr(C)]`
hir_analysis_dispatch_from_dyn_zst = the trait `DispatchFromDyn` may only be implemented for structs containing the field being coerced, ZST fields with 1 byte alignment that don't mention type/const generics, and nothing else

170
compiler/rustc_hir_analysis/src/coherence/builtin.rs
View file

@ -17,7 +17,7 @@ use rustc_middle::ty::print::PrintTraitRefExt as _;
use rustc_middle::ty::{
self, Ty, TyCtxt, TypeVisitableExt, TypingMode, suggest_constraining_type_params,
};
use rustc_span::{DUMMY_SP, Span};
use rustc_span::{DUMMY_SP, Span, sym};
use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
use rustc_trait_selection::traits::misc::{
ConstParamTyImplementationError, CopyImplementationError, InfringingFieldsReason,
@ -195,8 +195,14 @@ fn visit_implementation_of_coerce_unsized(checker: &Checker<'_>) -> Result<(), E
// Just compute this for the side-effects, in particular reporting
// errors; other parts of the code may demand it for the info of
// course.
let span = tcx.def_span(impl_did);
tcx.at(span).ensure_ok().coerce_unsized_info(impl_did)
tcx.ensure_ok().coerce_unsized_info(impl_did)
}
fn is_from_coerce_pointee_derive(tcx: TyCtxt<'_>, span: Span) -> bool {
span.ctxt()
.outer_expn_data()
.macro_def_id
.is_some_and(|def_id| tcx.is_diagnostic_item(sym::CoercePointee, def_id))
}
fn visit_implementation_of_dispatch_from_dyn(checker: &Checker<'_>) -> Result<(), ErrorGuaranteed> {
@ -206,17 +212,29 @@ fn visit_implementation_of_dispatch_from_dyn(checker: &Checker<'_>) -> Result<()
debug!("visit_implementation_of_dispatch_from_dyn: impl_did={:?}", impl_did);
let span = tcx.def_span(impl_did);
let trait_name = "DispatchFromDyn";
let dispatch_from_dyn_trait = tcx.require_lang_item(LangItem::DispatchFromDyn, Some(span));
let source = trait_ref.self_ty();
assert!(!source.has_escaping_bound_vars());
let target = {
assert_eq!(trait_ref.def_id, dispatch_from_dyn_trait);
trait_ref.args.type_at(1)
};
// Check `CoercePointee` impl is WF -- if not, then there's no reason to report
// redundant errors for `DispatchFromDyn`. This is best effort, though.
let mut res = Ok(());
tcx.for_each_relevant_impl(
tcx.require_lang_item(LangItem::CoerceUnsized, Some(span)),
source,
|impl_def_id| {
res = res.and(tcx.ensure_ok().coerce_unsized_info(impl_def_id));
},
);
res?;
debug!("visit_implementation_of_dispatch_from_dyn: {:?} -> {:?}", source, target);
let param_env = tcx.param_env(impl_did);
@ -242,26 +260,25 @@ fn visit_implementation_of_dispatch_from_dyn(checker: &Checker<'_>) -> Result<()
if def_a != def_b {
let source_path = tcx.def_path_str(def_a.did());
let target_path = tcx.def_path_str(def_b.did());
return Err(tcx.dcx().emit_err(errors::DispatchFromDynCoercion {
return Err(tcx.dcx().emit_err(errors::CoerceSameStruct {
span,
trait_name: "DispatchFromDyn",
trait_name,
note: true,
source_path,
target_path,
}));
}
let mut res = Ok(());
if def_a.repr().c() || def_a.repr().packed() {
res = Err(tcx.dcx().emit_err(errors::DispatchFromDynRepr { span }));
return Err(tcx.dcx().emit_err(errors::DispatchFromDynRepr { span }));
}
let fields = &def_a.non_enum_variant().fields;
let mut res = Ok(());
let coerced_fields = fields
.iter()
.filter(|field| {
.iter_enumerated()
.filter_map(|(i, field)| {
// Ignore PhantomData fields
let unnormalized_ty = tcx.type_of(field.did).instantiate_identity();
if tcx
@ -272,7 +289,7 @@ fn visit_implementation_of_dispatch_from_dyn(checker: &Checker<'_>) -> Result<()
.unwrap_or(unnormalized_ty)
.is_phantom_data()
{
return false;
return None;
}
let ty_a = field.ty(tcx, args_a);
@ -290,7 +307,7 @@ fn visit_implementation_of_dispatch_from_dyn(checker: &Checker<'_>) -> Result<()
&& !ty_a.has_non_region_param()
{
// ignore 1-ZST fields
return false;
return None;
}
res = Err(tcx.dcx().emit_err(errors::DispatchFromDynZST {
@ -299,64 +316,57 @@ fn visit_implementation_of_dispatch_from_dyn(checker: &Checker<'_>) -> Result<()
ty: ty_a,
}));
return false;
None
} else {
Some((i, ty_a, ty_b, tcx.def_span(field.did)))
}
true
})
.collect::<Vec<_>>();
res?;
if coerced_fields.is_empty() {
res = Err(tcx.dcx().emit_err(errors::DispatchFromDynSingle {
return Err(tcx.dcx().emit_err(errors::CoerceNoField {
span,
trait_name: "DispatchFromDyn",
trait_name,
note: true,
}));
} else if coerced_fields.len() > 1 {
res = Err(tcx.dcx().emit_err(errors::DispatchFromDynMulti {
span,
coercions_note: true,
number: coerced_fields.len(),
coercions: coerced_fields
.iter()
.map(|field| {
format!(
"`{}` (`{}` to `{}`)",
field.name,
field.ty(tcx, args_a),
field.ty(tcx, args_b),
)
})
.collect::<Vec<_>>()
.join(", "),
}));
} else {
} else if let &[(_, ty_a, ty_b, field_span)] = &coerced_fields[..] {
let ocx = ObligationCtxt::new_with_diagnostics(&infcx);
for field in coerced_fields {
ocx.register_obligation(Obligation::new(
tcx,
cause.clone(),
param_env,
ty::TraitRef::new(
tcx,
dispatch_from_dyn_trait,
[field.ty(tcx, args_a), field.ty(tcx, args_b)],
),
));
}
ocx.register_obligation(Obligation::new(
tcx,
cause.clone(),
param_env,
ty::TraitRef::new(tcx, dispatch_from_dyn_trait, [ty_a, ty_b]),
));
let errors = ocx.select_all_or_error();
if !errors.is_empty() {
res = Err(infcx.err_ctxt().report_fulfillment_errors(errors));
if is_from_coerce_pointee_derive(tcx, span) {
return Err(tcx.dcx().emit_err(errors::CoerceFieldValidity {
span,
trait_name,
ty: trait_ref.self_ty(),
field_span,
field_ty: ty_a,
}));
} else {
return Err(infcx.err_ctxt().report_fulfillment_errors(errors));
}
}
// Finally, resolve all regions.
res = res.and(ocx.resolve_regions_and_report_errors(impl_did, param_env, []));
ocx.resolve_regions_and_report_errors(impl_did, param_env, [])?;
Ok(())
} else {
return Err(tcx.dcx().emit_err(errors::CoerceMulti {
span,
trait_name,
number: coerced_fields.len(),
fields: coerced_fields.iter().map(|(_, _, _, s)| *s).collect::<Vec<_>>().into(),
}));
}
res
}
_ => Err(tcx
.dcx()
.emit_err(errors::CoerceUnsizedMay { span, trait_name: "DispatchFromDyn" })),
_ => Err(tcx.dcx().emit_err(errors::CoerceUnsizedNonStruct { span, trait_name })),
}
}
@ -366,13 +376,14 @@ pub(crate) fn coerce_unsized_info<'tcx>(
) -> Result<CoerceUnsizedInfo, ErrorGuaranteed> {
debug!("compute_coerce_unsized_info(impl_did={:?})", impl_did);
let span = tcx.def_span(impl_did);
let trait_name = "CoerceUnsized";
let coerce_unsized_trait = tcx.require_lang_item(LangItem::CoerceUnsized, Some(span));
let unsize_trait = tcx.require_lang_item(LangItem::Unsize, Some(span));
let source = tcx.type_of(impl_did).instantiate_identity();
let trait_ref = tcx.impl_trait_ref(impl_did).unwrap().instantiate_identity();
assert_eq!(trait_ref.def_id, coerce_unsized_trait);
let target = trait_ref.args.type_at(1);
debug!("visit_implementation_of_coerce_unsized: {:?} -> {:?} (bound)", source, target);
@ -399,9 +410,9 @@ pub(crate) fn coerce_unsized_info<'tcx>(
)
.emit();
}
(mt_a.ty, mt_b.ty, unsize_trait, None)
(mt_a.ty, mt_b.ty, unsize_trait, None, span)
};
let (source, target, trait_def_id, kind) = match (source.kind(), target.kind()) {
let (source, target, trait_def_id, kind, field_span) = match (source.kind(), target.kind()) {
(&ty::Ref(r_a, ty_a, mutbl_a), &ty::Ref(r_b, ty_b, mutbl_b)) => {
infcx.sub_regions(infer::RelateObjectBound(span), r_b, r_a);
let mt_a = ty::TypeAndMut { ty: ty_a, mutbl: mutbl_a };
@ -422,9 +433,9 @@ pub(crate) fn coerce_unsized_info<'tcx>(
if def_a != def_b {
let source_path = tcx.def_path_str(def_a.did());
let target_path = tcx.def_path_str(def_b.did());
return Err(tcx.dcx().emit_err(errors::DispatchFromDynSame {
return Err(tcx.dcx().emit_err(errors::CoerceSameStruct {
span,
trait_name: "CoerceUnsized",
trait_name,
note: true,
source_path,
target_path,
@ -504,14 +515,14 @@ pub(crate) fn coerce_unsized_info<'tcx>(
// Collect up all fields that were significantly changed
// i.e., those that contain T in coerce_unsized T -> U
Some((i, a, b))
Some((i, a, b, tcx.def_span(f.did)))
})
.collect::<Vec<_>>();
if diff_fields.is_empty() {
return Err(tcx.dcx().emit_err(errors::CoerceUnsizedOneField {
return Err(tcx.dcx().emit_err(errors::CoerceNoField {
span,
trait_name: "CoerceUnsized",
trait_name,
note: true,
}));
} else if diff_fields.len() > 1 {
@ -522,27 +533,21 @@ pub(crate) fn coerce_unsized_info<'tcx>(
tcx.def_span(impl_did)
};
return Err(tcx.dcx().emit_err(errors::CoerceUnsizedMulti {
return Err(tcx.dcx().emit_err(errors::CoerceMulti {
span,
coercions_note: true,
trait_name,
number: diff_fields.len(),
coercions: diff_fields
.iter()
.map(|&(i, a, b)| format!("`{}` (`{}` to `{}`)", fields[i].name, a, b))
.collect::<Vec<_>>()
.join(", "),
fields: diff_fields.iter().map(|(_, _, _, s)| *s).collect::<Vec<_>>().into(),
}));
}
let (i, a, b) = diff_fields[0];
let (i, a, b, field_span) = diff_fields[0];
let kind = ty::adjustment::CustomCoerceUnsized::Struct(i);
(a, b, coerce_unsized_trait, Some(kind))
(a, b, coerce_unsized_trait, Some(kind), field_span)
}
_ => {
return Err(tcx
.dcx()
.emit_err(errors::DispatchFromDynStruct { span, trait_name: "CoerceUnsized" }));
return Err(tcx.dcx().emit_err(errors::CoerceUnsizedNonStruct { span, trait_name }));
}
};
@ -557,12 +562,23 @@ pub(crate) fn coerce_unsized_info<'tcx>(
);
ocx.register_obligation(obligation);
let errors = ocx.select_all_or_error();
if !errors.is_empty() {
infcx.err_ctxt().report_fulfillment_errors(errors);
if is_from_coerce_pointee_derive(tcx, span) {
return Err(tcx.dcx().emit_err(errors::CoerceFieldValidity {
span,
trait_name,
ty: trait_ref.self_ty(),
field_span,
field_ty: source,
}));
} else {
return Err(infcx.err_ctxt().report_fulfillment_errors(errors));
}
}
// Finally, resolve all regions.
let _ = ocx.resolve_regions_and_report_errors(impl_did, param_env, []);
ocx.resolve_regions_and_report_errors(impl_did, param_env, [])?;
Ok(CoerceUnsizedInfo { custom_kind: kind })
}

80
compiler/rustc_hir_analysis/src/errors.rs
View file

@ -1164,18 +1164,6 @@ pub(crate) struct InherentTyOutside {
pub span: Span,
}
#[derive(Diagnostic)]
#[diag(hir_analysis_coerce_unsized_may, code = E0378)]
pub(crate) struct DispatchFromDynCoercion<'a> {
#[primary_span]
pub span: Span,
pub trait_name: &'a str,
#[note(hir_analysis_coercion_between_struct_same_note)]
pub note: bool,
pub source_path: String,
pub target_path: String,
}
#[derive(Diagnostic)]
#[diag(hir_analysis_dispatch_from_dyn_repr, code = E0378)]
pub(crate) struct DispatchFromDynRepr {
@ -1293,41 +1281,40 @@ pub(crate) struct DispatchFromDynZST<'a> {
}
#[derive(Diagnostic)]
#[diag(hir_analysis_coerce_unsized_may, code = E0378)]
pub(crate) struct DispatchFromDynSingle<'a> {
#[diag(hir_analysis_coerce_zero, code = E0374)]
pub(crate) struct CoerceNoField {
#[primary_span]
pub span: Span,
pub trait_name: &'a str,
pub trait_name: &'static str,
#[note(hir_analysis_coercion_between_struct_single_note)]
pub note: bool,
}
#[derive(Diagnostic)]
#[diag(hir_analysis_dispatch_from_dyn_multi, code = E0378)]
#[note]
pub(crate) struct DispatchFromDynMulti {
#[diag(hir_analysis_coerce_multi, code = E0375)]
pub(crate) struct CoerceMulti {
pub trait_name: &'static str,
#[primary_span]
pub span: Span,
#[note(hir_analysis_coercions_note)]
pub coercions_note: bool,
pub number: usize,
pub coercions: String,
}
#[derive(Diagnostic)]
#[diag(hir_analysis_coerce_unsized_may, code = E0376)]
pub(crate) struct DispatchFromDynStruct<'a> {
#[primary_span]
pub span: Span,
pub trait_name: &'a str,
#[note]
pub fields: MultiSpan,
}
#[derive(Diagnostic)]
#[diag(hir_analysis_coerce_unsized_may, code = E0377)]
pub(crate) struct DispatchFromDynSame<'a> {
pub(crate) struct CoerceUnsizedNonStruct {
#[primary_span]
pub span: Span,
pub trait_name: &'a str,
pub trait_name: &'static str,
}
#[derive(Diagnostic)]
#[diag(hir_analysis_coerce_unsized_may, code = E0377)]
pub(crate) struct CoerceSameStruct {
#[primary_span]
pub span: Span,
pub trait_name: &'static str,
#[note(hir_analysis_coercion_between_struct_same_note)]
pub note: bool,
pub source_path: String,
@ -1335,34 +1322,15 @@ pub(crate) struct DispatchFromDynSame<'a> {
}
#[derive(Diagnostic)]
#[diag(hir_analysis_coerce_unsized_may, code = E0374)]
pub(crate) struct CoerceUnsizedOneField<'a> {
#[diag(hir_analysis_coerce_unsized_field_validity)]
pub(crate) struct CoerceFieldValidity<'tcx> {
#[primary_span]
pub span: Span,
pub trait_name: &'a str,
#[note(hir_analysis_coercion_between_struct_single_note)]
pub note: bool,
}
#[derive(Diagnostic)]
#[diag(hir_analysis_coerce_unsized_multi, code = E0375)]
#[note]
pub(crate) struct CoerceUnsizedMulti {
#[primary_span]
pub ty: Ty<'tcx>,
pub trait_name: &'static str,
#[label]
pub span: Span,
#[note(hir_analysis_coercions_note)]
pub coercions_note: bool,
pub number: usize,
pub coercions: String,
}
#[derive(Diagnostic)]
#[diag(hir_analysis_coerce_unsized_may, code = E0378)]
pub(crate) struct CoerceUnsizedMay<'a> {
#[primary_span]
pub span: Span,
pub trait_name: &'a str,
pub field_span: Span,
pub field_ty: Ty<'tcx>,
}
#[derive(Diagnostic)]