From f52b2d88903036beb0533b04011064575b3abd36 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 5 Jul 2020 12:16:25 +0100 Subject: [PATCH] Avoid cycle in nested obligations for object candidate Bounds of the form `type Future: Future` exist in some ecosystem crates. To validate these bounds for trait objects we need to normalize `Self::Result` in a way that doesn't cause a cycle. --- compiler/rustc_middle/src/ty/sty.rs | 1 + .../src/traits/select/confirmation.rs | 227 ++++++++++++++---- .../ui/traits/check-trait-object-bounds-4.rs | 17 ++ .../traits/check-trait-object-bounds-4.stderr | 12 + .../ui/traits/check-trait-object-bounds-5.rs | 27 +++ .../traits/check-trait-object-bounds-5.stderr | 12 + .../ui/traits/check-trait-object-bounds-6.rs | 24 ++ .../traits/check-trait-object-bounds-6.stderr | 12 + .../ui/traits/trait-object-bounds-cycle-1.rs | 24 ++ .../ui/traits/trait-object-bounds-cycle-2.rs | 28 +++ .../ui/traits/trait-object-bounds-cycle-3.rs | 25 ++ .../ui/traits/trait-object-bounds-cycle-4.rs | 25 ++ 12 files changed, 382 insertions(+), 52 deletions(-) create mode 100644 src/test/ui/traits/check-trait-object-bounds-4.rs create mode 100644 src/test/ui/traits/check-trait-object-bounds-4.stderr create mode 100644 src/test/ui/traits/check-trait-object-bounds-5.rs create mode 100644 src/test/ui/traits/check-trait-object-bounds-5.stderr create mode 100644 src/test/ui/traits/check-trait-object-bounds-6.rs create mode 100644 src/test/ui/traits/check-trait-object-bounds-6.stderr create mode 100644 src/test/ui/traits/trait-object-bounds-cycle-1.rs create mode 100644 src/test/ui/traits/trait-object-bounds-cycle-2.rs create mode 100644 src/test/ui/traits/trait-object-bounds-cycle-3.rs create mode 100644 src/test/ui/traits/trait-object-bounds-cycle-4.rs diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index 5cba451ea6e..09895605dca 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -1513,6 +1513,7 @@ impl<'tcx> ExistentialProjection<'tcx> { /// then this function would return a `exists T. T: Iterator` existential trait /// reference. pub fn trait_ref(&self, tcx: TyCtxt<'_>) -> ty::ExistentialTraitRef<'tcx> { + // FIXME(generic_associated_types): truncate substs to have the right length. let def_id = tcx.associated_item(self.item_def_id).container.id(); ty::ExistentialTraitRef { def_id, substs: self.substs } } diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 319217e8fdc..59399133dff 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -9,13 +9,15 @@ use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_hir::lang_items::LangItem; use rustc_index::bit_set::GrowableBitSet; +use rustc_infer::infer::LateBoundRegionConversionTime::HigherRankedType; use rustc_infer::infer::{self, InferOk}; +use rustc_middle::ty::fold::TypeFolder; use rustc_middle::ty::subst::{GenericArg, GenericArgKind, Subst, SubstsRef}; -use rustc_middle::ty::{self, Ty}; +use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable}; use rustc_middle::ty::{ToPolyTraitRef, ToPredicate, WithConstness}; use rustc_span::def_id::DefId; -use crate::traits::project::{self, normalize_with_depth}; +use crate::traits::project::{normalize_with_depth, normalize_with_depth_to}; use crate::traits::select::TraitObligationExt; use crate::traits::util; use crate::traits::util::{closure_trait_ref_and_return_type, predicate_for_trait_def}; @@ -340,16 +342,27 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { &mut self, obligation: &TraitObligation<'tcx>, ) -> ImplSourceObjectData<'tcx, PredicateObligation<'tcx>> { + let tcx = self.tcx(); debug!("confirm_object_candidate({:?})", obligation); - let self_ty = self.infcx.shallow_resolve(obligation.self_ty()); - let self_ty = self.infcx.replace_bound_vars_with_placeholders(&self_ty); + let trait_predicate = + self.infcx.replace_bound_vars_with_placeholders(&obligation.predicate); + let self_ty = self.infcx.shallow_resolve(trait_predicate.self_ty()); + let obligation_trait_ref = ty::Binder::dummy(trait_predicate.trait_ref); let data = match self_ty.kind() { - ty::Dynamic(data, ..) => data, + ty::Dynamic(data, ..) => { + self.infcx + .replace_bound_vars_with_fresh_vars( + obligation.cause.span, + HigherRankedType, + data, + ) + .0 + } _ => span_bug!(obligation.cause.span, "object candidate with non-object"), }; - let poly_trait_ref = data + let object_trait_ref = data .principal() .unwrap_or_else(|| { span_bug!(obligation.cause.span, "object candidate with no principal") @@ -361,24 +374,29 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { let vtable_base; { - let tcx = self.tcx(); - // We want to find the first supertrait in the list of // supertraits that we can unify with, and do that // unification. We know that there is exactly one in the list // where we can unify, because otherwise select would have // reported an ambiguity. (When we do find a match, also // record it for later.) - let nonmatching = util::supertraits(tcx, poly_trait_ref).take_while(|&t| { - match self.infcx.commit_if_ok(|_| self.match_poly_trait_ref(obligation, t)) { - Ok(obligations) => { - upcast_trait_ref = Some(t); - nested.extend(obligations); - false + let nonmatching = util::supertraits(tcx, ty::Binder::dummy(object_trait_ref)) + .take_while(|&t| { + match self.infcx.commit_if_ok(|_| { + self.infcx + .at(&obligation.cause, obligation.param_env) + .sup(obligation_trait_ref, t) + .map(|InferOk { obligations, .. }| obligations) + .map_err(|_| ()) + }) { + Ok(obligations) => { + upcast_trait_ref = Some(t); + nested.extend(obligations); + false + } + Err(_) => true, } - Err(_) => true, - } - }); + }); // Additionally, for each of the non-matching predicates that // we pass over, we sum up the set of number of vtable @@ -387,47 +405,105 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { vtable_base = nonmatching.map(|t| super::util::count_own_vtable_entries(tcx, t)).sum(); } - for bound in data.skip_binder() { - match bound { - ty::ExistentialPredicate::Projection(projection) => { - // This maybe belongs in wf, but that can't (doesn't) handle - // higher-ranked things. - // Prevent, e.g., `dyn Iterator`. - // FIXME(generic_associated_types): We need some way to - // ensure that for `dyn for<'a> X = &'a ()>` the - // bound holds for all `'a`. - let (infer_projection, _) = self.infcx.replace_bound_vars_with_fresh_vars( + // Check supertraits hold + nested.extend(util::supertraits(tcx, obligation_trait_ref).skip(1).map(|super_trait| { + Obligation::new( + obligation.cause.clone(), + obligation.param_env, + super_trait.without_const().to_predicate(tcx), + ) + })); + + let upcast_trait_ref = upcast_trait_ref.unwrap(); + + let assoc_types: Vec<_> = tcx + .associated_items(upcast_trait_ref.def_id()) + .in_definition_order() + .filter_map( + |item| if item.kind == ty::AssocKind::Type { Some(item.def_id) } else { None }, + ) + .collect(); + + if !assoc_types.is_empty() { + let predicates: Vec<_> = + data.iter() + .filter_map(|pred| match pred { + ty::ExistentialPredicate::Projection(proj) => { + if assoc_types.contains(&proj.item_def_id) { + match self.infcx.commit_if_ok(|_| { + self.infcx + .at(&obligation.cause, obligation.param_env) + .sup( + ty::Binder::dummy( + proj.trait_ref(tcx).with_self_ty(tcx, self_ty), + ), + upcast_trait_ref, + ) + .map(|InferOk { obligations, .. }| obligations) + .map_err(|_| ()) + }) { + Ok(obligations) => { + nested.extend(obligations); + Some(proj) + } + Err(_) => None, + } + } else { + None + } + } + ty::ExistentialPredicate::AutoTrait(_) + | ty::ExistentialPredicate::Trait(_) => None, + }) + .collect(); + + let upcast_trait_ref = upcast_trait_ref + .no_bound_vars() + .expect("sup shouldn't return binder with bound vars"); + let mut normalizer = ObjectAssociatedTypeNormalizer { + infcx: self.infcx, + object_ty: self_ty, + object_bounds: &predicates, + param_env: obligation.param_env, + cause: &obligation.cause, + nested: &mut nested, + }; + for assoc_type in assoc_types { + if !tcx.generics_of(assoc_type).params.is_empty() { + // FIXME(generic_associated_types) generate placeholders to + // extend the trait substs. + tcx.sess.span_fatal( obligation.cause.span, - infer::HigherRankedType, - &ty::Binder::bind(projection), + "generic associated types in trait objects are not supported yet", ); - let substs: Vec<_> = - iter::once(self_ty.into()).chain(infer_projection.substs).collect(); - let bounds = - self.tcx().item_bounds(projection.item_def_id).iter().map(|bound| { - // In the example above, `bound` is `::Item: Sized` - // `subst_bound` is `str: Sized`. - let subst_bound = util::subst_assoc_item_bound( - self.tcx(), - bound, - infer_projection.ty, - &substs, - ); - Obligation::new( - obligation.cause.clone(), - obligation.param_env.clone(), - subst_bound, - ) - }); - debug!("confirm_object_candidate: adding bounds: {:?}", bounds); - nested.extend(bounds); } - ty::ExistentialPredicate::Trait(_) | ty::ExistentialPredicate::AutoTrait(_) => {} + // This maybe belongs in wf, but that can't (doesn't) handle + // higher-ranked things. + // Prevent, e.g., `dyn Iterator`. + for bound in self.tcx().item_bounds(assoc_type) { + let subst_bound = bound.subst(tcx, upcast_trait_ref.substs); + // Normalize projections the trait object manually to + // avoid evaluation overflow. + let object_normalized = subst_bound.fold_with(&mut normalizer); + let normalized_bound = normalize_with_depth_to( + self, + obligation.param_env, + obligation.cause.clone(), + obligation.recursion_depth + 1, + &object_normalized, + normalizer.nested, + ); + normalizer.nested.push(Obligation::new( + obligation.cause.clone(), + obligation.param_env.clone(), + normalized_bound, + )); + } } } debug!("confirm_object_candidate: nested: {:?}", nested); - ImplSourceObjectData { upcast_trait_ref: upcast_trait_ref.unwrap(), vtable_base, nested } + ImplSourceObjectData { upcast_trait_ref, vtable_base, nested } } fn confirm_fn_pointer_candidate( @@ -450,7 +526,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { .map_bound(|(trait_ref, _)| trait_ref); let Normalized { value: trait_ref, mut obligations } = ensure_sufficient_stack(|| { - project::normalize_with_depth( + normalize_with_depth( self, obligation.param_env, obligation.cause.clone(), @@ -867,3 +943,50 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { Ok(ImplSourceBuiltinData { nested }) } } + +struct ObjectAssociatedTypeNormalizer<'a, 'tcx> { + infcx: &'a infer::InferCtxt<'a, 'tcx>, + object_ty: Ty<'tcx>, + object_bounds: &'a [ty::ExistentialProjection<'tcx>], + param_env: ty::ParamEnv<'tcx>, + cause: &'a ObligationCause<'tcx>, + nested: &'a mut Vec>, +} + +impl<'tcx> TypeFolder<'tcx> for ObjectAssociatedTypeNormalizer<'_, 'tcx> { + fn tcx<'a>(&'a self) -> TyCtxt<'tcx> { + self.infcx.tcx + } + + fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> { + if !t.has_projections() { + return t; + } + if let ty::Projection(proj) = t.kind { + if let ty::Dynamic(..) = proj.self_ty().kind { + for bound in self.object_bounds { + if proj.item_def_id == bound.item_def_id { + // FIXME(generic_associated_types): This isn't relating + // the substs for the associated type. + match self.infcx.commit_if_ok(|_| { + self.infcx.at(self.cause, self.param_env).sub( + bound + .with_self_ty(self.infcx.tcx, self.object_ty) + .projection_ty + .trait_ref(self.infcx.tcx), + proj.trait_ref(self.infcx.tcx), + ) + }) { + Ok(InferOk { value: (), obligations }) => { + self.nested.extend(obligations); + return bound.ty; + } + Err(_) => {} + } + } + } + } + } + t.super_fold_with(self) + } +} diff --git a/src/test/ui/traits/check-trait-object-bounds-4.rs b/src/test/ui/traits/check-trait-object-bounds-4.rs new file mode 100644 index 00000000000..323508db2f2 --- /dev/null +++ b/src/test/ui/traits/check-trait-object-bounds-4.rs @@ -0,0 +1,17 @@ +// Check that we validate associated type bounds on super traits for trait +// objects + +trait Super { + type Y: Clone; +} + +trait X: Super {} + +fn f() { + None::.clone(); +} + +fn main() { + f::>(); + //~^ ERROR the trait bound `str: std::clone::Clone` is not satisfied +} diff --git a/src/test/ui/traits/check-trait-object-bounds-4.stderr b/src/test/ui/traits/check-trait-object-bounds-4.stderr new file mode 100644 index 00000000000..75d6862579d --- /dev/null +++ b/src/test/ui/traits/check-trait-object-bounds-4.stderr @@ -0,0 +1,12 @@ +error[E0277]: the trait bound `str: std::clone::Clone` is not satisfied + --> $DIR/check-trait-object-bounds-4.rs:15:5 + | +LL | fn f() { + | - required by this bound in `f` +... +LL | f::>(); + | ^^^^^^^^^^^^^^^^^^^ the trait `std::clone::Clone` is not implemented for `str` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/traits/check-trait-object-bounds-5.rs b/src/test/ui/traits/check-trait-object-bounds-5.rs new file mode 100644 index 00000000000..7d733ad26b7 --- /dev/null +++ b/src/test/ui/traits/check-trait-object-bounds-5.rs @@ -0,0 +1,27 @@ +// Check that we validate associated type bounds on super traits for trait +// objects + +trait Is { + type T; +} + +impl Is for U { + type T = U; +} + +trait Super { + type V; +} + +trait Obj: Super { + type U: Is; +} + +fn is_obj(_: &T) {} + +fn f(x: &dyn Obj) { + is_obj(x) + //~^ type mismatch resolving `::T == i64` +} + +fn main() {} diff --git a/src/test/ui/traits/check-trait-object-bounds-5.stderr b/src/test/ui/traits/check-trait-object-bounds-5.stderr new file mode 100644 index 00000000000..bd2b789cd99 --- /dev/null +++ b/src/test/ui/traits/check-trait-object-bounds-5.stderr @@ -0,0 +1,12 @@ +error[E0271]: type mismatch resolving `::T == i64` + --> $DIR/check-trait-object-bounds-5.rs:23:5 + | +LL | fn is_obj(_: &T) {} + | --- required by this bound in `is_obj` +... +LL | is_obj(x) + | ^^^^^^ expected `i64`, found `i32` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0271`. diff --git a/src/test/ui/traits/check-trait-object-bounds-6.rs b/src/test/ui/traits/check-trait-object-bounds-6.rs new file mode 100644 index 00000000000..cb196d67f67 --- /dev/null +++ b/src/test/ui/traits/check-trait-object-bounds-6.rs @@ -0,0 +1,24 @@ +// Check that we validate associated type bounds on super traits for trait +// objects + +trait Is { + type T; +} + +impl Is for U { + type T = U; +} + +trait Obj { + type U: Is; + type V; +} + +fn is_obj(_: &T) {} + +fn f(x: &dyn Obj) { + is_obj(x) + //~^ ERROR type mismatch resolving `::T == i64` +} + +fn main() {} diff --git a/src/test/ui/traits/check-trait-object-bounds-6.stderr b/src/test/ui/traits/check-trait-object-bounds-6.stderr new file mode 100644 index 00000000000..ea1fdaf46f6 --- /dev/null +++ b/src/test/ui/traits/check-trait-object-bounds-6.stderr @@ -0,0 +1,12 @@ +error[E0271]: type mismatch resolving `::T == i64` + --> $DIR/check-trait-object-bounds-6.rs:20:5 + | +LL | fn is_obj(_: &T) {} + | --- required by this bound in `is_obj` +... +LL | is_obj(x) + | ^^^^^^ expected `i64`, found `i32` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0271`. diff --git a/src/test/ui/traits/trait-object-bounds-cycle-1.rs b/src/test/ui/traits/trait-object-bounds-cycle-1.rs new file mode 100644 index 00000000000..3146764927c --- /dev/null +++ b/src/test/ui/traits/trait-object-bounds-cycle-1.rs @@ -0,0 +1,24 @@ +// Check that we don't have a cycle when we try to normalize `Self::U` in the +// bound below. + +// check-pass + +trait Is { + type T; +} + +impl Is for U { + type T = U; +} + +trait Obj { + type U: Is; +} + +fn is_obj(_: &T) {} + +fn f(x: &dyn Obj) { + is_obj(x) +} + +fn main() {} diff --git a/src/test/ui/traits/trait-object-bounds-cycle-2.rs b/src/test/ui/traits/trait-object-bounds-cycle-2.rs new file mode 100644 index 00000000000..4c1df38058d --- /dev/null +++ b/src/test/ui/traits/trait-object-bounds-cycle-2.rs @@ -0,0 +1,28 @@ +// Check that we don't have a cycle when we try to normalize `Self::V` in the +// bound below. + +// check-pass + +trait Is { + type T; +} + +impl Is for U { + type T = U; +} + +trait Super { + type V; +} + +trait Obj: Super { + type U: Is; +} + +fn is_obj(_: &T) {} + +fn f(x: &dyn Obj) { + is_obj(x) +} + +fn main() {} diff --git a/src/test/ui/traits/trait-object-bounds-cycle-3.rs b/src/test/ui/traits/trait-object-bounds-cycle-3.rs new file mode 100644 index 00000000000..55726a5ae45 --- /dev/null +++ b/src/test/ui/traits/trait-object-bounds-cycle-3.rs @@ -0,0 +1,25 @@ +// Check that we don't have a cycle when we try to normalize `Self::V` in the +// bound below. + +// check-pass + +trait Is { + type T; +} + +impl Is for U { + type T = U; +} + +trait Obj { + type U: Is; + type V; +} + +fn is_obj(_: &T) {} + +fn f(x: &dyn Obj) { + is_obj(x) +} + +fn main() {} diff --git a/src/test/ui/traits/trait-object-bounds-cycle-4.rs b/src/test/ui/traits/trait-object-bounds-cycle-4.rs new file mode 100644 index 00000000000..f83cb75c7f2 --- /dev/null +++ b/src/test/ui/traits/trait-object-bounds-cycle-4.rs @@ -0,0 +1,25 @@ +// Check that we don't have a cycle when we try to normalize `Self::U` in the +// bound below. Make sure that having a lifetime on the trait object doesn't break things + +// check-pass + +trait Is { + type T; +} + +impl Is for U { + type T = U; +} + +trait Obj<'a> { + type U: Is; + type V; +} + +fn is_obj<'a, T: ?Sized + Obj<'a>>(_: &T) {} + +fn f<'a>(x: &dyn Obj<'a, U = i32, V = i32>) { + is_obj(x) +} + +fn main() {}