Skip to content

Commit 88de0f9

Browse files
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. Fix #148727 Signed-off-by: Xiangfei Ding <[email protected]>
1 parent 3d461af commit 88de0f9

File tree

4 files changed

+153
-112
lines changed

4 files changed

+153
-112
lines changed

compiler/rustc_hir_analysis/src/coherence/builtin.rs

Lines changed: 134 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -203,17 +203,15 @@ fn visit_implementation_of_dispatch_from_dyn(checker: &Checker<'_>) -> Result<()
203203
let tcx = checker.tcx;
204204
let impl_did = checker.impl_def_id;
205205
let trait_ref = checker.impl_header.trait_ref.instantiate_identity();
206+
assert!(tcx.is_lang_item(trait_ref.def_id, LangItem::DispatchFromDyn));
207+
let dispatch_from_dyn_trait_did = trait_ref.def_id;
206208
debug!("visit_implementation_of_dispatch_from_dyn: impl_did={:?}", impl_did);
207209

208210
let span = tcx.def_span(impl_did);
209211
let trait_name = "DispatchFromDyn";
210212

211-
let source = trait_ref.self_ty();
212-
let target = {
213-
assert!(tcx.is_lang_item(trait_ref.def_id, LangItem::DispatchFromDyn));
214-
215-
trait_ref.args.type_at(1)
216-
};
213+
let mut source = trait_ref.self_ty();
214+
let mut target = trait_ref.args.type_at(1);
217215

218216
// Check `CoercePointee` impl is WF -- if not, then there's no reason to report
219217
// redundant errors for `DispatchFromDyn`. This is best effort, though.
@@ -242,138 +240,164 @@ fn visit_implementation_of_dispatch_from_dyn(checker: &Checker<'_>) -> Result<()
242240
// trait, they *do* satisfy the repr(transparent) rules, and then we assume that everything else
243241
// in the compiler (in particular, all the call ABI logic) will treat them as repr(transparent)
244242
// even if they do not carry that attribute.
245-
match (source.kind(), target.kind()) {
246-
(&ty::Pat(_, pat_a), &ty::Pat(_, pat_b)) => {
247-
if pat_a != pat_b {
248-
return Err(tcx.dcx().emit_err(errors::CoerceSamePatKind {
249-
span,
250-
trait_name,
251-
pat_a: pat_a.to_string(),
252-
pat_b: pat_b.to_string(),
253-
}));
243+
loop {
244+
match (source.kind(), target.kind()) {
245+
(&ty::Pat(ty_a, pat_a), &ty::Pat(ty_b, pat_b)) => {
246+
if pat_a != pat_b {
247+
return Err(tcx.dcx().emit_err(errors::CoerceSamePatKind {
248+
span,
249+
trait_name,
250+
pat_a: pat_a.to_string(),
251+
pat_b: pat_b.to_string(),
252+
}));
253+
}
254+
source = ty_a;
255+
target = ty_b;
254256
}
255-
Ok(())
256-
}
257257

258-
(&ty::Ref(r_a, _, mutbl_a), ty::Ref(r_b, _, mutbl_b))
259-
if r_a == *r_b && mutbl_a == *mutbl_b =>
260-
{
261-
Ok(())
262-
}
263-
(&ty::RawPtr(_, a_mutbl), &ty::RawPtr(_, b_mutbl)) if a_mutbl == b_mutbl => Ok(()),
264-
(&ty::Adt(def_a, args_a), &ty::Adt(def_b, args_b))
265-
if def_a.is_struct() && def_b.is_struct() =>
266-
{
267-
if def_a != def_b {
268-
let source_path = tcx.def_path_str(def_a.did());
269-
let target_path = tcx.def_path_str(def_b.did());
270-
return Err(tcx.dcx().emit_err(errors::CoerceSameStruct {
271-
span,
272-
trait_name,
273-
note: true,
274-
source_path,
275-
target_path,
276-
}));
258+
(&ty::Ref(r_a, ty_a, mutbl_a), &ty::Ref(r_b, ty_b, mutbl_b))
259+
if r_a == r_b && mutbl_a == mutbl_b =>
260+
{
261+
source = ty_a;
262+
target = ty_b;
277263
}
278-
279-
if def_a.repr().c() || def_a.repr().packed() {
280-
return Err(tcx.dcx().emit_err(errors::DispatchFromDynRepr { span }));
264+
(&ty::RawPtr(ty_a, a_mutbl), &ty::RawPtr(ty_b, b_mutbl)) if a_mutbl == b_mutbl => {
265+
source = ty_a;
266+
target = ty_b;
281267
}
268+
(&ty::Adt(def_a, args_a), &ty::Adt(def_b, args_b))
269+
if def_a.is_struct() && def_b.is_struct() =>
270+
{
271+
if def_a != def_b {
272+
let source_path = tcx.def_path_str(def_a.did());
273+
let target_path = tcx.def_path_str(def_b.did());
274+
return Err(tcx.dcx().emit_err(errors::CoerceSameStruct {
275+
span,
276+
trait_name,
277+
note: true,
278+
source_path,
279+
target_path,
280+
}));
281+
}
282282

283-
let fields = &def_a.non_enum_variant().fields;
284-
285-
let mut res = Ok(());
286-
let coerced_fields = fields
287-
.iter_enumerated()
288-
.filter_map(|(i, field)| {
289-
// Ignore PhantomData fields
290-
let unnormalized_ty = tcx.type_of(field.did).instantiate_identity();
291-
if tcx
292-
.try_normalize_erasing_regions(
293-
ty::TypingEnv::non_body_analysis(tcx, def_a.did()),
294-
unnormalized_ty,
295-
)
296-
.unwrap_or(unnormalized_ty)
297-
.is_phantom_data()
298-
{
299-
return None;
300-
}
283+
if def_a.repr().c() || def_a.repr().packed() {
284+
return Err(tcx.dcx().emit_err(errors::DispatchFromDynRepr { span }));
285+
}
301286

302-
let ty_a = field.ty(tcx, args_a);
303-
let ty_b = field.ty(tcx, args_b);
304-
305-
// FIXME: We could do normalization here, but is it really worth it?
306-
if ty_a == ty_b {
307-
// Allow 1-ZSTs that don't mention type params.
308-
//
309-
// Allowing type params here would allow us to possibly transmute
310-
// between ZSTs, which may be used to create library unsoundness.
311-
if let Ok(layout) =
312-
tcx.layout_of(infcx.typing_env(param_env).as_query_input(ty_a))
313-
&& layout.is_1zst()
314-
&& !ty_a.has_non_region_param()
287+
let fields = &def_a.non_enum_variant().fields;
288+
289+
let coerced_fields: Vec<_> = fields
290+
.iter_enumerated()
291+
.filter_map(|(i, field)| {
292+
// Ignore PhantomData fields
293+
let unnormalized_ty = tcx.type_of(field.did).instantiate_identity();
294+
if tcx
295+
.try_normalize_erasing_regions(
296+
ty::TypingEnv::non_body_analysis(tcx, def_a.did()),
297+
unnormalized_ty,
298+
)
299+
.unwrap_or(unnormalized_ty)
300+
.is_phantom_data()
315301
{
316-
// ignore 1-ZST fields
317302
return None;
318303
}
319304

320-
res = Err(tcx.dcx().emit_err(errors::DispatchFromDynZST {
321-
span,
322-
name: field.ident(tcx),
323-
ty: ty_a,
324-
}));
305+
let ty_a = field.ty(tcx, args_a);
306+
let ty_b = field.ty(tcx, args_b);
307+
308+
// FIXME: We could do normalization here, but is it really worth it?
309+
if ty_a == ty_b {
310+
// Allow 1-ZSTs that don't mention type params.
311+
//
312+
// Allowing type params here would allow us to possibly transmute
313+
// between ZSTs, which may be used to create library unsoundness.
314+
if let Ok(layout) =
315+
tcx.layout_of(infcx.typing_env(param_env).as_query_input(ty_a))
316+
&& layout.is_1zst()
317+
&& !ty_a.has_non_region_param()
318+
{
319+
// ignore 1-ZST fields
320+
return None;
321+
}
325322

326-
None
327-
} else {
328-
Some((i, ty_a, ty_b, tcx.def_span(field.did)))
323+
Some(Err(tcx.dcx().emit_err(errors::DispatchFromDynZST {
324+
span,
325+
name: field.ident(tcx),
326+
ty: ty_a,
327+
})))
328+
} else {
329+
Some(Ok((i, ty_a, ty_b, tcx.def_span(field.did))))
330+
}
331+
})
332+
.collect::<Result<_, _>>()?;
333+
334+
if coerced_fields.is_empty() {
335+
return Err(tcx.dcx().emit_err(errors::CoerceNoField {
336+
span,
337+
trait_name,
338+
note: true,
339+
}));
340+
}
341+
if let &[(_, ty_a, ty_b, field_span)] = &coerced_fields[..] {
342+
let ocx = ObligationCtxt::new_with_diagnostics(&infcx);
343+
ocx.register_obligation(Obligation::new(
344+
tcx,
345+
cause.clone(),
346+
param_env,
347+
ty::TraitRef::new(tcx, dispatch_from_dyn_trait_did, [ty_a, ty_b]),
348+
));
349+
let errors = ocx.evaluate_obligations_error_on_ambiguity();
350+
if !errors.is_empty() {
351+
if is_from_coerce_pointee_derive(tcx, span) {
352+
return Err(tcx.dcx().emit_err(errors::CoerceFieldValidity {
353+
span,
354+
trait_name,
355+
ty: trait_ref.self_ty(),
356+
field_span,
357+
field_ty: ty_a,
358+
}));
359+
} else {
360+
return Err(infcx.err_ctxt().report_fulfillment_errors(errors));
361+
}
329362
}
330-
})
331-
.collect::<Vec<_>>();
332-
res?;
333363

334-
if coerced_fields.is_empty() {
335-
return Err(tcx.dcx().emit_err(errors::CoerceNoField {
364+
// Finally, resolve all regions.
365+
ocx.resolve_regions_and_report_errors(impl_did, param_env, [])?;
366+
367+
return Ok(());
368+
}
369+
return Err(tcx.dcx().emit_err(errors::CoerceMulti {
336370
span,
337371
trait_name,
338-
note: true,
372+
number: coerced_fields.len(),
373+
fields: coerced_fields.iter().map(|(_, _, _, s)| *s).collect::<Vec<_>>().into(),
339374
}));
340-
} else if let &[(_, ty_a, ty_b, field_span)] = &coerced_fields[..] {
375+
}
376+
(ty::Param(_), ty::Param(_)) => {
341377
let ocx = ObligationCtxt::new_with_diagnostics(&infcx);
342378
ocx.register_obligation(Obligation::new(
343379
tcx,
344380
cause.clone(),
345381
param_env,
346-
ty::TraitRef::new(tcx, trait_ref.def_id, [ty_a, ty_b]),
382+
ty::TraitRef::new(
383+
tcx,
384+
tcx.require_lang_item(LangItem::Unsize, span),
385+
[source, target],
386+
),
347387
));
348388
let errors = ocx.evaluate_obligations_error_on_ambiguity();
349389
if !errors.is_empty() {
350-
if is_from_coerce_pointee_derive(tcx, span) {
351-
return Err(tcx.dcx().emit_err(errors::CoerceFieldValidity {
352-
span,
353-
trait_name,
354-
ty: trait_ref.self_ty(),
355-
field_span,
356-
field_ty: ty_a,
357-
}));
358-
} else {
359-
return Err(infcx.err_ctxt().report_fulfillment_errors(errors));
360-
}
390+
return Err(infcx.err_ctxt().report_fulfillment_errors(errors));
361391
}
362-
363-
// Finally, resolve all regions.
364392
ocx.resolve_regions_and_report_errors(impl_did, param_env, [])?;
365-
366-
Ok(())
367-
} else {
368-
return Err(tcx.dcx().emit_err(errors::CoerceMulti {
369-
span,
370-
trait_name,
371-
number: coerced_fields.len(),
372-
fields: coerced_fields.iter().map(|(_, _, _, s)| *s).collect::<Vec<_>>().into(),
373-
}));
393+
return Ok(());
394+
}
395+
_ => {
396+
return Err(tcx
397+
.dcx()
398+
.emit_err(errors::CoerceUnsizedNonStruct { span, trait_name }));
374399
}
375400
}
376-
_ => Err(tcx.dcx().emit_err(errors::CoerceUnsizedNonStruct { span, trait_name })),
377401
}
378402
}
379403

library/core/src/pat.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ impl<T: PointeeSized, U: PointeeSized> CoerceUnsized<pattern_type!(*const U is !
8282
{
8383
}
8484

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

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

tests/ui/traits/dispatch-from-dyn-invalid-impls.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,4 +68,9 @@ where
6868
{
6969
}
7070

71+
pub struct Ptr<T: ?Sized>(Box<T>, Box<T>);
72+
73+
impl<'a, T: ?Sized, U: ?Sized> DispatchFromDyn<&'a Ptr<U>> for &'a Ptr<T> {}
74+
//~^ ERROR [E0375]
75+
7176
fn main() {}

tests/ui/traits/dispatch-from-dyn-invalid-impls.stderr

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,19 @@ LL | | T: Unsize<U>
5454
|
5555
= note: extra field `1` of type `OverAlignedZst` is not allowed
5656

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

5971
Some errors have detailed explanations: E0374, E0375, E0378.
6072
For more information about an error, try `rustc --explain E0374`.

0 commit comments

Comments
 (0)