-Znext-solver: slightly strenghten closure sig inference

Author: lcnr

compiler/rustc_hir_typeck/src/closure.rs

@@ -9,17 +9,18 @@ use rustc_hir as hir;
 use rustc_hir::lang_items::LangItem;
 use rustc_hir_analysis::hir_ty_lowering::HirTyLowerer;
 use rustc_infer::infer::{BoundRegionConversionTime, DefineOpaqueTypes, InferOk, InferResult};
-use rustc_infer::traits::{ObligationCauseCode, PredicateObligations};
+use rustc_infer::traits::{ObligationCause, ObligationCauseCode, PredicateObligations};
 use rustc_macros::{TypeFoldable, TypeVisitable};
 use rustc_middle::span_bug;
+use rustc_middle::ty::error::TypeError;
 use rustc_middle::ty::{
-    self, ClosureKind, GenericArgs, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable,
-    TypeVisitableExt, TypeVisitor,
+    self, ClosureKind, GenericArgs, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable,
+    TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor,
 };
 use rustc_span::def_id::LocalDefId;
 use rustc_span::{DUMMY_SP, Span};
 use rustc_trait_selection::error_reporting::traits::ArgKind;
-use rustc_trait_selection::traits;
+use rustc_trait_selection::traits::{self, ObligationCtxt};
 use tracing::{debug, instrument, trace};
 
 use super::{CoroutineTypes, Expectation, FnCtxt, check_fn};
@@ -384,56 +385,83 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 // 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.
-                struct MentionsTy<'tcx> {
-                    expected_ty: Ty<'tcx>,
-                }
-                impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for MentionsTy<'tcx> {
-                    type Result = ControlFlow<()>;
-
-                    fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result {
-                        if t == self.expected_ty {
-                            ControlFlow::Break(())
-                        } else {
-                            t.super_visit_with(self)
-                        }
-                    }
-                }
-
-                // 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.
+                //
+                // Doing so 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.
+                    // If we've got `F: FnOnce(<u32 as Id<F>>::This)` we want to
+                    // use this to infer the signature `FnOnce(u32)` for the closure.
                     //
-                    //    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.
+                    // We handle self-referential aliases here by relying on generalization
+                    // which replaces such aliases with inference variables. This is currently
+                    // a bit too weak, see trait-system-refactor-initiative#191.
+                    struct ReplaceTy<'tcx> {
+                        tcx: TyCtxt<'tcx>,
+                        expected_ty: Ty<'tcx>,
+                        with_ty: Ty<'tcx>,
+                    }
+                    impl<'tcx> TypeFolder<TyCtxt<'tcx>> for ReplaceTy<'tcx> {
+                        fn cx(&self) -> TyCtxt<'tcx> {
+                            self.tcx
+                        }
+
+                        fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> {
+                            if t == self.expected_ty {
+                                self.with_ty
+                            } else {
+                                t.super_fold_with(self)
+                            }
+                        }
+                    }
                     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),
-                        });
+                    let inferred_fnptr_sig = inferred_fnptr_sig.fold_with(&mut ReplaceTy {
+                        tcx: self.tcx,
+                        expected_ty,
+                        with_ty: generalized_fnptr_sig,
+                    });
+                    let resolved_sig = self.commit_if_ok(|snapshot| {
+                        let outer_universe = self.universe();
+                        let ocx = ObligationCtxt::new(self);
+                        ocx.eq(
+                            &ObligationCause::dummy(),
+                            self.param_env,
+                            generalized_fnptr_sig,
+                            inferred_fnptr_sig,
+                        )?;
+                        if ocx.select_where_possible().is_empty() {
+                            self.leak_check(outer_universe, Some(snapshot))?;
+                            Ok(self.resolve_vars_if_possible(generalized_fnptr_sig))
+                        } else {
+                            Err(TypeError::Mismatch)
+                        }
+                    });
+                    match resolved_sig {
+                        Ok(resolved_sig) => {
+                            expected_sig = Some(ExpectedSig {
+                                cause_span: inferred_sig.cause_span,
+                                sig: resolved_sig.fn_sig(self.tcx),
+                            })
+                        }
+                        Err(_) => {}
                     }
                 } else {
+                    struct MentionsTy<'tcx> {
+                        expected_ty: Ty<'tcx>,
+                    }
+                    impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for MentionsTy<'tcx> {
+                        type Result = ControlFlow<()>;
+
+                        fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result {
+                            if t == self.expected_ty {
+                                ControlFlow::Break(())
+                            } else {
+                                t.super_visit_with(self)
+                            }
+                        }
+                    }
                     if inferred_sig.visit_with(&mut MentionsTy { expected_ty }).is_continue() {
                         expected_sig = inferred_sig;
                     }

compiler/rustc_infer/src/infer/relate/generalize.rs

@@ -6,8 +6,7 @@ use rustc_hir::def_id::DefId;
 use rustc_middle::bug;
 use rustc_middle::ty::error::TypeError;
 use rustc_middle::ty::{
-    self, AliasRelationDirection, InferConst, Term, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable,
-    TypeVisitableExt, TypeVisitor, TypingMode,
+    self, AliasRelationDirection, InferConst, Term, Ty, TyCtxt, TypeVisitableExt, TypingMode,
 };
 use rustc_span::Span;
 use tracing::{debug, instrument, warn};
@@ -290,45 +289,6 @@ impl<'tcx> InferCtxt<'tcx> {
     }
 }
 
-/// Finds the max universe present
-struct MaxUniverse {
-    max_universe: ty::UniverseIndex,
-}
-
-impl MaxUniverse {
-    fn new() -> Self {
-        MaxUniverse { max_universe: ty::UniverseIndex::ROOT }
-    }
-
-    fn max_universe(self) -> ty::UniverseIndex {
-        self.max_universe
-    }
-}
-
-impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for MaxUniverse {
-    fn visit_ty(&mut self, t: Ty<'tcx>) {
-        if let ty::Placeholder(placeholder) = t.kind() {
-            self.max_universe = self.max_universe.max(placeholder.universe);
-        }
-
-        t.super_visit_with(self)
-    }
-
-    fn visit_const(&mut self, c: ty::Const<'tcx>) {
-        if let ty::ConstKind::Placeholder(placeholder) = c.kind() {
-            self.max_universe = self.max_universe.max(placeholder.universe);
-        }
-
-        c.super_visit_with(self)
-    }
-
-    fn visit_region(&mut self, r: ty::Region<'tcx>) {
-        if let ty::RePlaceholder(placeholder) = r.kind() {
-            self.max_universe = self.max_universe.max(placeholder.universe);
-        }
-    }
-}
-
 /// The "generalizer" is used when handling inference variables.
 ///
 /// The basic strategy for handling a constraint like `?A <: B` is to
@@ -437,15 +397,8 @@ impl<'tcx> Generalizer<'_, 'tcx> {
                 if is_nested_alias {
                     return Err(e);
                 } else {
-                    let mut visitor = MaxUniverse::new();
-                    alias.visit_with(&mut visitor);
-                    let infer_replacement_is_complete =
-                        self.for_universe.can_name(visitor.max_universe())
-                            && !alias.has_escaping_bound_vars();
-                    if !infer_replacement_is_complete {
-                        warn!("may incompletely handle alias type: {alias:?}");
-                    }
-
+                    // FIXME(trait-system-refactor-initiative#8): This is incomplete
+                    // in case the alias has escaping bound vars.
                     debug!("generalization failure in alias");
                     Ok(self.next_ty_var_for_alias())
                 }

tests/ui/closures/deduce-signature/deduce-signature-infer-vars.rs

@@ -0,0 +1,40 @@
+//@ revisions: current next
+//@ ignore-compare-mode-next-solver (explicit revisions)
+//@[next] compile-flags: -Znext-solver
+//@ check-pass
+
+// Regression test for an issue found in #146720.
+
+trait Trait {
+    type Assoc;
+}
+
+struct W<T>(T);
+impl<T: Trait> Trait for W<T>
+where
+    u32: Trait<Assoc = T::Assoc>,
+{
+    type Assoc = T;
+}
+
+impl<T> Trait for (T,) {
+    type Assoc = T;
+}
+
+impl Trait for u32 {
+    type Assoc = u32;
+}
+
+fn foo<T: Trait>(_: impl FnOnce(T::Assoc)) {}
+
+fn main() {
+    // The closure signature is `fn(<W<?infer> as Trait>::Assoc)`.
+    // Normalizing it results in `?t` with `u32: Trait<Assoc = <?t>::Assoc>`.
+    // Equating `?t` with the argument pattern constrains it to `(?t,)`, at
+    // which point the `u32: Trait<Assoc = <?t>::Assoc>` obligations constrains
+    // `(?t,)` to `(u32,)`.
+    //
+    // This breaks when fudging inference to replace `?t` with an unconstrained
+    // infer var.
+    foo::<W<_>>(|(field,)| { let _ = field.count_ones(); })
+}

tests/ui/closures/deduce-signature/higher-ranked-alias-norm-to-hr.current_ambig.stderr

@@ -0,0 +1,49 @@
+error: implementation of `Foo` is not general enough
+  --> $DIR/higher-ranked-alias-norm-to-hr.rs:23:5
+   |
+LL |     needs_super(|_| {});
+   |     ^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough
+   |
+   = note: `{closure@$DIR/higher-ranked-alias-norm-to-hr.rs:23:17: 23:20}` must implement `Foo<'0>`, for any lifetime `'0`...
+   = note: ...but it actually implements `Foo<'1>`, for some specific lifetime `'1`
+
+error: implementation of `Fn` is not general enough
+  --> $DIR/higher-ranked-alias-norm-to-hr.rs:23:5
+   |
+LL |     needs_super(|_| {});
+   |     ^^^^^^^^^^^^^^^^^^^ implementation of `Fn` is not general enough
+   |
+   = note: closure with signature `fn(&'2 u32)` must implement `Fn<(&'1 u32,)>`, for any lifetime `'1`...
+   = note: ...but it actually implements `Fn<(&'2 u32,)>`, for some specific lifetime `'2`
+
+error: implementation of `Foo` is not general enough
+  --> $DIR/higher-ranked-alias-norm-to-hr.rs:23:5
+   |
+LL |     needs_super(|_| {});
+   |     ^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough
+   |
+   = note: `{closure@$DIR/higher-ranked-alias-norm-to-hr.rs:23:17: 23:20}` must implement `Foo<'0>`, for any lifetime `'0`...
+   = note: ...but it actually implements `Foo<'1>`, for some specific lifetime `'1`
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+error: implementation of `FnOnce` is not general enough
+  --> $DIR/higher-ranked-alias-norm-to-hr.rs:23:5
+   |
+LL |     needs_super(|_| {});
+   |     ^^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough
+   |
+   = note: closure with signature `fn(&'2 u32)` must implement `FnOnce<(&'1 u32,)>`, for any lifetime `'1`...
+   = note: ...but it actually implements `FnOnce<(&'2 u32,)>`, for some specific lifetime `'2`
+
+error: implementation of `Foo` is not general enough
+  --> $DIR/higher-ranked-alias-norm-to-hr.rs:23:5
+   |
+LL |     needs_super(|_| {});
+   |     ^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough
+   |
+   = note: `{closure@$DIR/higher-ranked-alias-norm-to-hr.rs:23:17: 23:20}` must implement `Foo<'0>`, for any lifetime `'0`...
+   = note: ...but it actually implements `Foo<'1>`, for some specific lifetime `'1`
+   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
+
+error: aborting due to 5 previous errors
+

tests/ui/closures/deduce-signature/higher-ranked-alias-norm-to-hr.next_ambig.stderr

@@ -0,0 +1,51 @@
+error[E0277]: expected a `Fn(&'a u32)` closure, found `{closure@$DIR/higher-ranked-alias-norm-to-hr.rs:23:17: 23:20}`
+  --> $DIR/higher-ranked-alias-norm-to-hr.rs:23:17
+   |
+LL |     needs_super(|_| {});
+   |     ----------- ^^^^^^ expected an `Fn(&'a u32)` closure, found `{closure@$DIR/higher-ranked-alias-norm-to-hr.rs:23:17: 23:20}`
+   |     |
+   |     required by a bound introduced by this call
+   |
+   = help: the trait `for<'a> Fn(&'a u32)` is not implemented for closure `{closure@$DIR/higher-ranked-alias-norm-to-hr.rs:23:17: 23:20}`
+   = note: expected a closure with signature `for<'a> fn(&'a _)`
+              found a closure with signature `fn(&_)`
+note: this is a known limitation of the trait solver that will be lifted in the future
+  --> $DIR/higher-ranked-alias-norm-to-hr.rs:23:17
+   |
+LL |     needs_super(|_| {});
+   |     ------------^^^----
+   |     |           |
+   |     |           the trait solver is unable to infer the generic types that should be inferred from this argument
+   |     add turbofish arguments to this call to specify the types manually, even if it's redundant
+note: required by a bound in `needs_super`
+  --> $DIR/higher-ranked-alias-norm-to-hr.rs:17:19
+   |
+LL | fn needs_super<F: for<'a> Fn(<F as Foo<'a>>::Input) + for<'a> Foo<'a>>(_: F) {}
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `needs_super`
+
+error[E0277]: the trait bound `for<'a> {closure@$DIR/higher-ranked-alias-norm-to-hr.rs:23:17: 23:20}: Foo<'a>` is not satisfied
+  --> $DIR/higher-ranked-alias-norm-to-hr.rs:23:17
+   |
+LL |     needs_super(|_| {});
+   |     ----------- ^^^^^^ unsatisfied trait bound
+   |     |
+   |     required by a bound introduced by this call
+   |
+   = help: the trait `for<'a> Foo<'a>` is not implemented for closure `{closure@$DIR/higher-ranked-alias-norm-to-hr.rs:23:17: 23:20}`
+note: this is a known limitation of the trait solver that will be lifted in the future
+  --> $DIR/higher-ranked-alias-norm-to-hr.rs:23:17
+   |
+LL |     needs_super(|_| {});
+   |     ------------^^^----
+   |     |           |
+   |     |           the trait solver is unable to infer the generic types that should be inferred from this argument
+   |     add turbofish arguments to this call to specify the types manually, even if it's redundant
+note: required by a bound in `needs_super`
+  --> $DIR/higher-ranked-alias-norm-to-hr.rs:17:55
+   |
+LL | fn needs_super<F: for<'a> Fn(<F as Foo<'a>>::Input) + for<'a> Foo<'a>>(_: F) {}
+   |                                                       ^^^^^^^^^^^^^^^ required by this bound in `needs_super`
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0277`.

tests/ui/closures/deduce-signature/higher-ranked-alias-norm-to-hr.rs

@@ -0,0 +1,31 @@
+//@ revisions: current_ok next_ok current_ambig next_ambig
+//@ ignore-compare-mode-next-solver (explicit revisions)
+//@[next_ok] compile-flags: -Znext-solver
+//@[next_ambig] compile-flags: -Znext-solver
+//@[current_ok] check-pass
+//@[next_ok] check-pass
+
+// Regression test for trait-system-refactor-initiative#191.
+trait Foo<'a> {
+    type Input;
+}
+
+impl<'a, F: Fn(&'a u32)> Foo<'a> for F {
+    type Input = &'a u32;
+}
+
+fn needs_super<F: for<'a> Fn(<F as Foo<'a>>::Input) + for<'a> Foo<'a>>(_: F) {}
+
+fn main() {
+    #[cfg(any(current_ok, next_ok))]
+    needs_super(|_: &u32| {});
+    #[cfg(any(current_ambig, next_ambig))]
+    needs_super(|_| {});
+    //[next_ambig]~^ ERROR expected a `Fn(&'a u32)` closure, found
+    //[next_ambig]~| ERROR the trait bound
+    //[current_ambig]~^^^ ERROR implementation of `Foo` is not general enough
+    //[current_ambig]~| ERROR implementation of `Fn` is not general enough
+    //[current_ambig]~| ERROR implementation of `Foo` is not general enough
+    //[current_ambig]~| ERROR implementation of `FnOnce` is not general enough
+    //[current_ambig]~| ERROR implementation of `Foo` is not general enough
+}