Rollup merge of #135892 - BoxyUwU:next_solver_deduce_sig_normalized, r=lcnr

-Znext-solver: "normalize" signature before checking it mentions self in `deduce_closure_signature`

Fixes rust-lang/trait-system-refactor-initiative#138

r? ``@lcnr``

Tbh I wrote so much comments I don't feel like writing it all again but as a description.

Author: matthiaskrgr

compiler/rustc_hir_typeck/src/closure.rs

@@ -296,7 +296,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
     /// Given the expected type, figures out what it can about this closure we
     /// are about to type check:
-    #[instrument(skip(self), level = "debug")]
+    #[instrument(skip(self), level = "debug", ret)]
     fn deduce_closure_signature(
         &self,
         expected_ty: Ty<'tcx>,
@@ -378,6 +378,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         bound_predicate.rebind(proj_predicate),
                     ),
                 );
+
                 // Make sure that we didn't infer a signature that mentions itself.
                 // This can happen when we elaborate certain supertrait bounds that
                 // mention projections containing the `Self` type. See #105401.
@@ -395,8 +396,45 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         }
                     }
                 }
-                if inferred_sig.visit_with(&mut MentionsTy { expected_ty }).is_continue() {
-                    expected_sig = inferred_sig;
+
+                // Don't infer a closure signature from a goal that names the closure type as this will
+                // (almost always) lead to occurs check errors later in type checking.
+                if self.next_trait_solver()
+                    && let Some(inferred_sig) = inferred_sig
+                {
+                    // In the new solver it is difficult to explicitly normalize the inferred signature as we
+                    // would have to manually handle universes and rewriting bound vars and placeholders back
+                    // and forth.
+                    //
+                    // Instead we take advantage of the fact that we relating an inference variable with an alias
+                    // will only instantiate the variable if the alias is rigid(*not quite). Concretely we:
+                    // - Create some new variable `?sig`
+                    // - Equate `?sig` with the unnormalized signature, e.g. `fn(<Foo<?x> as Trait>::Assoc)`
+                    // - Depending on whether `<Foo<?x> as Trait>::Assoc` is rigid, ambiguous or normalizeable,
+                    //   we will either wind up with `?sig=<Foo<?x> as Trait>::Assoc/?y/ConcreteTy` respectively.
+                    //
+                    // *: In cases where there are ambiguous aliases in the signature that make use of bound vars
+                    //    they will wind up present in `?sig` even though they are non-rigid.
+                    //
+                    //    This is a bit weird and means we may wind up discarding the goal due to it naming `expected_ty`
+                    //    even though the normalized form may not name `expected_ty`. However, this matches the existing
+                    //    behaviour of the old solver and would be technically a breaking change to fix.
+                    let generalized_fnptr_sig = self.next_ty_var(span);
+                    let inferred_fnptr_sig = Ty::new_fn_ptr(self.tcx, inferred_sig.sig);
+                    self.demand_eqtype(span, inferred_fnptr_sig, generalized_fnptr_sig);
+
+                    let resolved_sig = self.resolve_vars_if_possible(generalized_fnptr_sig);
+
+                    if resolved_sig.visit_with(&mut MentionsTy { expected_ty }).is_continue() {
+                        expected_sig = Some(ExpectedSig {
+                            cause_span: inferred_sig.cause_span,
+                            sig: resolved_sig.fn_sig(self.tcx),
+                        });
+                    }
+                } else {
+                    if inferred_sig.visit_with(&mut MentionsTy { expected_ty }).is_continue() {
+                        expected_sig = inferred_sig;
+                    }
                 }
             }
 

compiler/rustc_hir_typeck/src/fn_ctxt/inspect_obligations.rs

@@ -7,6 +7,7 @@ use rustc_span::Span;
 use rustc_trait_selection::solve::inspect::{
     InspectConfig, InspectGoal, ProofTreeInferCtxtExt, ProofTreeVisitor,
 };
+use rustc_type_ir::solve::GoalSource;
 use tracing::{debug, instrument, trace};
 
 use crate::FnCtxt;
@@ -119,7 +120,21 @@ impl<'a, 'tcx> ProofTreeVisitor<'tcx> for NestedObligationsForSelfTy<'a, 'tcx> {
     fn visit_goal(&mut self, inspect_goal: &InspectGoal<'_, 'tcx>) {
         let tcx = self.fcx.tcx;
         let goal = inspect_goal.goal();
-        if self.fcx.predicate_has_self_ty(goal.predicate, self.self_ty) {
+        if self.fcx.predicate_has_self_ty(goal.predicate, self.self_ty)
+            // We do not push the instantiated forms of goals as it would cause any
+            // aliases referencing bound vars to go from having escaping bound vars to
+            // being able to be normalized to an inference variable.
+            //
+            // This is mostly just a hack as arbitrary nested goals could still contain
+            // such aliases while having a different `GoalSource`. Closure signature inference
+            // however can't really handle *every* higher ranked `Fn` goal also being present
+            // in the form of `?c: Fn<(<?x as Trait<'!a>>::Assoc)`.
+            //
+            // This also just better matches the behaviour of the old solver where we do not
+            // encounter instantiated forms of goals, only nested goals that referred to bound
+            // vars from instantiated goals.
+            && !matches!(inspect_goal.source(), GoalSource::InstantiateHigherRanked)
+        {
             self.obligations_for_self_ty.push(traits::Obligation::new(
                 tcx,
                 self.root_cause.clone(),

tests/ui/borrowck/issue-103095.rs

@@ -1,4 +1,7 @@
 //@ check-pass
+//@ revisions: current next
+//@ ignore-compare-mode-next-solver (explicit revisions)
+//@[next] compile-flags: -Znext-solver
 
 trait FnOnceForGenericRef<T>: FnOnce(&T) -> Self::FnOutput {
     type FnOutput;
@@ -16,10 +19,7 @@ struct Data<T, D: FnOnceForGenericRef<T>> {
 impl<T, D: FnOnceForGenericRef<T>> Data<T, D> {
     fn new(value: T, f: D) -> Self {
         let output = f(&value);
-        Self {
-            value: Some(value),
-            output: Some(output),
-        }
+        Self { value: Some(value), output: Some(output) }
     }
 }
 

tests/ui/closures/supertrait-hint-references-assoc-ty.rs

@@ -1,4 +1,7 @@
 //@ check-pass
+//@ revisions: current next
+//@ ignore-compare-mode-next-solver (explicit revisions)
+//@[next] compile-flags: -Znext-solver
 
 pub trait Fn0: Fn(i32) -> Self::Out {
     type Out;

tests/ui/mismatched_types/closure-arg-type-mismatch-issue-45727.current.fixed

@@ -9,4 +9,5 @@ fn main() {
     let _ = (-10..=10).find(|x: &i32| x.signum() == 0);
     //[current]~^ ERROR type mismatch in closure arguments
     //[next]~^^ ERROR expected `RangeInclusive<{integer}>` to be an iterator that yields `&&i32`, but it yields `{integer}`
+    //[next]~| ERROR expected a `FnMut(&<RangeInclusive<{integer}> as Iterator>::Item)` closure, found
 }

tests/ui/mismatched_types/closure-arg-type-mismatch-issue-45727.next.stderr

@@ -13,12 +13,26 @@ note: required by a bound in `find`
   --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
 
 error[E0271]: expected `RangeInclusive<{integer}>` to be an iterator that yields `&&i32`, but it yields `{integer}`
-  --> $DIR/closure-arg-type-mismatch-issue-45727.rs:9:33
+  --> $DIR/closure-arg-type-mismatch-issue-45727.rs:9:24
    |
 LL |     let _ = (-10..=10).find(|x: &&&i32| x.signum() == 0);
-   |                                 ^^^^^^ expected `&&i32`, found integer
+   |                        ^^^^ expected `&&i32`, found integer
 
-error: aborting due to 2 previous errors
+error[E0277]: expected a `FnMut(&<RangeInclusive<{integer}> as Iterator>::Item)` closure, found `{closure@$DIR/closure-arg-type-mismatch-issue-45727.rs:9:29: 9:40}`
+  --> $DIR/closure-arg-type-mismatch-issue-45727.rs:9:29
+   |
+LL |     let _ = (-10..=10).find(|x: &&&i32| x.signum() == 0);
+   |                        ---- ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected an `FnMut(&<RangeInclusive<{integer}> as Iterator>::Item)` closure, found `{closure@$DIR/closure-arg-type-mismatch-issue-45727.rs:9:29: 9:40}`
+   |                        |
+   |                        required by a bound introduced by this call
+   |
+   = help: the trait `for<'a> FnMut(&'a <RangeInclusive<{integer}> as Iterator>::Item)` is not implemented for closure `{closure@$DIR/closure-arg-type-mismatch-issue-45727.rs:9:29: 9:40}`
+   = note: expected a closure with arguments `(&&&i32,)`
+              found a closure with arguments `(&<RangeInclusive<{integer}> as Iterator>::Item,)`
+note: required by a bound in `find`
+  --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
+
+error: aborting due to 3 previous errors
 
 Some errors have detailed explanations: E0271, E0277.
 For more information about an error, try `rustc --explain E0271`.

tests/ui/mismatched_types/closure-arg-type-mismatch-issue-45727.rs

@@ -9,4 +9,5 @@ fn main() {
     let _ = (-10..=10).find(|x: &&&i32| x.signum() == 0);
     //[current]~^ ERROR type mismatch in closure arguments
     //[next]~^^ ERROR expected `RangeInclusive<{integer}>` to be an iterator that yields `&&i32`, but it yields `{integer}`
+    //[next]~| ERROR expected a `FnMut(&<RangeInclusive<{integer}> as Iterator>::Item)` closure, found
 }

tests/ui/traits/next-solver/closure-signature-inference-hr-ambig-alias-naming-self.rs

@@ -0,0 +1,52 @@
+//@ check-pass
+//@ revisions: current next
+//@[next] compile-flags: -Znext-solver
+
+// When type checking a closure expr we look at the list of unsolved goals
+// to determine if there are any bounds on the closure type to infer a signature from.
+//
+// We attempt to discard goals that name the closure type so as to avoid inferring the
+// closure type to something like `?x = closure(sig=fn(?x))`. This test checks that when
+// such a goal names the closure type inside of an ambiguous alias and there exists another
+// potential goal to infer the closure signature from, we do that.
+
+trait Trait<'a> {
+    type Assoc;
+}
+
+impl<'a, F> Trait<'a> for F {
+    type Assoc = u32;
+}
+
+fn closure_typer1<F>(_: F)
+where
+    F: Fn(u32) + for<'a> Fn(<F as Trait<'a>>::Assoc),
+{
+}
+
+fn closure_typer2<F>(_: F)
+where
+    F: for<'a> Fn(<F as Trait<'a>>::Assoc) + Fn(u32),
+{
+}
+
+fn main() {
+    // Here we have some closure with a yet to be inferred type of `?c`. There are two goals
+    // involving `?c` that can be used to determine the closure signature:
+    // - `?c: for<'a> Fn<(<?c as Trait<'a>>::Assoc,), Output = ()>`
+    // - `?c: Fn<(u32,), Output = ()>`
+    //
+    // If we were to infer the argument of the closure (`x` below) to `<?c as Trait<'a>>::Assoc`
+    // then we would not be able to call `x.into()` as `x` is some unknown type. Instead we must
+    // use the `?c: Fn(u32)` goal to infer a signature in order for this code to compile.
+    //
+    // As the algorithm for picking a goal to infer the signature from is dependent on the ordering
+    // of pending goals in the type checker, we test both orderings of bounds to ensure we aren't
+    // testing that we just *happen* to pick `?c: Fn(u32)`.
+    closure_typer1(move |x| {
+        let _: u32 = x.into();
+    });
+    closure_typer2(move |x| {
+        let _: u32 = x.into();
+    });
+}