Rollup merge of #137000 - compiler-errors:deeply-normalize-item-bounds, r=lcnr

Deeply normalize item bounds in new solver

Built on #136863.

Fixes rust-lang/trait-system-refactor-initiative#142.
Fixes rust-lang/trait-system-refactor-initiative#151.

cc rust-lang/trait-system-refactor-initiative#116

First commit reworks candidate preference for projection bounds to prefer param-env projection clauses even if the corresponding trait ref doesn't come from the param-env.

Second commit adjusts the associated type item bounds check to deeply normalize in the new solver. This causes some test fallout which I will point out.

r? lcnr

Author: matthiaskrgr

compiler/rustc_hir_analysis/src/check/compare_impl_item.rs

@@ -2104,18 +2104,21 @@ pub(super) fn check_type_bounds<'tcx>(
         ObligationCause::new(impl_ty_span, impl_ty_def_id, code)
     };
 
-    let mut obligations: Vec<_> = tcx
-        .explicit_item_bounds(trait_ty.def_id)
-        .iter_instantiated_copied(tcx, rebased_args)
-        .map(|(concrete_ty_bound, span)| {
-            debug!(?concrete_ty_bound);
-            traits::Obligation::new(tcx, mk_cause(span), param_env, concrete_ty_bound)
-        })
-        .collect();
+    let mut obligations: Vec<_> = util::elaborate(
+        tcx,
+        tcx.explicit_item_bounds(trait_ty.def_id).iter_instantiated_copied(tcx, rebased_args).map(
+            |(concrete_ty_bound, span)| {
+                debug!(?concrete_ty_bound);
+                traits::Obligation::new(tcx, mk_cause(span), param_env, concrete_ty_bound)
+            },
+        ),
+    )
+    .collect();
 
     // Only in a const implementation do we need to check that the `~const` item bounds hold.
     if tcx.is_conditionally_const(impl_ty_def_id) {
-        obligations.extend(
+        obligations.extend(util::elaborate(
+            tcx,
             tcx.explicit_implied_const_bounds(trait_ty.def_id)
                 .iter_instantiated_copied(tcx, rebased_args)
                 .map(|(c, span)| {
@@ -2126,34 +2129,27 @@ pub(super) fn check_type_bounds<'tcx>(
                         c.to_host_effect_clause(tcx, ty::BoundConstness::Maybe),
                     )
                 }),
-        );
+        ));
     }
     debug!(item_bounds=?obligations);
 
     // Normalize predicates with the assumption that the GAT may always normalize
     // to its definition type. This should be the param-env we use to *prove* the
     // predicate too, but we don't do that because of performance issues.
     // See <https://github.com/rust-lang/rust/pull/117542#issue-1976337685>.
-    let trait_projection_ty = Ty::new_projection_from_args(tcx, trait_ty.def_id, rebased_args);
-    let impl_identity_ty = tcx.type_of(impl_ty.def_id).instantiate_identity();
     let normalize_param_env = param_env_with_gat_bounds(tcx, impl_ty, impl_trait_ref);
-    for mut obligation in util::elaborate(tcx, obligations) {
-        let normalized_predicate = if infcx.next_trait_solver() {
-            obligation.predicate.fold_with(&mut ReplaceTy {
-                tcx,
-                from: trait_projection_ty,
-                to: impl_identity_ty,
-            })
-        } else {
-            ocx.normalize(&normalize_cause, normalize_param_env, obligation.predicate)
-        };
-        debug!(?normalized_predicate);
-        obligation.predicate = normalized_predicate;
-
-        ocx.register_obligation(obligation);
+    for obligation in &mut obligations {
+        match ocx.deeply_normalize(&normalize_cause, normalize_param_env, obligation.predicate) {
+            Ok(pred) => obligation.predicate = pred,
+            Err(e) => {
+                return Err(infcx.err_ctxt().report_fulfillment_errors(e));
+            }
+        }
     }
+
     // Check that all obligations are satisfied by the implementation's
     // version.
+    ocx.register_obligations(obligations);
     let errors = ocx.select_all_or_error();
     if !errors.is_empty() {
         let reported = infcx.err_ctxt().report_fulfillment_errors(errors);
@@ -2165,22 +2161,6 @@ pub(super) fn check_type_bounds<'tcx>(
     ocx.resolve_regions_and_report_errors(impl_ty_def_id, param_env, assumed_wf_types)
 }
 
-struct ReplaceTy<'tcx> {
-    tcx: TyCtxt<'tcx>,
-    from: Ty<'tcx>,
-    to: Ty<'tcx>,
-}
-
-impl<'tcx> TypeFolder<TyCtxt<'tcx>> for ReplaceTy<'tcx> {
-    fn cx(&self) -> TyCtxt<'tcx> {
-        self.tcx
-    }
-
-    fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> {
-        if self.from == ty { self.to } else { ty.super_fold_with(self) }
-    }
-}
-
 /// Install projection predicates that allow GATs to project to their own
 /// definition types. This is not allowed in general in cases of default
 /// associated types in trait definitions, or when specialization is involved,

compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs

@@ -791,7 +791,7 @@ where
             return Ok(self.make_ambiguous_response_no_constraints(MaybeCause::Ambiguity));
         };
 
-        let responses: Vec<_> = match proven_via {
+        match proven_via {
             // Even when a trait bound has been proven using a where-bound, we
             // still need to consider alias-bounds for normalization, see
             // tests/ui/next-solver/alias-bound-shadowed-by-env.rs.
@@ -800,7 +800,7 @@ where
             // constness checking. Doing so is *at least theoretically* breaking,
             // see github.com/rust-lang/rust/issues/133044#issuecomment-2500709754
             TraitGoalProvenVia::ParamEnv | TraitGoalProvenVia::AliasBound => {
-                let mut candidates_from_env: Vec<_> = candidates
+                let mut candidates_from_env_and_bounds: Vec<_> = candidates
                     .iter()
                     .filter(|c| {
                         matches!(
@@ -813,16 +813,37 @@ where
 
                 // If the trait goal has been proven by using the environment, we want to treat
                 // aliases as rigid if there are no applicable projection bounds in the environment.
-                if candidates_from_env.is_empty() {
+                if candidates_from_env_and_bounds.is_empty() {
                     if let Ok(response) = inject_normalize_to_rigid_candidate(self) {
-                        candidates_from_env.push(response);
+                        candidates_from_env_and_bounds.push(response);
                     }
                 }
-                candidates_from_env
+
+                if let Some(response) = self.try_merge_responses(&candidates_from_env_and_bounds) {
+                    Ok(response)
+                } else {
+                    self.flounder(&candidates_from_env_and_bounds)
+                }
             }
-            TraitGoalProvenVia::Misc => candidates.iter().map(|c| c.result).collect(),
-        };
+            TraitGoalProvenVia::Misc => {
+                // Prefer "orphaned" param-env normalization predicates, which are used
+                // (for example, and ideally only) when proving item bounds for an impl.
+                let candidates_from_env: Vec<_> = candidates
+                    .iter()
+                    .filter(|c| matches!(c.source, CandidateSource::ParamEnv(_)))
+                    .map(|c| c.result)
+                    .collect();
+                if let Some(response) = self.try_merge_responses(&candidates_from_env) {
+                    return Ok(response);
+                }
 
-        self.try_merge_responses(&responses).map_or_else(|| self.flounder(&responses), Ok)
+                let responses: Vec<_> = candidates.iter().map(|c| c.result).collect();
+                if let Some(response) = self.try_merge_responses(&responses) {
+                    Ok(response)
+                } else {
+                    self.flounder(&responses)
+                }
+            }
+        }
     }
 }

tests/ui/generic-associated-types/issue-91883.stderr renamed to tests/ui/generic-associated-types/issue-91883.current.stderr

@@ -1,5 +1,5 @@
 error[E0478]: lifetime bound not satisfied
-  --> $DIR/issue-91883.rs:30:24
+  --> $DIR/issue-91883.rs:34:24
    |
 LL |     type Cursor<'tx>: Cursor<'tx>
    |     ----------------------------- definition of `Cursor` from trait
@@ -8,12 +8,12 @@ LL |     type Cursor<'tx> = CursorImpl<'tx>;
    |                        ^^^^^^^^^^^^^^^
    |
 note: lifetime parameter instantiated with the lifetime `'db` as defined here
-  --> $DIR/issue-91883.rs:29:6
+  --> $DIR/issue-91883.rs:33:6
    |
 LL | impl<'db> Transaction<'db> for TransactionImpl<'db> {
    |      ^^^
 note: but lifetime parameter must outlive the lifetime `'tx` as defined here
-  --> $DIR/issue-91883.rs:30:17
+  --> $DIR/issue-91883.rs:34:17
    |
 LL |     type Cursor<'tx> = CursorImpl<'tx>;
    |                 ^^^

tests/ui/generic-associated-types/issue-91883.next.stderr

@@ -0,0 +1,20 @@
+error[E0478]: lifetime bound not satisfied
+  --> $DIR/issue-91883.rs:34:24
+   |
+LL |     type Cursor<'tx> = CursorImpl<'tx>;
+   |                        ^^^^^^^^^^^^^^^
+   |
+note: lifetime parameter instantiated with the lifetime `'db` as defined here
+  --> $DIR/issue-91883.rs:33:6
+   |
+LL | impl<'db> Transaction<'db> for TransactionImpl<'db> {
+   |      ^^^
+note: but lifetime parameter must outlive the lifetime `'tx` as defined here
+  --> $DIR/issue-91883.rs:34:17
+   |
+LL |     type Cursor<'tx> = CursorImpl<'tx>;
+   |                 ^^^
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0478`.

tests/ui/generic-associated-types/issue-91883.rs

@@ -1,3 +1,7 @@
+//@ revisions: current next
+//@ ignore-compare-mode-next-solver (explicit revisions)
+//@[next] compile-flags: -Znext-solver
+
 use std::fmt::Debug;
 use std::marker::PhantomData;
 

tests/ui/impl-trait/in-trait/default-body-with-rpit.rs

@@ -1,5 +1,8 @@
 //@ edition:2021
 //@ check-pass
+//@ revisions: current next
+//@ ignore-compare-mode-next-solver (explicit revisions)
+//@[next] compile-flags: -Znext-solver
 
 use std::fmt::Debug;
 

tests/ui/impl-trait/in-trait/nested-rpitit-bounds.rs

@@ -1,4 +1,7 @@
 //@ check-pass
+//@ revisions: current next
+//@ ignore-compare-mode-next-solver (explicit revisions)
+//@[next] compile-flags: -Znext-solver
 
 use std::ops::Deref;
 

tests/ui/traits/const-traits/predicate-entailment-passes.rs

@@ -6,32 +6,21 @@
 #[const_trait] trait Bar {}
 impl const Bar for () {}
 
-
 #[const_trait] trait TildeConst {
-    type Bar<T> where T: ~const Bar;
-
     fn foo<T>() where T: ~const Bar;
 }
 impl TildeConst for () {
-    type Bar<T> = () where T: Bar;
-
     fn foo<T>() where T: Bar {}
 }
 
 
 #[const_trait] trait AlwaysConst {
-    type Bar<T> where T: const Bar;
-
     fn foo<T>() where T: const Bar;
 }
 impl AlwaysConst for i32 {
-    type Bar<T> = () where T: Bar;
-
     fn foo<T>() where T: Bar {}
 }
 impl const AlwaysConst for u32 {
-    type Bar<T> = () where T: ~const Bar;
-
     fn foo<T>() where T: ~const Bar {}
 }
 

tests/ui/traits/next-solver/coherence/trait_ref_is_knowable-norm-overflow.stderr

@@ -1,18 +1,8 @@
-error[E0275]: overflow evaluating the requirement `<T as Overflow>::Assoc: Sized`
+error[E0275]: overflow evaluating the requirement `<T as Overflow>::Assoc == _`
   --> $DIR/trait_ref_is_knowable-norm-overflow.rs:10:18
    |
 LL |     type Assoc = <T as Overflow>::Assoc;
    |                  ^^^^^^^^^^^^^^^^^^^^^^
-   |
-note: required by a bound in `Overflow::Assoc`
-  --> $DIR/trait_ref_is_knowable-norm-overflow.rs:7:5
-   |
-LL |     type Assoc;
-   |     ^^^^^^^^^^^ required by this bound in `Overflow::Assoc`
-help: consider relaxing the implicit `Sized` restriction
-   |
-LL |     type Assoc: ?Sized;
-   |               ++++++++
 
 error[E0119]: conflicting implementations of trait `Trait`
   --> $DIR/trait_ref_is_knowable-norm-overflow.rs:18:1

tests/ui/traits/next-solver/gat-wf.rs

@@ -0,0 +1,16 @@
+//@ compile-flags: -Znext-solver
+
+// Make sure that, like the old trait solver, we end up requiring that the WC of
+// impl GAT matches that of the trait. This is not a restriction that we *need*,
+// but is a side-effect of registering the where clauses when normalizing the GAT
+// when proving it satisfies its item bounds.
+
+trait Foo {
+    type T<'a>: Sized where Self: 'a;
+}
+
+impl Foo for &() {
+    type T<'a> = (); //~ the type `&()` does not fulfill the required lifetime
+}
+
+fn main() {}