Avoid cycle with projections from object types

Normalizing `<dyn Iterator<Item = ()> as Iterator>::Item` no longer
requires selecting `dyn Iterator<Item = ()>: Iterator`. This was
previously worked around by using a special type-folder to normalize
things.

Author: matthewjasper

compiler/rustc_trait_selection/src/traits/project.rs

@@ -1,6 +1,5 @@
 //! Code for projecting associated types out of trait references.
 
-use super::elaborate_predicates;
 use super::specialization_graph;
 use super::translate_substs;
 use super::util;
@@ -53,13 +52,16 @@ pub enum ProjectionTyError<'tcx> {
 
 #[derive(PartialEq, Eq, Debug)]
 enum ProjectionTyCandidate<'tcx> {
-    // from a where-clause in the env or object type
+    /// From a where-clause in the env or object type
     ParamEnv(ty::PolyProjectionPredicate<'tcx>),
 
-    // from the definition of `Trait` when you have something like <<A as Trait>::B as Trait2>::C
+    /// From the definition of `Trait` when you have something like <<A as Trait>::B as Trait2>::C
     TraitDef(ty::PolyProjectionPredicate<'tcx>),
 
-    // from a "impl" (or a "pseudo-impl" returned by select)
+    /// Bounds specified on an object type
+    Object(ty::PolyProjectionPredicate<'tcx>),
+
+    /// From a "impl" (or a "pseudo-impl" returned by select)
     Select(Selection<'tcx>),
 }
 
@@ -561,14 +563,6 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
             } else {
                 obligations.extend(ty.obligations);
             }
-
-            obligations.push(get_paranoid_cache_value_obligation(
-                infcx,
-                param_env,
-                projection_ty,
-                cause,
-                depth,
-            ));
             return Ok(Some(ty.value));
         }
         Err(ProjectionCacheEntry::Error) => {
@@ -703,45 +697,6 @@ fn prune_cache_value_obligations<'a, 'tcx>(
     NormalizedTy { value: result.value, obligations }
 }
 
-/// Whenever we give back a cache result for a projection like `<T as
-/// Trait>::Item ==> X`, we *always* include the obligation to prove
-/// that `T: Trait` (we may also include some other obligations). This
-/// may or may not be necessary -- in principle, all the obligations
-/// that must be proven to show that `T: Trait` were also returned
-/// when the cache was first populated. But there are some vague concerns,
-/// and so we take the precautionary measure of including `T: Trait` in
-/// the result:
-///
-/// Concern #1. The current setup is fragile. Perhaps someone could
-/// have failed to prove the concerns from when the cache was
-/// populated, but also not have used a snapshot, in which case the
-/// cache could remain populated even though `T: Trait` has not been
-/// shown. In this case, the "other code" is at fault -- when you
-/// project something, you are supposed to either have a snapshot or
-/// else prove all the resulting obligations -- but it's still easy to
-/// get wrong.
-///
-/// Concern #2. Even within the snapshot, if those original
-/// obligations are not yet proven, then we are able to do projections
-/// that may yet turn out to be wrong. This *may* lead to some sort
-/// of trouble, though we don't have a concrete example of how that
-/// can occur yet. But it seems risky at best.
-fn get_paranoid_cache_value_obligation<'a, 'tcx>(
-    infcx: &'a InferCtxt<'a, 'tcx>,
-    param_env: ty::ParamEnv<'tcx>,
-    projection_ty: ty::ProjectionTy<'tcx>,
-    cause: ObligationCause<'tcx>,
-    depth: usize,
-) -> PredicateObligation<'tcx> {
-    let trait_ref = projection_ty.trait_ref(infcx.tcx).to_poly_trait_ref();
-    Obligation {
-        cause,
-        recursion_depth: depth,
-        param_env,
-        predicate: trait_ref.without_const().to_predicate(infcx.tcx),
-    }
-}
-
 /// If we are projecting `<T as Trait>::Item`, but `T: Trait` does not
 /// hold. In various error cases, we cannot generate a valid
 /// normalized projection. Therefore, we create an inference variable
@@ -848,12 +803,21 @@ fn project_type<'cx, 'tcx>(
 
     assemble_candidates_from_trait_def(selcx, obligation, &obligation_trait_ref, &mut candidates);
 
-    assemble_candidates_from_impls(selcx, obligation, &obligation_trait_ref, &mut candidates);
+    assemble_candidates_from_object_ty(selcx, obligation, &obligation_trait_ref, &mut candidates);
+
+    if let ProjectionTyCandidateSet::Single(ProjectionTyCandidate::Object(_)) = candidates {
+        // Avoid normalization cycle from selection (see
+        // `assemble_candidates_from_object_ty`).
+        // FIXME(lazy_normalization): Lazy normalization should save us from
+        // having to do special case this.
+    } else {
+        assemble_candidates_from_impls(selcx, obligation, &obligation_trait_ref, &mut candidates);
+    };
 
     match candidates {
-        ProjectionTyCandidateSet::Single(candidate) => Ok(ProjectedTy::Progress(
-            confirm_candidate(selcx, obligation, &obligation_trait_ref, candidate),
-        )),
+        ProjectionTyCandidateSet::Single(candidate) => {
+            Ok(ProjectedTy::Progress(confirm_candidate(selcx, obligation, candidate)))
+        }
         ProjectionTyCandidateSet::None => Ok(ProjectedTy::NoProgress(
             selcx
                 .tcx()
@@ -932,6 +896,53 @@ fn assemble_candidates_from_trait_def<'cx, 'tcx>(
     )
 }
 
+/// In the case of a trait object like
+/// `<dyn Iterator<Item = ()> as Iterator>::Item` we can use the existential
+/// predicate in the trait object.
+///
+/// We don't go through the select candidate for these bounds to avoid cycles:
+/// In the above case, `dyn Iterator<Item = ()>: Iterator` would create a
+/// nested obligation of `<dyn Iterator<Item = ()> as Iterator>::Item: Sized`,
+/// this then has to be normalized without having to prove
+/// `dyn Iterator<Item = ()>: Iterator` again.
+fn assemble_candidates_from_object_ty<'cx, 'tcx>(
+    selcx: &mut SelectionContext<'cx, 'tcx>,
+    obligation: &ProjectionTyObligation<'tcx>,
+    obligation_trait_ref: &ty::TraitRef<'tcx>,
+    candidate_set: &mut ProjectionTyCandidateSet<'tcx>,
+) {
+    debug!("assemble_candidates_from_object_ty(..)");
+
+    let tcx = selcx.tcx();
+
+    let self_ty = obligation_trait_ref.self_ty();
+    let object_ty = selcx.infcx().shallow_resolve(self_ty);
+    let data = match object_ty.kind {
+        ty::Dynamic(ref data, ..) => data,
+        ty::Infer(ty::TyVar(_)) => {
+            // If the self-type is an inference variable, then it MAY wind up
+            // being an object type, so induce an ambiguity.
+            candidate_set.mark_ambiguous();
+            return;
+        }
+        _ => return,
+    };
+    let env_predicates = data
+        .projection_bounds()
+        .filter(|bound| bound.item_def_id() == obligation.predicate.item_def_id)
+        .map(|p| p.with_self_ty(tcx, object_ty).to_predicate(tcx));
+
+    assemble_candidates_from_predicates(
+        selcx,
+        obligation,
+        obligation_trait_ref,
+        candidate_set,
+        ProjectionTyCandidate::Object,
+        env_predicates,
+        false,
+    );
+}
+
 fn assemble_candidates_from_predicates<'cx, 'tcx>(
     selcx: &mut SelectionContext<'cx, 'tcx>,
     obligation: &ProjectionTyObligation<'tcx>,
@@ -1000,7 +1011,6 @@ fn assemble_candidates_from_impls<'cx, 'tcx>(
             super::ImplSource::Closure(_)
             | super::ImplSource::Generator(_)
             | super::ImplSource::FnPointer(_)
-            | super::ImplSource::Object(_)
             | super::ImplSource::TraitAlias(_) => {
                 debug!("assemble_candidates_from_impls: impl_source={:?}", impl_source);
                 true
@@ -1125,6 +1135,12 @@ fn assemble_candidates_from_impls<'cx, 'tcx>(
                 // in `assemble_candidates_from_param_env`.
                 false
             }
+            super::ImplSource::Object(_) => {
+                // Handled by the `Object` projection candidate. See
+                // `assemble_candidates_from_object_ty` for an explanation of
+                // why we special case object types.
+                false
+            }
             super::ImplSource::AutoImpl(..) | super::ImplSource::Builtin(..) => {
                 // These traits have no associated types.
                 selcx.tcx().sess.delay_span_bug(
@@ -1150,13 +1166,13 @@ fn assemble_candidates_from_impls<'cx, 'tcx>(
 fn confirm_candidate<'cx, 'tcx>(
     selcx: &mut SelectionContext<'cx, 'tcx>,
     obligation: &ProjectionTyObligation<'tcx>,
-    obligation_trait_ref: &ty::TraitRef<'tcx>,
     candidate: ProjectionTyCandidate<'tcx>,
 ) -> Progress<'tcx> {
     debug!("confirm_candidate(candidate={:?}, obligation={:?})", candidate, obligation);
 
     let mut progress = match candidate {
-        ProjectionTyCandidate::ParamEnv(poly_projection) => {
+        ProjectionTyCandidate::ParamEnv(poly_projection)
+        | ProjectionTyCandidate::Object(poly_projection) => {
             confirm_param_env_candidate(selcx, obligation, poly_projection, false)
         }
 
@@ -1165,7 +1181,7 @@ fn confirm_candidate<'cx, 'tcx>(
         }
 
         ProjectionTyCandidate::Select(impl_source) => {
-            confirm_select_candidate(selcx, obligation, obligation_trait_ref, impl_source)
+            confirm_select_candidate(selcx, obligation, impl_source)
         }
     };
     // When checking for cycle during evaluation, we compare predicates with
@@ -1182,7 +1198,6 @@ fn confirm_candidate<'cx, 'tcx>(
 fn confirm_select_candidate<'cx, 'tcx>(
     selcx: &mut SelectionContext<'cx, 'tcx>,
     obligation: &ProjectionTyObligation<'tcx>,
-    obligation_trait_ref: &ty::TraitRef<'tcx>,
     impl_source: Selection<'tcx>,
 ) -> Progress<'tcx> {
     match impl_source {
@@ -1193,10 +1208,8 @@ fn confirm_select_candidate<'cx, 'tcx>(
         super::ImplSource::DiscriminantKind(data) => {
             confirm_discriminant_kind_candidate(selcx, obligation, data)
         }
-        super::ImplSource::Object(_) => {
-            confirm_object_candidate(selcx, obligation, obligation_trait_ref)
-        }
-        super::ImplSource::AutoImpl(..)
+        super::ImplSource::Object(_)
+        | super::ImplSource::AutoImpl(..)
         | super::ImplSource::Param(..)
         | super::ImplSource::Builtin(..)
         | super::ImplSource::TraitAlias(..) =>
@@ -1211,72 +1224,6 @@ fn confirm_select_candidate<'cx, 'tcx>(
     }
 }
 
-fn confirm_object_candidate<'cx, 'tcx>(
-    selcx: &mut SelectionContext<'cx, 'tcx>,
-    obligation: &ProjectionTyObligation<'tcx>,
-    obligation_trait_ref: &ty::TraitRef<'tcx>,
-) -> Progress<'tcx> {
-    let self_ty = obligation_trait_ref.self_ty();
-    let object_ty = selcx.infcx().shallow_resolve(self_ty);
-    debug!("confirm_object_candidate(object_ty={:?})", object_ty);
-    let data = match object_ty.kind() {
-        ty::Dynamic(data, ..) => data,
-        _ => span_bug!(
-            obligation.cause.span,
-            "confirm_object_candidate called with non-object: {:?}",
-            object_ty
-        ),
-    };
-    let env_predicates = data
-        .projection_bounds()
-        .map(|p| p.with_self_ty(selcx.tcx(), object_ty).to_predicate(selcx.tcx()));
-    let env_predicate = {
-        let env_predicates = elaborate_predicates(selcx.tcx(), env_predicates);
-
-        // select only those projections that are actually projecting an
-        // item with the correct name
-
-        let env_predicates = env_predicates.filter_map(|o| match o.predicate.skip_binders() {
-            ty::PredicateAtom::Projection(data)
-                if data.projection_ty.item_def_id == obligation.predicate.item_def_id =>
-            {
-                Some(ty::Binder::bind(data))
-            }
-            _ => None,
-        });
-
-        // select those with a relevant trait-ref
-        let mut env_predicates = env_predicates.filter(|data| {
-            let data_poly_trait_ref = data.to_poly_trait_ref(selcx.tcx());
-            let obligation_poly_trait_ref = obligation_trait_ref.to_poly_trait_ref();
-            selcx.infcx().probe(|_| {
-                selcx
-                    .infcx()
-                    .at(&obligation.cause, obligation.param_env)
-                    .sup(obligation_poly_trait_ref, data_poly_trait_ref)
-                    .is_ok()
-            })
-        });
-
-        // select the first matching one; there really ought to be one or
-        // else the object type is not WF, since an object type should
-        // include all of its projections explicitly
-        match env_predicates.next() {
-            Some(env_predicate) => env_predicate,
-            None => {
-                debug!(
-                    "confirm_object_candidate: no env-predicate \
-                     found in object type `{:?}`; ill-formed",
-                    object_ty
-                );
-                return Progress::error(selcx.tcx());
-            }
-        }
-    };
-
-    confirm_param_env_candidate(selcx, obligation, env_predicate, false)
-}
-
 fn confirm_generator_candidate<'cx, 'tcx>(
     selcx: &mut SelectionContext<'cx, 'tcx>,
     obligation: &ProjectionTyObligation<'tcx>,