From fdf32a04d73e41f5c57e6939a86958ecc18754ba Mon Sep 17 00:00:00 2001 From: Xiangfei Ding Date: Tue, 18 Nov 2025 21:33:23 +0000 Subject: [PATCH] DispatchFromDyn: require additional checks The new checks are we check the pair of constituent types for same shapes structurally, and demand capability for dispatch for ADTs as usual; and we demand source generic parameter to be unsized of another for sanity. Signed-off-by: Xiangfei Ding --- compiler/rustc_hir_analysis/messages.ftl | 3 + .../src/coherence/builtin.rs | 253 ++++++++++-------- compiler/rustc_hir_analysis/src/errors.rs | 7 + library/core/src/pat.rs | 2 +- .../traits/dispatch-from-dyn-invalid-impls.rs | 5 + .../dispatch-from-dyn-invalid-impls.stderr | 8 +- 6 files changed, 166 insertions(+), 112 deletions(-) diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index a313b91ef9bda..f980c2b6de936 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/compiler/rustc_hir_analysis/messages.ftl @@ -149,6 +149,9 @@ 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_multiple_refs = the trait `DispatchFromDyn` does not allow dispatch through references + .note = the trait `DispatchFromDyn` may only be implemented when dispatch goes through at most one reference to a generic + 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 diff --git a/compiler/rustc_hir_analysis/src/coherence/builtin.rs b/compiler/rustc_hir_analysis/src/coherence/builtin.rs index 61562cc1e4f30..e89db14127df7 100644 --- a/compiler/rustc_hir_analysis/src/coherence/builtin.rs +++ b/compiler/rustc_hir_analysis/src/coherence/builtin.rs @@ -203,17 +203,15 @@ fn visit_implementation_of_dispatch_from_dyn(checker: &Checker<'_>) -> Result<() let tcx = checker.tcx; let impl_did = checker.impl_def_id; let trait_ref = checker.impl_header.trait_ref.instantiate_identity(); + assert!(tcx.is_lang_item(trait_ref.def_id, LangItem::DispatchFromDyn)); + let dispatch_from_dyn_trait_did = trait_ref.def_id; debug!("visit_implementation_of_dispatch_from_dyn: impl_did={:?}", impl_did); let span = tcx.def_span(impl_did); let trait_name = "DispatchFromDyn"; - let source = trait_ref.self_ty(); - let target = { - assert!(tcx.is_lang_item(trait_ref.def_id, LangItem::DispatchFromDyn)); - - trait_ref.args.type_at(1) - }; + let mut source = trait_ref.self_ty(); + let mut target = 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. @@ -242,138 +240,173 @@ fn visit_implementation_of_dispatch_from_dyn(checker: &Checker<'_>) -> Result<() // trait, they *do* satisfy the repr(transparent) rules, and then we assume that everything else // in the compiler (in particular, all the call ABI logic) will treat them as repr(transparent) // even if they do not carry that attribute. - match (source.kind(), target.kind()) { - (&ty::Pat(_, pat_a), &ty::Pat(_, pat_b)) => { - if pat_a != pat_b { - return Err(tcx.dcx().emit_err(errors::CoerceSamePatKind { - span, - trait_name, - pat_a: pat_a.to_string(), - pat_b: pat_b.to_string(), - })); + let mut behind_ref = false; + loop { + match (source.kind(), target.kind()) { + (&ty::Pat(ty_a, pat_a), &ty::Pat(ty_b, pat_b)) => { + if pat_a != pat_b { + return Err(tcx.dcx().emit_err(errors::CoerceSamePatKind { + span, + trait_name, + pat_a: pat_a.to_string(), + pat_b: pat_b.to_string(), + })); + } + source = ty_a; + target = ty_b; } - Ok(()) - } - (&ty::Ref(r_a, _, mutbl_a), ty::Ref(r_b, _, mutbl_b)) - if r_a == *r_b && mutbl_a == *mutbl_b => - { - Ok(()) - } - (&ty::RawPtr(_, a_mutbl), &ty::RawPtr(_, b_mutbl)) if a_mutbl == b_mutbl => Ok(()), - (&ty::Adt(def_a, args_a), &ty::Adt(def_b, args_b)) - if def_a.is_struct() && def_b.is_struct() => - { - 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::CoerceSameStruct { - span, - trait_name, - note: true, - source_path, - target_path, - })); + (&ty::Ref(r_a, ty_a, mutbl_a), &ty::Ref(r_b, ty_b, mutbl_b)) + if !behind_ref && r_a == r_b && mutbl_a == mutbl_b => + { + source = ty_a; + target = ty_b; + behind_ref = true; } - - if def_a.repr().c() || def_a.repr().packed() { - return Err(tcx.dcx().emit_err(errors::DispatchFromDynRepr { span })); + (&ty::RawPtr(ty_a, a_mutbl), &ty::RawPtr(ty_b, b_mutbl)) + if !behind_ref && a_mutbl == b_mutbl => + { + source = ty_a; + target = ty_b; + behind_ref = true; } + (&ty::Adt(def_a, args_a), &ty::Adt(def_b, args_b)) + if !behind_ref && def_a.is_struct() && def_b.is_struct() => + { + 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::CoerceSameStruct { + span, + trait_name, + note: true, + source_path, + target_path, + })); + } - let fields = &def_a.non_enum_variant().fields; - - let mut res = Ok(()); - let coerced_fields = fields - .iter_enumerated() - .filter_map(|(i, field)| { - // Ignore PhantomData fields - let unnormalized_ty = tcx.type_of(field.did).instantiate_identity(); - if tcx - .try_normalize_erasing_regions( - ty::TypingEnv::non_body_analysis(tcx, def_a.did()), - unnormalized_ty, - ) - .unwrap_or(unnormalized_ty) - .is_phantom_data() - { - return None; - } + if def_a.repr().c() || def_a.repr().packed() { + return Err(tcx.dcx().emit_err(errors::DispatchFromDynRepr { span })); + } - let ty_a = field.ty(tcx, args_a); - let ty_b = field.ty(tcx, args_b); - - // FIXME: We could do normalization here, but is it really worth it? - if ty_a == ty_b { - // Allow 1-ZSTs that don't mention type params. - // - // Allowing type params here would allow us to possibly transmute - // between ZSTs, which may be used to create library unsoundness. - if let Ok(layout) = - tcx.layout_of(infcx.typing_env(param_env).as_query_input(ty_a)) - && layout.is_1zst() - && !ty_a.has_non_region_param() + let fields = &def_a.non_enum_variant().fields; + + let coerced_fields: Vec<_> = fields + .iter_enumerated() + .filter_map(|(i, field)| { + // Ignore PhantomData fields + let unnormalized_ty = tcx.type_of(field.did).instantiate_identity(); + if tcx + .try_normalize_erasing_regions( + ty::TypingEnv::non_body_analysis(tcx, def_a.did()), + unnormalized_ty, + ) + .unwrap_or(unnormalized_ty) + .is_phantom_data() { - // ignore 1-ZST fields return None; } - res = Err(tcx.dcx().emit_err(errors::DispatchFromDynZST { - span, - name: field.ident(tcx), - ty: ty_a, - })); + let ty_a = field.ty(tcx, args_a); + let ty_b = field.ty(tcx, args_b); + + // FIXME: We could do normalization here, but is it really worth it? + if ty_a == ty_b { + // Allow 1-ZSTs that don't mention type params. + // + // Allowing type params here would allow us to possibly transmute + // between ZSTs, which may be used to create library unsoundness. + if let Ok(layout) = + tcx.layout_of(infcx.typing_env(param_env).as_query_input(ty_a)) + && layout.is_1zst() + && !ty_a.has_non_region_param() + { + // ignore 1-ZST fields + return None; + } - None - } else { - Some((i, ty_a, ty_b, tcx.def_span(field.did))) + Some(Err(tcx.dcx().emit_err(errors::DispatchFromDynZST { + span, + name: field.ident(tcx), + ty: ty_a, + }))) + } else { + Some(Ok((i, ty_a, ty_b, tcx.def_span(field.did)))) + } + }) + .collect::>()?; + + if coerced_fields.is_empty() { + return Err(tcx.dcx().emit_err(errors::CoerceNoField { + span, + trait_name, + note: true, + })); + } + if let &[(_, ty_a, ty_b, field_span)] = &coerced_fields[..] { + let ocx = ObligationCtxt::new_with_diagnostics(&infcx); + ocx.register_obligation(Obligation::new( + tcx, + cause.clone(), + param_env, + ty::TraitRef::new(tcx, dispatch_from_dyn_trait_did, [ty_a, ty_b]), + )); + let errors = ocx.evaluate_obligations_error_on_ambiguity(); + if !errors.is_empty() { + 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)); + } } - }) - .collect::>(); - res?; - if coerced_fields.is_empty() { - return Err(tcx.dcx().emit_err(errors::CoerceNoField { + // Finally, resolve all regions. + ocx.resolve_regions_and_report_errors(impl_did, param_env, [])?; + + return Ok(()); + } + return Err(tcx.dcx().emit_err(errors::CoerceMulti { span, trait_name, - note: true, + number: coerced_fields.len(), + fields: coerced_fields.iter().map(|(_, _, _, s)| *s).collect::>().into(), })); - } else if let &[(_, ty_a, ty_b, field_span)] = &coerced_fields[..] { + } + (ty::Param(_), ty::Param(_)) => { let ocx = ObligationCtxt::new_with_diagnostics(&infcx); ocx.register_obligation(Obligation::new( tcx, cause.clone(), param_env, - ty::TraitRef::new(tcx, trait_ref.def_id, [ty_a, ty_b]), + ty::TraitRef::new( + tcx, + tcx.require_lang_item(LangItem::Unsize, span), + [source, target], + ), )); let errors = ocx.evaluate_obligations_error_on_ambiguity(); if !errors.is_empty() { - 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)); - } + return Err(infcx.err_ctxt().report_fulfillment_errors(errors)); } - - // Finally, resolve all regions. 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::>().into(), - })); + return Ok(()); + } + _ => { + if behind_ref { + return Err(tcx.dcx().emit_err(errors::DispatchFromDynMultiRefs { span })); + } else { + return Err(tcx + .dcx() + .emit_err(errors::CoerceUnsizedNonStruct { span, trait_name })); + } } } - _ => Err(tcx.dcx().emit_err(errors::CoerceUnsizedNonStruct { span, trait_name })), } } diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index 209b0156de323..5d58a1fd99f49 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -1258,6 +1258,13 @@ pub(crate) struct CoerceSamePatKind { pub pat_b: String, } +#[derive(Diagnostic)] +#[diag(hir_analysis_dispatch_from_dyn_multiple_refs)] +pub(crate) struct DispatchFromDynMultiRefs { + #[primary_span] + pub span: Span, +} + #[derive(Diagnostic)] #[diag(hir_analysis_coerce_unsized_may, code = E0377)] pub(crate) struct CoerceSameStruct { diff --git a/library/core/src/pat.rs b/library/core/src/pat.rs index 2670c2614198c..90ac5e2608b4e 100644 --- a/library/core/src/pat.rs +++ b/library/core/src/pat.rs @@ -82,7 +82,7 @@ impl CoerceUnsized, U> DispatchFromDyn for pattern_type!(T is !null) {} +impl + Unsize, U> DispatchFromDyn for pattern_type!(T is !null) {} impl Unpin for pattern_type!(*const T is !null) {} diff --git a/tests/ui/traits/dispatch-from-dyn-invalid-impls.rs b/tests/ui/traits/dispatch-from-dyn-invalid-impls.rs index f5f66ca69cfc1..81635ccef50ea 100644 --- a/tests/ui/traits/dispatch-from-dyn-invalid-impls.rs +++ b/tests/ui/traits/dispatch-from-dyn-invalid-impls.rs @@ -68,4 +68,9 @@ where { } +pub struct Ptr(PhantomData, Box); + +impl<'a, T: ?Sized, U: ?Sized> DispatchFromDyn<&'a Ptr> for &'a Ptr {} +//~^ ERROR the trait `DispatchFromDyn` does not allow dispatch through references + fn main() {} diff --git a/tests/ui/traits/dispatch-from-dyn-invalid-impls.stderr b/tests/ui/traits/dispatch-from-dyn-invalid-impls.stderr index 676da0ce81fb1..ed52a1072a81e 100644 --- a/tests/ui/traits/dispatch-from-dyn-invalid-impls.stderr +++ b/tests/ui/traits/dispatch-from-dyn-invalid-impls.stderr @@ -54,7 +54,13 @@ LL | | T: Unsize | = note: extra field `1` of type `OverAlignedZst` is not allowed -error: aborting due to 5 previous errors +error: the trait `DispatchFromDyn` does not allow dispatch through references + --> $DIR/dispatch-from-dyn-invalid-impls.rs:73:1 + | +LL | impl<'a, T: ?Sized, U: ?Sized> DispatchFromDyn<&'a Ptr> for &'a Ptr {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 6 previous errors Some errors have detailed explanations: E0374, E0375, E0378. For more information about an error, try `rustc --explain E0374`.