Rollup merge of rust-lang#114117 - compiler-errors:return-to-uniq, r=lcnr

Restore region uniquification in the new solver 🎉

All of the bugs that were "due" to uniquification have been settled via other means (e.g. bidirectional alias-relate, param-env incompleteness, etc).

Firstly, revert the functional changes in rust-lang#110180. 😸

Secondly, we need to ignore regions when considering if a goal has changed (the "has_changed" boolean returned from `evaluate_goal`) -- otherwise, because we're doing region uniquification, we may perpetually consider a goal to be changed. See the UI test I committed for an explanation.

Author: GuillaumeGomez

compiler/rustc_trait_selection/src/solve/canonicalize.rs

@@ -125,9 +125,8 @@ impl<'a, 'tcx> Canonicalizer<'a, 'tcx> {
         // - var_infos: [E0, U1, E1, U1, E1, E6, U6], curr_compressed_uv: 1, next_orig_uv: 6
         // - var_infos: [E0, U1, E1, U1, E1, E2, U2], curr_compressed_uv: 2, next_orig_uv: -
         //
-        // This algorithm runs in `O(nm)` where `n` is the number of different universe
-        // indices in the input and `m` is the number of canonical variables.
-        // This should be fine as both `n` and `m` are expected to be small.
+        // This algorithm runs in `O(n²)` where `n` is the number of different universe
+        // indices in the input. This should be fine as `n` is expected to be small.
         let mut curr_compressed_uv = ty::UniverseIndex::ROOT;
         let mut existential_in_new_uv = false;
         let mut next_orig_uv = Some(ty::UniverseIndex::ROOT);
@@ -263,14 +262,18 @@ impl<'tcx> TypeFolder<TyCtxt<'tcx>> for Canonicalizer<'_, 'tcx> {
             ty::ReError(_) => return r,
         };
 
-        let var = ty::BoundVar::from(
-            self.variables.iter().position(|&v| v == r.into()).unwrap_or_else(|| {
-                let var = self.variables.len();
-                self.variables.push(r.into());
-                self.primitive_var_infos.push(CanonicalVarInfo { kind });
-                var
-            }),
-        );
+        let existing_bound_var = match self.canonicalize_mode {
+            CanonicalizeMode::Input => None,
+            CanonicalizeMode::Response { .. } => {
+                self.variables.iter().position(|&v| v == r.into()).map(ty::BoundVar::from)
+            }
+        };
+        let var = existing_bound_var.unwrap_or_else(|| {
+            let var = ty::BoundVar::from(self.variables.len());
+            self.variables.push(r.into());
+            self.primitive_var_infos.push(CanonicalVarInfo { kind });
+            var
+        });
         let br = ty::BoundRegion { var, kind: BrAnon(None) };
         ty::Region::new_late_bound(self.interner(), self.binder_index, br)
     }

compiler/rustc_trait_selection/src/solve/eval_ctxt.rs

@@ -344,7 +344,7 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
             Ok(response) => response,
         };
 
-        let has_changed = !canonical_response.value.var_values.is_identity()
+        let has_changed = !canonical_response.value.var_values.is_identity_modulo_regions()
             || !canonical_response.value.external_constraints.opaque_types.is_empty();
         let (certainty, nested_goals) = match self.instantiate_and_apply_query_response(
             goal.param_env,

tests/ui/impl-trait/autoderef.rs

@@ -1,5 +1,5 @@
 // revisions: current next
-//[next] compile-flag: -Ztrait-solver=next
+//[next] compile-flags: -Ztrait-solver=next
 // check-pass
 
 use std::path::Path;

tests/ui/traits/new-solver/dont-loop-fulfill-on-region-constraints.rs

@@ -0,0 +1,32 @@
+// compile-flags: -Ztrait-solver=next
+// check-pass
+
+trait Eq<'a, 'b, T> {}
+
+trait Ambig {}
+impl Ambig for () {}
+
+impl<'a, T> Eq<'a, 'a, T> for () where T: Ambig {}
+
+fn eq<'a, 'b, T>(t: T)
+where
+    (): Eq<'a, 'b, T>,
+{
+}
+
+fn test<'r>() {
+    let mut x = Default::default();
+
+    // When we evaluate `(): Eq<'r, 'r, ?0>` we uniquify the regions.
+    // That leads us to evaluate `(): Eq<'?0, '?1, ?0>`. The response of this
+    // will be ambiguous (because `?0: Ambig` is ambig) and also not an "identity"
+    // response, since the region constraints will contain `'?0 == '?1` (so
+    // `is_changed` will return true). Since it's both ambig and changed,
+    // fulfillment will both re-register the goal AND loop again. This hits the
+    // overflow limit. This should neither be considered overflow, nor ICE.
+    eq::<'r, 'r, _>(x);
+
+    x = ();
+}
+
+fn main() {}