From 1b0799157450a2ef6c50b4ffb9c0c40ee105fe8f Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 28 Jun 2020 17:36:41 +0100 Subject: [PATCH] Check associated type bounds for object safety violations --- .../src/traits/object_safety.rs | 101 +++++++++++------- .../ui/object-safety/object-safety-bounds.rs | 12 +++ .../object-safety/object-safety-bounds.stderr | 14 +++ 3 files changed, 88 insertions(+), 39 deletions(-) create mode 100644 src/test/ui/object-safety/object-safety-bounds.rs create mode 100644 src/test/ui/object-safety/object-safety-bounds.stderr diff --git a/compiler/rustc_trait_selection/src/traits/object_safety.rs b/compiler/rustc_trait_selection/src/traits/object_safety.rs index 8fc14cb2997..0e43f1655dd 100644 --- a/compiler/rustc_trait_selection/src/traits/object_safety.rs +++ b/compiler/rustc_trait_selection/src/traits/object_safety.rs @@ -160,6 +160,10 @@ fn object_safety_violations_for_trait( if !spans.is_empty() { violations.push(ObjectSafetyViolation::SupertraitSelf(spans)); } + let spans = bounds_reference_self(tcx, trait_def_id); + if !spans.is_empty() { + violations.push(ObjectSafetyViolation::SupertraitSelf(spans)); + } violations.extend( tcx.associated_items(trait_def_id) @@ -239,51 +243,70 @@ fn predicates_reference_self( } else { tcx.predicates_of(trait_def_id) }; - let self_ty = tcx.types.self_param; - let has_self_ty = |arg: &GenericArg<'_>| arg.walk().any(|arg| arg == self_ty.into()); predicates .predicates .iter() - .map(|(predicate, sp)| (predicate.subst_supertrait(tcx, &trait_ref), sp)) - .filter_map(|(predicate, &sp)| { - match predicate.skip_binders() { - ty::PredicateAtom::Trait(ref data, _) => { - // In the case of a trait predicate, we can skip the "self" type. - if data.trait_ref.substs[1..].iter().any(has_self_ty) { Some(sp) } else { None } - } - ty::PredicateAtom::Projection(ref data) => { - // And similarly for projections. This should be redundant with - // the previous check because any projection should have a - // matching `Trait` predicate with the same inputs, but we do - // the check to be safe. - // - // Note that we *do* allow projection *outputs* to contain - // `self` (i.e., `trait Foo: Bar { type Result; }`), - // we just require the user to specify *both* outputs - // in the object type (i.e., `dyn Foo`). - // - // This is ALT2 in issue #56288, see that for discussion of the - // possible alternatives. - if data.projection_ty.trait_ref(tcx).substs[1..].iter().any(has_self_ty) { - Some(sp) - } else { - None - } - } - ty::PredicateAtom::WellFormed(..) - | ty::PredicateAtom::ObjectSafe(..) - | ty::PredicateAtom::TypeOutlives(..) - | ty::PredicateAtom::RegionOutlives(..) - | ty::PredicateAtom::ClosureKind(..) - | ty::PredicateAtom::Subtype(..) - | ty::PredicateAtom::ConstEvaluatable(..) - | ty::PredicateAtom::ConstEquate(..) - | ty::PredicateAtom::TypeWellFormedFromEnv(..) => None, - } - }) + .map(|(predicate, sp)| (predicate.subst_supertrait(tcx, &trait_ref), *sp)) + .filter_map(|predicate| predicate_references_self(tcx, predicate)) .collect() } +fn bounds_reference_self(tcx: TyCtxt<'_>, trait_def_id: DefId) -> SmallVec<[Span; 1]> { + let trait_ref = ty::Binder::dummy(ty::TraitRef::identity(tcx, trait_def_id)); + tcx.associated_items(trait_def_id) + .in_definition_order() + .filter(|item| item.kind == ty::AssocKind::Type) + .flat_map(|item| tcx.explicit_item_bounds(item.def_id)) + .map(|(predicate, sp)| (predicate.subst_supertrait(tcx, &trait_ref), *sp)) + .filter_map(|predicate| predicate_references_self(tcx, predicate)) + .collect() +} + +fn predicate_references_self( + tcx: TyCtxt<'tcx>, + (predicate, sp): (ty::Predicate<'tcx>, Span), +) -> Option { + let self_ty = tcx.types.self_param; + let has_self_ty = |arg: &GenericArg<'_>| arg.walk().any(|arg| arg == self_ty.into()); + match predicate.skip_binders() { + ty::PredicateAtom::Trait(ref data, _) => { + // In the case of a trait predicate, we can skip the "self" type. + if data.trait_ref.substs[1..].iter().any(has_self_ty) { Some(sp) } else { None } + } + ty::PredicateAtom::Projection(ref data) => { + // And similarly for projections. This should be redundant with + // the previous check because any projection should have a + // matching `Trait` predicate with the same inputs, but we do + // the check to be safe. + // + // It's also won't be redundant if we allow type-generic associated + // types for trait objects. + // + // Note that we *do* allow projection *outputs* to contain + // `self` (i.e., `trait Foo: Bar { type Result; }`), + // we just require the user to specify *both* outputs + // in the object type (i.e., `dyn Foo`). + // + // This is ALT2 in issue #56288, see that for discussion of the + // possible alternatives. + if data.projection_ty.trait_ref(tcx).substs[1..].iter().any(has_self_ty) { + Some(sp) + } else { + None + } + } + ty::PredicateAtom::WellFormed(..) + | ty::PredicateAtom::ObjectSafe(..) + | ty::PredicateAtom::TypeOutlives(..) + | ty::PredicateAtom::RegionOutlives(..) + | ty::PredicateAtom::ClosureKind(..) + | ty::PredicateAtom::Subtype(..) + | ty::PredicateAtom::ConstEvaluatable(..) + | ty::PredicateAtom::ConstEquate(..) + | ty::PredicateAtom::TypeWellFormedFromEnv(..) => None, + } +} + fn trait_has_sized_self(tcx: TyCtxt<'_>, trait_def_id: DefId) -> bool { generics_require_sized_self(tcx, trait_def_id) } diff --git a/src/test/ui/object-safety/object-safety-bounds.rs b/src/test/ui/object-safety/object-safety-bounds.rs new file mode 100644 index 00000000000..44bd369324a --- /dev/null +++ b/src/test/ui/object-safety/object-safety-bounds.rs @@ -0,0 +1,12 @@ +// Traits with bounds mentioning `Self` are not object safe + +trait X { + type U: PartialEq; +} + +fn f() -> Box> { + //~^ ERROR the trait `X` cannot be made into an object + loop {} +} + +fn main() {} diff --git a/src/test/ui/object-safety/object-safety-bounds.stderr b/src/test/ui/object-safety/object-safety-bounds.stderr new file mode 100644 index 00000000000..af454830863 --- /dev/null +++ b/src/test/ui/object-safety/object-safety-bounds.stderr @@ -0,0 +1,14 @@ +error[E0038]: the trait `X` cannot be made into an object + --> $DIR/object-safety-bounds.rs:7:11 + | +LL | trait X { + | - this trait cannot be made into an object... +LL | type U: PartialEq; + | --------------- ...because it uses `Self` as a type parameter in this +... +LL | fn f() -> Box> { + | ^^^^^^^^^^^^^^^^^^^ the trait `X` cannot be made into an object + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0038`.