Skip to content

DispatchFromDyn: require additional checks #149068

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+153 −112
Open

DispatchFromDyn: require additional checks #149068

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
244 changes: 134 additions & 110 deletions compiler/rustc_hir_analysis/src/coherence/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -242,138 +240,164 @@ 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(),
}));
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 r_a == r_b && mutbl_a == mutbl_b =>
{
source = ty_a;
target = ty_b;
}

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 a_mutbl == b_mutbl => {
source = ty_a;
target = ty_b;
}
(&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,
}));
}

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::<Result<_, _>>()?;

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::<Vec<_>>();
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::<Vec<_>>().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::<Vec<_>>().into(),
}));
return Ok(());
}
_ => {
return Err(tcx
.dcx()
.emit_err(errors::CoerceUnsizedNonStruct { span, trait_name }));
}
}
_ => Err(tcx.dcx().emit_err(errors::CoerceUnsizedNonStruct { span, trait_name })),
}
}

Expand Down
2 changes: 1 addition & 1 deletion library/core/src/pat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ impl<T: PointeeSized, U: PointeeSized> CoerceUnsized<pattern_type!(*const U is !
{
}

impl<T: DispatchFromDyn<U>, U> DispatchFromDyn<pattern_type!(U is !null)> for pattern_type!(T is !null) {}
impl<T: DispatchFromDyn<U> + Unsize<U>, U> DispatchFromDyn<pattern_type!(U is !null)> for pattern_type!(T is !null) {}

impl<T: PointeeSized> Unpin for pattern_type!(*const T is !null) {}

Expand Down
5 changes: 5 additions & 0 deletions tests/ui/traits/dispatch-from-dyn-invalid-impls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,9 @@ where
{
}

pub struct Ptr<T: ?Sized>(Box<T>, Box<T>);

impl<'a, T: ?Sized, U: ?Sized> DispatchFromDyn<&'a Ptr<U>> for &'a Ptr<T> {}
//~^ ERROR [E0375]

fn main() {}
14 changes: 13 additions & 1 deletion tests/ui/traits/dispatch-from-dyn-invalid-impls.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,19 @@ LL | | T: Unsize<U>
|
= note: extra field `1` of type `OverAlignedZst` is not allowed

error: aborting due to 5 previous errors
error[E0375]: implementing `DispatchFromDyn` does not allow multiple fields to be coerced
--> $DIR/dispatch-from-dyn-invalid-impls.rs:73:1
|
LL | impl<'a, T: ?Sized, U: ?Sized> DispatchFromDyn<&'a Ptr<U>> for &'a Ptr<T> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: the trait `DispatchFromDyn` may only be implemented when a single field is being coerced
--> $DIR/dispatch-from-dyn-invalid-impls.rs:71:27
|
LL | pub struct Ptr<T: ?Sized>(Box<T>, Box<T>);
| ^^^^^^ ^^^^^^

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`.
Loading