Fix ICE from projection cycle

Cycles in normalization can cause evaluations to change from Unknown to
Err. This means that some selection that were applicable no longer are.

To avoid this:
* Selection candidates that are known to be applicable are prefered
  over candidates that are not.
* We don't ICE if a candidate is no longer applicable.

2 files changed, 9 insertions, 20 deletions

diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs
index 37d619d5942..7fd588ffce3 100644
--- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs
+++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs
@@ -69,7 +69,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             }
 
             ProjectionCandidate(idx) => {
-                let obligations = self.confirm_projection_candidate(obligation, idx);
+                let obligations = self.confirm_projection_candidate(obligation, idx)?;
                 Ok(ImplSource::Param(obligations))
             }
 
@@ -120,7 +120,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         &mut self,
         obligation: &TraitObligation<'tcx>,
         idx: usize,
-    ) -> Vec<PredicateObligation<'tcx>> {
+    ) -> Result<Vec<PredicateObligation<'tcx>>, SelectionError<'tcx>> {
         self.infcx.commit_unconditionally(|_| {
             let tcx = self.tcx();
 
@@ -148,19 +148,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 &mut obligations,
             );
 
-            obligations.extend(
+            obligations.extend(self.infcx.commit_if_ok(|_| {
                 self.infcx
                     .at(&obligation.cause, obligation.param_env)
                     .sup(placeholder_trait_predicate.trait_ref.to_poly_trait_ref(), candidate)
                     .map(|InferOk { obligations, .. }| obligations)
-                    .unwrap_or_else(|_| {
-                        bug!(
-                            "Projection bound `{:?}` was applicable to `{:?}` but now is not",
-                            candidate,
-                            obligation
-                        );
-                    }),
-            );
+                    .map_err(|_| Unimplemented)
+            })?);
 
             if let ty::Projection(..) = placeholder_self_ty.kind() {
                 for predicate in tcx.predicates_of(def_id).instantiate_own(tcx, substs).predicates {
@@ -181,7 +175,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 }
             }
 
-            obligations
+            Ok(obligations)
         })
     }
 
diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs
index b838602e76c..ce7b963473f 100644
--- a/compiler/rustc_trait_selection/src/traits/select/mod.rs
+++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs
@@ -518,12 +518,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                             result
                         }
                         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),
+                        Ok(Err(project::InProgress)) => Ok(EvaluatedToRecur),
                         Err(_) => Ok(EvaluatedToErr),
                     }
                 }
@@ -1382,9 +1377,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             }
 
             (ProjectionCandidate(i), ProjectionCandidate(j)) => {
-                // Arbitrarily pick the first candidate for backwards
+                // Arbitrarily pick the lower numbered candidate for backwards
                 // compatibility reasons. Don't let this affect inference.
-                i > j && !needs_infer
+                i < j && !needs_infer
             }
             (ObjectCandidate, ObjectCandidate) => bug!("Duplicate object candidate"),
             (ObjectCandidate, ProjectionCandidate(_))