move leak check out of candidate evaluation

this prevents higher ranked goals from guiding selection

Author: lcnr

compiler/rustc_trait_selection/src/traits/select/mod.rs

@@ -581,7 +581,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         obligation: &PredicateObligation<'tcx>,
     ) -> Result<EvaluationResult, OverflowError> {
         debug_assert!(!self.infcx.next_trait_solver());
-        self.evaluation_probe(|this| {
+        self.evaluation_probe(|this, _outer_universe| {
             let goal =
                 this.infcx.resolve_vars_if_possible((obligation.predicate, obligation.param_env));
             let mut result = this.evaluate_predicate_recursively(
@@ -597,13 +597,18 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         })
     }
 
+    /// Computes the evaluation result of `op`, discarding any constraints.
+    ///
+    /// This also runs for leak check to allow higher ranked region errors to impact
+    /// selection. By default it checks for leaks from all universes created inside of
+    /// `op`, but this can be overwritten if necessary.
     fn evaluation_probe(
         &mut self,
-        op: impl FnOnce(&mut Self) -> Result<EvaluationResult, OverflowError>,
+        op: impl FnOnce(&mut Self, &mut ty::UniverseIndex) -> Result<EvaluationResult, OverflowError>,
     ) -> Result<EvaluationResult, OverflowError> {
         self.infcx.probe(|snapshot| -> Result<EvaluationResult, OverflowError> {
-            let outer_universe = self.infcx.universe();
-            let result = op(self)?;
+            let mut outer_universe = self.infcx.universe();
+            let result = op(self, &mut outer_universe)?;
 
             match self.infcx.leak_check(outer_universe, Some(snapshot)) {
                 Ok(()) => {}
@@ -1267,9 +1272,19 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         stack: &TraitObligationStack<'o, 'tcx>,
         candidate: &SelectionCandidate<'tcx>,
     ) -> Result<EvaluationResult, OverflowError> {
-        let mut result = self.evaluation_probe(|this| {
-            let candidate = (*candidate).clone();
-            match this.confirm_candidate(stack.obligation, candidate) {
+        let mut result = self.evaluation_probe(|this, outer_universe| {
+            // We eagerly instantiate higher ranked goals to prevent universe errors
+            // from impacting candidate selection. This matches the behavior of the new
+            // solver. This slightly weakens type inference.
+            //
+            // In case there are no unresolved type or const variables this
+            // should still not be necessary to select a unique impl as any overlap
+            // relying on a universe error from higher ranked goals should have resulted
+            // in an overlap error in coherence.
+            let p = self.infcx.instantiate_binder_with_placeholders(stack.obligation.predicate);
+            let obligation = stack.obligation.with(this.tcx(), ty::Binder::dummy(p));
+            *outer_universe = self.infcx.universe();
+            match this.confirm_candidate(&obligation, candidate.clone()) {
                 Ok(selection) => {
                     debug!(?selection);
                     this.evaluate_predicates_recursively(
@@ -1706,8 +1721,14 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
         stack: &TraitObligationStack<'o, 'tcx>,
         where_clause_trait_ref: ty::PolyTraitRef<'tcx>,
     ) -> Result<EvaluationResult, OverflowError> {
-        self.evaluation_probe(|this| {
-            match this.match_where_clause_trait_ref(stack.obligation, where_clause_trait_ref) {
+        self.evaluation_probe(|this, outer_universe| {
+            // Eagerly instantiate higher ranked goals.
+            //
+            // See the comment in `evaluate_candidate` to see why.
+            let p = self.infcx.instantiate_binder_with_placeholders(stack.obligation.predicate);
+            let obligation = stack.obligation.with(this.tcx(), ty::Binder::dummy(p));
+            *outer_universe = self.infcx.universe();
+            match this.match_where_clause_trait_ref(&obligation, where_clause_trait_ref) {
                 Ok(obligations) => this.evaluate_predicates_recursively(stack.list(), obligations),
                 Err(()) => Ok(EvaluatedToErr),
             }

tests/ui/higher-ranked/trait-bounds/fn-ptr.classic.stderr

@@ -1,6 +1,6 @@
 // revisions: classic next
 //[next] compile-flags: -Znext-solver
-//[next] check-pass
+// check-pass
 
 fn ice()
 where
@@ -10,5 +10,4 @@ where
 
 fn main() {
     ice();
-    //[classic]~^ ERROR expected a `Fn(&'w ())` closure, found `fn(&'w ())`
 }

tests/ui/higher-ranked/trait-bounds/fn-ptr.rs

@@ -1,15 +1,7 @@
-// ignore-tidy-linelength
 // edition:2021
 // revisions: classic next
 //[next] compile-flags: -Znext-solver
-//[next] check-pass
-//[classic] known-bug: #112347
-//[classic] build-fail
-//[classic] failure-status: 101
-//[classic] normalize-stderr-test "note: .*\n\n" -> ""
-//[classic] normalize-stderr-test "thread 'rustc' panicked.*\n.*\n" -> ""
-//[classic] normalize-stderr-test "(error: internal compiler error: [^:]+):\d+:\d+: " -> "$1:LL:CC: "
-//[classic] rustc-env:RUST_BACKTRACE=0
+// check-pass
 
 #![feature(unboxed_closures)]
 

tests/ui/higher-ranked/trait-bounds/future.classic.stderr

@@ -0,0 +1,22 @@
+error[E0283]: type annotations needed
+  --> $DIR/hr-leak-check-does-not-guide-selection.rs:20:5
+   |
+LL |     impl_trait::<Box<_>>();
+   |     ^^^^^^^^^^^^^^^^^^^^ cannot infer type of the type parameter `T` declared on the function `impl_trait`
+   |
+note: multiple `impl`s satisfying `for<'a> Box<_>: Leak<'a>` found
+  --> $DIR/hr-leak-check-does-not-guide-selection.rs:14:1
+   |
+LL | impl<'a> Leak<'a> for Box<u32> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL | impl Leak<'static> for Box<u16> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+note: required by a bound in `impl_trait`
+  --> $DIR/hr-leak-check-does-not-guide-selection.rs:17:18
+   |
+LL | fn impl_trait<T: for<'a> Leak<'a>>() {}
+   |                  ^^^^^^^^^^^^^^^^ required by this bound in `impl_trait`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0283`.

tests/ui/higher-ranked/trait-bounds/future.rs

@@ -0,0 +1,22 @@
+error[E0283]: type annotations needed
+  --> $DIR/hr-leak-check-does-not-guide-selection.rs:20:5
+   |
+LL |     impl_trait::<Box<_>>();
+   |     ^^^^^^^^^^^^^^^^^^^^ cannot infer type of the type parameter `T` declared on the function `impl_trait`
+   |
+note: multiple `impl`s satisfying `for<'a> Box<_>: Leak<'a>` found
+  --> $DIR/hr-leak-check-does-not-guide-selection.rs:14:1
+   |
+LL | impl<'a> Leak<'a> for Box<u32> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL | impl Leak<'static> for Box<u16> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+note: required by a bound in `impl_trait`
+  --> $DIR/hr-leak-check-does-not-guide-selection.rs:17:18
+   |
+LL | fn impl_trait<T: for<'a> Leak<'a>>() {}
+   |                  ^^^^^^^^^^^^^^^^ required by this bound in `impl_trait`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0283`.

tests/ui/higher-ranked/trait-bounds/hr-leak-check-does-not-guide-selection.next.stderr

@@ -0,0 +1,22 @@
+// When proving the higher ranked goal `for<'a> Box<_>: Leak<'a>` the
+// impl for `Box<u16>` would result in a higher ranked region error.
+//
+// However, we instantiate higher ranked goals outside of candidate
+// selection, so we end up with ambiguity. The old solver previously
+// used the leak check to guide selection here.
+//
+// See trait-system-refactor-initiative#34 for more details here.
+
+// revisions: next old
+//[next] compile-flags: -Znext-solver
+
+trait Leak<'a> {}
+impl<'a> Leak<'a> for Box<u32> {}
+impl Leak<'static> for Box<u16> {}
+
+fn impl_trait<T: for<'a> Leak<'a>>() {}
+
+fn main() {
+    impl_trait::<Box<_>>();
+    //~^ ERROR type annotations needed
+}

tests/ui/higher-ranked/trait-bounds/hr-leak-check-does-not-guide-selection.old.stderr

@@ -1,23 +1,17 @@
-error[E0277]: the trait bound `for<'ccx> B: Bar<'ccx>` is not satisfied
-  --> $DIR/hrtb-higher-ranker-supertraits-transitive.rs:47:26
+error[E0308]: mismatched types
+  --> $DIR/hrtb-higher-ranker-supertraits-transitive.rs:47:5
    |
 LL |     want_bar_for_any_ccx(b);
-   |     -------------------- ^ the trait `for<'ccx> Bar<'ccx>` is not implemented for `B`
-   |     |
-   |     required by a bound introduced by this call
+   |     ^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
    |
-note: required by a bound in `want_bar_for_any_ccx`
+   = note: expected trait `for<'ccx> Bar<'ccx>`
+              found trait `Bar<'static>`
+note: the lifetime requirement is introduced here
   --> $DIR/hrtb-higher-ranker-supertraits-transitive.rs:32:15
    |
-LL | fn want_bar_for_any_ccx<B>(b: &B)
-   |    -------------------- required by a bound in this function
 LL |     where B : for<'ccx> Bar<'ccx>
-   |               ^^^^^^^^^^^^^^^^^^^ required by this bound in `want_bar_for_any_ccx`
-help: consider further restricting this bound
-   |
-LL |     where B : Qux + for<'ccx> Bar<'ccx>
-   |                   +++++++++++++++++++++
+   |               ^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to 1 previous error
 
-For more information about this error, try `rustc --explain E0277`.
+For more information about this error, try `rustc --explain E0308`.

tests/ui/higher-ranked/trait-bounds/hr-leak-check-does-not-guide-selection.rs

@@ -1,43 +1,37 @@
 // Test a trait (`Bar`) with a higher-ranked supertrait.
+#![allow(unconditional_recursion)]
 
-trait Foo<'tcx>
-{
+trait Foo<'tcx> {
     fn foo(&'tcx self) -> &'tcx isize;
 }
 
-trait Bar<'ccx>
-    : for<'tcx> Foo<'tcx>
-{
+trait Bar<'ccx>: for<'tcx> Foo<'tcx> {
     fn bar(&'ccx self) -> &'ccx isize;
 }
 
-fn want_foo_for_some_tcx<'x,F>(f: &'x F)
-    where F : Foo<'x>
-{
+fn want_foo_for_some_tcx<'x, F: Foo<'x>>(f: &'x F) {
     want_foo_for_some_tcx(f);
-    want_foo_for_any_tcx(f); //~ ERROR not satisfied
+    want_foo_for_any_tcx(f);
+    //~^ ERROR lifetime may not live long enough
+    //~| ERROR mismatched types
 }
 
-fn want_foo_for_any_tcx<F>(f: &F)
-    where F : for<'tcx> Foo<'tcx>
-{
+fn want_foo_for_any_tcx<F: for<'tcx> Foo<'tcx>>(f: &F) {
     want_foo_for_some_tcx(f);
     want_foo_for_any_tcx(f);
 }
 
-fn want_bar_for_some_ccx<'x,B>(b: &B)
-    where B : Bar<'x>
-{
+fn want_bar_for_some_ccx<'x, B: Bar<'x>>(b: &B) {
     want_foo_for_some_tcx(b);
     want_foo_for_any_tcx(b);
 
     want_bar_for_some_ccx(b);
-    want_bar_for_any_ccx(b); //~ ERROR not satisfied
+    want_bar_for_any_ccx(b);
+    //~^ ERROR lifetime may not live long enough
+    //~| ERROR mismatched types
 }
 
-fn want_bar_for_any_ccx<B>(b: &B)
-    where B : for<'ccx> Bar<'ccx>
-{
+fn want_bar_for_any_ccx<B: for<'ccx> Bar<'ccx>>(b: &B) {
     want_foo_for_some_tcx(b);
     want_foo_for_any_tcx(b);