From 6c43a649313a2a9d03923598c6a5b260624e8f66 Mon Sep 17 00:00:00 2001
From: Matthew Jasper <mjjasper1@gmail.com>
Date: Thu, 8 Oct 2020 21:49:36 +0100
Subject: [PATCH] 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.
---
 .../src/traits/select/confirmation.rs         | 18 +++++--------
 .../src/traits/select/mod.rs                  | 11 +++-----
 .../normalization-probe-cycle.rs              | 25 +++++++++++++++++++
 3 files changed, 34 insertions(+), 20 deletions(-)
 create mode 100644 src/test/ui/associated-types/normalization-probe-cycle.rs

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(_))
diff --git a/src/test/ui/associated-types/normalization-probe-cycle.rs b/src/test/ui/associated-types/normalization-probe-cycle.rs
new file mode 100644
index 00000000000..9c1a488e951
--- /dev/null
+++ b/src/test/ui/associated-types/normalization-probe-cycle.rs
@@ -0,0 +1,25 @@
+// Regression test for #77656
+
+// check-pass
+
+trait Value: PartialOrd {}
+
+impl<T: PartialOrd> Value for T {}
+
+trait Distance
+where
+    Self: PartialOrd<<Self as Distance>::Value>,
+    Self: PartialOrd,
+{
+    type Value: Value;
+}
+
+impl<T: Value> Distance for T {
+    type Value = T;
+}
+
+trait Proximity<T = Self> {
+    type Distance: Distance;
+}
+
+fn main() {}