diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index bcf4d7f59e34a..abf941d796336 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 f8809e051b600..42400897de781 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/compiler/rustc_hir_analysis/src/errors.rs @@ -1247,6 +1247,14 @@ pub(crate) struct CoerceSamePatKind { pub pat_b: String, } +#[derive(Diagnostic)] +#[diag(hir_analysis_dispatch_from_dyn_multiple_refs)] +#[note] +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/ops/unsize.rs b/library/core/src/ops/unsize.rs index f0781ee01fd53..8eb5eef99abe0 100644 --- a/library/core/src/ops/unsize.rs +++ b/library/core/src/ops/unsize.rs @@ -71,35 +71,59 @@ impl, U: PointeeSized> CoerceUnsized<*const U> for * /// `DispatchFromDyn` is used in the implementation of dyn-compatibility[^1] checks (specifically /// allowing arbitrary self types), to guarantee that a method's receiver type can be dispatched on. /// -/// Note: `DispatchFromDyn` was briefly named `CoerceSized` (and had a slightly different -/// interpretation). +/// *Note*: `DispatchFromDyn` was briefly named `CoerceSized` which had a slightly different +/// interpretation. /// /// Imagine we have a trait object `t` with type `&dyn Tr`, where `Tr` is some trait with a method -/// `m` defined as `fn m(&self);`. When calling `t.m()`, the receiver `t` is a wide pointer, but an -/// implementation of `m` will expect a narrow pointer as `&self` (a reference to the concrete -/// type). The compiler must generate an implicit conversion from the trait object/wide pointer to -/// the concrete reference/narrow pointer. Implementing `DispatchFromDyn` indicates that that -/// conversion is allowed and thus that the type implementing `DispatchFromDyn` is safe to use as -/// the self type in an dyn-compatible method. (in the above example, the compiler will require -/// `DispatchFromDyn` is implemented for `&'a U`). +/// `m` defined as `fn m(&self);`. +/// When calling `t.m()`, the receiver `t` is a wide pointer, but an implementation of `m` will +/// expect a narrow pointer as `&self`, specifically a reference to the concrete type. +/// The compiler must generate an implicit conversion from the trait object `&dyn Trait` or +/// wide pointer `&UnsizedType` to a concrete reference `&ConcreteType` or narrow pointer `&SizedType` +/// respectively. /// -/// `DispatchFromDyn` does not specify the conversion from wide pointer to narrow pointer; the -/// conversion is hard-wired into the compiler. For the conversion to work, the following -/// properties must hold (i.e., it is only safe to implement `DispatchFromDyn` for types which have -/// these properties, these are also checked by the compiler): +/// Implementing `DispatchFromDyn` indicates that such conversion is allowed and, thus, that the +/// type implementing `DispatchFromDyn` is safe to use as the type of `self`, also known as a method +/// receiver type, in an dyn-compatible method. +/// In the above example, the compiler will require `DispatchFromDyn` is implemented for `&'a T` +/// against `&'a dyn Tr`, given any `T` with `T: Unsize`. /// -/// * EITHER `Self` and `T` are either both references or both raw pointers; in either case, with -/// the same mutability. -/// * OR, all of the following hold +/// `DispatchFromDyn` does not specify the conversion from wide pointer to narrow pointer. +/// The conversion is *hard-wired* into the compiler. +/// Therefore, the compiler will check that the following properties must hold, +/// so that it is only safe to implement `DispatchFromDyn` for types with these properties. +/// +/// * *Either* `Self := &A` and `T := &B` are either both references or both raw pointers to +/// generic types `A` and `B`, so that `A: Unsize`[^2]. +/// In addition, the mutability of the references or raw pointers must match. +/// In other words, `&A`/`&B` and `&mut A`/`&mut B` are valid pairings. +/// * *Or*, all of the following hold: /// - `Self` and `T` must have the same type constructor, and only vary in a single type parameter -/// formal (the *coerced type*, e.g., `impl DispatchFromDyn> for Rc` is ok and the -/// single type parameter (instantiated with `T` or `U`) is the coerced type, -/// `impl DispatchFromDyn> for Rc` is not ok). -/// - The definition for `Self` must be a struct. +/// formal which is undergoing *coercion*. +/// For instance, `impl + PointeeSized, U + PointeeSized> DispatchFromDyn> for Rc` +/// is acceptable because the single type parameter, the `T` and `U` respectively, is the coerced type. +/// `impl + PointeeSized, U + PointeeSized> DispatchFromDyn> for Rc` is +/// unacceptable because `Arc` and `Rc` does not match on the type constructor. +/// One is `Arc<_>` and the other is `Rc<_>`. +/// - The definition for `Self` must be a `struct`. /// - The definition for `Self` must not be `#[repr(packed)]` or `#[repr(C)]`. -/// - Other than one-aligned, zero-sized fields, the definition for `Self` must have exactly one -/// field and that field's type must be the coerced type. Furthermore, `Self`'s field type must -/// implement `DispatchFromDyn` where `F` is the type of `T`'s field type. +/// - Excluding one-aligned zero-sized fields, the definition for `Self` must have exactly one +/// field and that field's type must be of the coerced type. +/// Furthermore, the type `FSelf` of this cocerced field type in `Self` must also implement +/// `DispatchFromDyn` where `FTarget` is the type of the corresponding coerced field in +/// `T`. +/// +/// Note that we do not support the case where multiple pointer narrowings or downcasting of trait +/// objects into concrete objects are involved in coercing one type to another through +/// the `DispatchFromDyn` trait. +/// For instance, we do not support dispatch from `&Arc` to `&Arc`. +/// `&Arc` is already behind an immutable reference while the coercion would require a +/// downcasting of the trait object `Arc` by chipping of the virtual table. +/// This will break the invariant of an immutable reference that the place behind shall remain +/// not mutated during the access. +/// Similarly, the dispatch from `Box>` to `Box>` also requires a similar memory +/// layout change. +/// Therefore, this category of coercion is not supported. /// /// An example implementation of the trait: /// @@ -114,6 +138,7 @@ impl, U: PointeeSized> CoerceUnsized<*const U> for * /// ``` /// /// [^1]: Formerly known as *object safety*. +/// [^2]: crate::marker::Unsize #[unstable(feature = "dispatch_from_dyn", issue = "none")] #[lang = "dispatch_from_dyn"] pub trait DispatchFromDyn { 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-blanket-impl.rs b/tests/ui/traits/dispatch-from-dyn-blanket-impl.rs index 4e0e7ca9793f8..bfce906ea3132 100644 --- a/tests/ui/traits/dispatch-from-dyn-blanket-impl.rs +++ b/tests/ui/traits/dispatch-from-dyn-blanket-impl.rs @@ -1,10 +1,11 @@ // Test that blanket impl of DispatchFromDyn is rejected. // regression test for issue -#![feature(dispatch_from_dyn)] +#![feature(dispatch_from_dyn, unsize)] -impl std::ops::DispatchFromDyn for T {} +use std::marker::Unsize; + +impl> std::ops::DispatchFromDyn for T {} //~^ ERROR type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct`) -//~| ERROR the trait `DispatchFromDyn` may only be implemented for a coercion between structures fn main() {} diff --git a/tests/ui/traits/dispatch-from-dyn-blanket-impl.stderr b/tests/ui/traits/dispatch-from-dyn-blanket-impl.stderr index 69f360817805a..ba51b8d3c5d43 100644 --- a/tests/ui/traits/dispatch-from-dyn-blanket-impl.stderr +++ b/tests/ui/traits/dispatch-from-dyn-blanket-impl.stderr @@ -1,19 +1,12 @@ error[E0210]: type parameter `T` must be used as the type parameter for some local type (e.g., `MyStruct`) - --> $DIR/dispatch-from-dyn-blanket-impl.rs:6:6 + --> $DIR/dispatch-from-dyn-blanket-impl.rs:8:6 | -LL | impl std::ops::DispatchFromDyn for T {} +LL | impl> std::ops::DispatchFromDyn for T {} | ^ type parameter `T` must be used as the type parameter for some local type | = note: implementing a foreign trait is only possible if at least one of the types for which it is implemented is local = note: only traits defined in the current crate can be implemented for a type parameter -error[E0377]: the trait `DispatchFromDyn` may only be implemented for a coercion between structures - --> $DIR/dispatch-from-dyn-blanket-impl.rs:6:1 - | -LL | impl std::ops::DispatchFromDyn for T {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: aborting due to 2 previous errors +error: aborting due to 1 previous error -Some errors have detailed explanations: E0210, E0377. -For more information about an error, try `rustc --explain E0210`. +For more information about this error, try `rustc --explain E0210`. 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..d864af4c1502d 100644 --- a/tests/ui/traits/dispatch-from-dyn-invalid-impls.stderr +++ b/tests/ui/traits/dispatch-from-dyn-invalid-impls.stderr @@ -54,7 +54,15 @@ 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 {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the trait `DispatchFromDyn` may only be implemented when dispatch goes through at most one reference to a generic + +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`.