Auto merge of rust-lang#75494 - matthewjasper:defer-recursive-projection-error, r=nikomatsakis

Don't immediately error for cycles during normalization

rust-lang#73452 meant some normalization cycles could be detected earlier, breaking some code.
This PR makes defers errors for normalization cycles to fulfillment, fixing said code.

Fixes rust-lang#74868

r? @nikomatsakis

Author: bors

src/librustc_trait_selection/traits/auto_trait.rs

@@ -738,7 +738,7 @@ impl AutoTraitFinder<'tcx> {
                     // and turn them into an explicit negative impl for our type.
                     debug!("Projecting and unifying projection predicate {:?}", predicate);
 
-                    match poly_project_and_unify_type(select, &obligation.with(p)) {
+                    match project::poly_project_and_unify_type(select, &obligation.with(p)) {
                         Err(e) => {
                             debug!(
                                 "evaluate_nested_obligations: Unable to unify predicate \
@@ -747,7 +747,11 @@ impl AutoTraitFinder<'tcx> {
                             );
                             return false;
                         }
-                        Ok(Some(v)) => {
+                        Ok(Err(project::InProgress)) => {
+                            debug!("evaluate_nested_obligations: recursive projection predicate");
+                            return false;
+                        }
+                        Ok(Ok(Some(v))) => {
                             // We only care about sub-obligations
                             // when we started out trying to unify
                             // some inference variables. See the comment above
@@ -766,7 +770,7 @@ impl AutoTraitFinder<'tcx> {
                                 }
                             }
                         }
-                        Ok(None) => {
+                        Ok(Ok(None)) => {
                             // It's ok not to make progress when have no inference variables -
                             // in that case, we were only performing unifcation to check if an
                             // error occurred (which would indicate that it's impossible for our

src/librustc_trait_selection/traits/fulfill.rs

@@ -622,15 +622,20 @@ impl<'a, 'b, 'tcx> FulfillProcessor<'a, 'b, 'tcx> {
         project_obligation: PolyProjectionObligation<'tcx>,
         stalled_on: &mut Vec<TyOrConstInferVar<'tcx>>,
     ) -> ProcessResult<PendingPredicateObligation<'tcx>, FulfillmentErrorCode<'tcx>> {
+        let tcx = self.selcx.tcx();
         match project::poly_project_and_unify_type(self.selcx, &project_obligation) {
-            Ok(None) => {
+            Ok(Ok(Some(os))) => ProcessResult::Changed(mk_pending(os)),
+            Ok(Ok(None)) => {
                 *stalled_on = trait_ref_infer_vars(
                     self.selcx,
                     project_obligation.predicate.to_poly_trait_ref(self.selcx.tcx()),
                 );
                 ProcessResult::Unchanged
             }
-            Ok(Some(os)) => ProcessResult::Changed(mk_pending(os)),
+            // Let the caller handle the recursion
+            Ok(Err(project::InProgress)) => ProcessResult::Changed(mk_pending(vec![
+                project_obligation.with(project_obligation.predicate.to_predicate(tcx)),
+            ])),
             Err(e) => ProcessResult::Error(CodeProjectionError(e)),
         }
     }

src/librustc_trait_selection/traits/mod.rs

@@ -51,9 +51,7 @@ pub use self::object_safety::is_vtable_safe_method;
 pub use self::object_safety::MethodViolationCode;
 pub use self::object_safety::ObjectSafetyViolation;
 pub use self::on_unimplemented::{OnUnimplementedDirective, OnUnimplementedNote};
-pub use self::project::{
-    normalize, normalize_projection_type, normalize_to, poly_project_and_unify_type,
-};
+pub use self::project::{normalize, normalize_projection_type, normalize_to};
 pub use self::select::{EvaluationCache, SelectionCache, SelectionContext};
 pub use self::select::{EvaluationResult, IntercrateAmbiguityCause, OverflowError};
 pub use self::specialize::specialization_graph::FutureCompatOverlapError;

src/librustc_trait_selection/traits/project.rs

@@ -41,6 +41,8 @@ pub type ProjectionObligation<'tcx> = Obligation<'tcx, ty::ProjectionPredicate<'
 
 pub type ProjectionTyObligation<'tcx> = Obligation<'tcx, ty::ProjectionTy<'tcx>>;
 
+pub(super) struct InProgress;
+
 /// When attempting to resolve `<T as TraitRef>::Name` ...
 #[derive(Debug)]
 pub enum ProjectionTyError<'tcx> {
@@ -143,10 +145,26 @@ impl<'tcx> ProjectionTyCandidateSet<'tcx> {
 ///
 /// If successful, this may result in additional obligations. Also returns
 /// the projection cache key used to track these additional obligations.
-pub fn poly_project_and_unify_type<'cx, 'tcx>(
+///
+/// ## Returns
+///
+/// - `Err(_)`: the projection can be normalized, but is not equal to the
+///   expected type.
+/// - `Ok(Err(InProgress))`: this is called recursively while normalizing
+///   the same projection.
+/// - `Ok(Ok(None))`: The projection cannot be normalized due to ambiguity
+///   (resolving some inference variables in the projection may fix this).
+/// - `Ok(Ok(Some(obligations)))`: The projection bound holds subject to
+///    the given obligations. If the projection cannot be normalized because
+///    the required trait bound doesn't hold this returned with `obligations`
+///    being a predicate that cannot be proven.
+pub(super) fn poly_project_and_unify_type<'cx, 'tcx>(
     selcx: &mut SelectionContext<'cx, 'tcx>,
     obligation: &PolyProjectionObligation<'tcx>,
-) -> Result<Option<Vec<PredicateObligation<'tcx>>>, MismatchedProjectionTypes<'tcx>> {
+) -> Result<
+    Result<Option<Vec<PredicateObligation<'tcx>>>, InProgress>,
+    MismatchedProjectionTypes<'tcx>,
+> {
     debug!("poly_project_and_unify_type(obligation={:?})", obligation);
 
     let infcx = selcx.infcx();
@@ -165,10 +183,15 @@ pub fn poly_project_and_unify_type<'cx, 'tcx>(
 ///     <T as Trait>::U == V
 ///
 /// If successful, this may result in additional obligations.
+///
+/// See [poly_project_and_unify_type] for an explanation of the return value.
 fn project_and_unify_type<'cx, 'tcx>(
     selcx: &mut SelectionContext<'cx, 'tcx>,
     obligation: &ProjectionObligation<'tcx>,
-) -> Result<Option<Vec<PredicateObligation<'tcx>>>, MismatchedProjectionTypes<'tcx>> {
+) -> Result<
+    Result<Option<Vec<PredicateObligation<'tcx>>>, InProgress>,
+    MismatchedProjectionTypes<'tcx>,
+> {
     debug!("project_and_unify_type(obligation={:?})", obligation);
 
     let mut obligations = vec![];
@@ -180,8 +203,9 @@ fn project_and_unify_type<'cx, 'tcx>(
         obligation.recursion_depth,
         &mut obligations,
     ) {
-        Some(n) => n,
-        None => return Ok(None),
+        Ok(Some(n)) => n,
+        Ok(None) => return Ok(Ok(None)),
+        Err(InProgress) => return Ok(Err(InProgress)),
     };
 
     debug!(
@@ -196,7 +220,7 @@ fn project_and_unify_type<'cx, 'tcx>(
     {
         Ok(InferOk { obligations: inferred_obligations, value: () }) => {
             obligations.extend(inferred_obligations);
-            Ok(Some(obligations))
+            Ok(Ok(Some(obligations)))
         }
         Err(err) => {
             debug!("project_and_unify_type: equating types encountered error {:?}", err);
@@ -419,6 +443,8 @@ pub fn normalize_projection_type<'a, 'b, 'tcx>(
         depth,
         obligations,
     )
+    .ok()
+    .flatten()
     .unwrap_or_else(move || {
         // if we bottom out in ambiguity, create a type variable
         // and a deferred predicate to resolve this when more type
@@ -455,7 +481,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
     cause: ObligationCause<'tcx>,
     depth: usize,
     obligations: &mut Vec<PredicateObligation<'tcx>>,
-) -> Option<Ty<'tcx>> {
+) -> Result<Option<Ty<'tcx>>, InProgress> {
     let infcx = selcx.infcx();
 
     let projection_ty = infcx.resolve_vars_if_possible(&projection_ty);
@@ -487,7 +513,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
                 "opt_normalize_projection_type: \
                  found cache entry: ambiguous"
             );
-            return None;
+            return Ok(None);
         }
         Err(ProjectionCacheEntry::InProgress) => {
             // If while normalized A::B, we are asked to normalize
@@ -502,24 +528,14 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
             // to normalize `A::B`, we will want to check the
             // where-clauses in scope. So we will try to unify `A::B`
             // with `A::B`, which can trigger a recursive
-            // normalization. In that case, I think we will want this code:
-            //
-            // ```
-            // let ty = selcx.tcx().mk_projection(projection_ty.item_def_id,
-            //                                    projection_ty.substs;
-            // return Some(NormalizedTy { value: v, obligations: vec![] });
-            // ```
+            // normalization.
 
             debug!(
                 "opt_normalize_projection_type: \
                  found cache entry: in-progress"
             );
 
-            // But for now, let's classify this as an overflow:
-            let recursion_limit = selcx.tcx().sess.recursion_limit();
-            let obligation =
-                Obligation::with_depth(cause, recursion_limit.0, param_env, projection_ty);
-            selcx.infcx().report_overflow_error(&obligation, false);
+            return Err(InProgress);
         }
         Err(ProjectionCacheEntry::NormalizedTy(ty)) => {
             // This is the hottest path in this function.
@@ -555,7 +571,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
                 cause,
                 depth,
             ));
-            return Some(ty.value);
+            return Ok(Some(ty.value));
         }
         Err(ProjectionCacheEntry::Error) => {
             debug!(
@@ -564,7 +580,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
             );
             let result = normalize_to_error(selcx, param_env, projection_ty, cause, depth);
             obligations.extend(result.obligations);
-            return Some(result.value);
+            return Ok(Some(result.value));
         }
     }
 
@@ -611,7 +627,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
             let cache_value = prune_cache_value_obligations(infcx, &result);
             infcx.inner.borrow_mut().projection_cache().insert_ty(cache_key, cache_value);
             obligations.extend(result.obligations);
-            Some(result.value)
+            Ok(Some(result.value))
         }
         Ok(ProjectedTy::NoProgress(projected_ty)) => {
             debug!(
@@ -622,15 +638,15 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
             let result = Normalized { value: projected_ty, obligations: vec![] };
             infcx.inner.borrow_mut().projection_cache().insert_ty(cache_key, result.clone());
             // No need to extend `obligations`.
-            Some(result.value)
+            Ok(Some(result.value))
         }
         Err(ProjectionTyError::TooManyCandidates) => {
             debug!(
                 "opt_normalize_projection_type: \
                  too many candidates"
             );
             infcx.inner.borrow_mut().projection_cache().ambiguous(cache_key);
-            None
+            Ok(None)
         }
         Err(ProjectionTyError::TraitSelectionError(_)) => {
             debug!("opt_normalize_projection_type: ERROR");
@@ -642,7 +658,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
             infcx.inner.borrow_mut().projection_cache().error(cache_key);
             let result = normalize_to_error(selcx, param_env, projection_ty, cause, depth);
             obligations.extend(result.obligations);
-            Some(result.value)
+            Ok(Some(result.value))
         }
     }
 }
@@ -1116,11 +1132,11 @@ fn assemble_candidates_from_impls<'cx, 'tcx>(
             }
             super::ImplSourceAutoImpl(..) | super::ImplSourceBuiltin(..) => {
                 // These traits have no associated types.
-                span_bug!(
+                selcx.tcx().sess.delay_span_bug(
                     obligation.cause.span,
-                    "Cannot project an associated type from `{:?}`",
-                    impl_source
+                    &format!("Cannot project an associated type from `{:?}`", impl_source),
                 );
+                return Err(());
             }
         };
 

src/librustc_trait_selection/traits/select/mod.rs

@@ -503,7 +503,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 let data = ty::Binder::bind(data);
                 let project_obligation = obligation.with(data);
                 match project::poly_project_and_unify_type(self, &project_obligation) {
-                    Ok(Some(mut subobligations)) => {
+                    Ok(Ok(Some(mut subobligations))) => {
                         self.add_depth(subobligations.iter_mut(), obligation.recursion_depth);
                         let result = self.evaluate_predicates_recursively(
                             previous_stack,
@@ -516,7 +516,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                         }
                         result
                     }
-                    Ok(None) => Ok(EvaluatedToAmbig),
+                    Ok(Ok(None)) => Ok(EvaluatedToAmbig),
+                    // EvaluatedToRecur might also be acceptable here, but use
+                    // Unknown for now because it means that we won't dismiss a
+                    // selection candidate solely because it has a projection
+                    // cycle. This is closest to the previous behavior of
+                    // immediately erroring.
+                    Ok(Err(project::InProgress)) => Ok(EvaluatedToUnknown),
                     Err(_) => Ok(EvaluatedToErr),
                 }
             }

src/test/ui/associated-types/defaults-cyclic-fail-1.rs

@@ -26,16 +26,16 @@ impl Tr for u32 {
 
 // ...but only if this actually breaks the cycle
 impl Tr for bool {
-//~^ ERROR overflow evaluating the requirement
+    //~^ ERROR type mismatch resolving `<bool as Tr>::B == _`
     type A = Box<Self::B>;
-    //~^ ERROR overflow evaluating the requirement
+    //~^ ERROR type mismatch resolving `<bool as Tr>::B == _`
 }
 // (the error is shown twice for some reason)
 
 impl Tr for usize {
-//~^ ERROR overflow evaluating the requirement
+    //~^ ERROR type mismatch resolving `<usize as Tr>::B == _`
     type B = &'static Self::A;
-    //~^ ERROR overflow evaluating the requirement
+    //~^ ERROR type mismatch resolving `<usize as Tr>::A == _`
 }
 
 fn main() {

src/test/ui/associated-types/defaults-cyclic-fail-1.stderr

@@ -1,33 +1,34 @@
-error[E0275]: overflow evaluating the requirement `<() as Tr>::B`
+error[E0275]: overflow evaluating the requirement `<() as Tr>::B == _`
   --> $DIR/defaults-cyclic-fail-1.rs:10:6
    |
 LL | impl Tr for () {}
    |      ^^
 
-error[E0275]: overflow evaluating the requirement `<bool as Tr>::B`
+error[E0271]: type mismatch resolving `<bool as Tr>::B == _`
   --> $DIR/defaults-cyclic-fail-1.rs:28:6
    |
 LL | impl Tr for bool {
-   |      ^^
+   |      ^^ cyclic type of infinite size
 
-error[E0275]: overflow evaluating the requirement `<usize as Tr>::B`
+error[E0271]: type mismatch resolving `<usize as Tr>::B == _`
   --> $DIR/defaults-cyclic-fail-1.rs:35:6
    |
 LL | impl Tr for usize {
-   |      ^^
+   |      ^^ cyclic type of infinite size
 
-error[E0275]: overflow evaluating the requirement `<bool as Tr>::B`
+error[E0271]: type mismatch resolving `<bool as Tr>::B == _`
   --> $DIR/defaults-cyclic-fail-1.rs:30:5
    |
 LL |     type A = Box<Self::B>;
-   |     ^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^^^^^^^^^^^^^^^^^^ cyclic type of infinite size
 
-error[E0275]: overflow evaluating the requirement `<usize as Tr>::A`
+error[E0271]: type mismatch resolving `<usize as Tr>::A == _`
   --> $DIR/defaults-cyclic-fail-1.rs:37:5
    |
 LL |     type B = &'static Self::A;
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^ cyclic type of infinite size
 
 error: aborting due to 5 previous errors
 
-For more information about this error, try `rustc --explain E0275`.
+Some errors have detailed explanations: E0271, E0275.
+For more information about an error, try `rustc --explain E0271`.

src/test/ui/associated-types/defaults-cyclic-fail-2.rs

@@ -10,7 +10,7 @@ trait Tr {
 
 // ...but is an error in any impl that doesn't override at least one of the defaults
 impl Tr for () {}
-//~^ ERROR overflow evaluating the requirement
+//~^ ERROR type mismatch resolving `<() as Tr>::B == _`
 
 // As soon as at least one is redefined, it works:
 impl Tr for u8 {
@@ -28,16 +28,16 @@ impl Tr for u32 {
 
 // ...but only if this actually breaks the cycle
 impl Tr for bool {
-//~^ ERROR overflow evaluating the requirement
+    //~^ ERROR type mismatch resolving `<bool as Tr>::B == _`
     type A = Box<Self::B>;
-    //~^ ERROR overflow evaluating the requirement
+    //~^ ERROR type mismatch resolving `<bool as Tr>::B == _`
 }
 // (the error is shown twice for some reason)
 
 impl Tr for usize {
-//~^ ERROR overflow evaluating the requirement
+    //~^ ERROR type mismatch resolving `<usize as Tr>::B == _`
     type B = &'static Self::A;
-    //~^ ERROR overflow evaluating the requirement
+    //~^ ERROR type mismatch resolving `<usize as Tr>::A == _`
 }
 
 fn main() {