Misc cleanups to borrowck crate

Author: BoxyUwU

compiler/rustc_borrowck/src/type_check/canonical.rs

@@ -343,8 +343,15 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
             return;
         }
 
-        // FIXME: Ideally MIR types are normalized, but this is not always true.
-        let mir_ty = self.normalize(mir_ty, Locations::All(span));
+        // This is a hack. `body.local_decls` are not necessarily normalized in the old
+        // solver due to not deeply normalizing in writeback. So we must re-normalize here.
+        //
+        // I am not sure of a test case where this actually matters. There is a similar
+        // hack in `equate_inputs_and_outputs` which does have associated test cases.
+        let mir_ty = match self.infcx.next_trait_solver() {
+            true => mir_ty,
+            false => self.normalize(mir_ty, Locations::All(span)),
+        };
 
         let cause = ObligationCause::dummy_with_span(span);
         let param_env = self.infcx.param_env;
@@ -353,6 +360,10 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
             ConstraintCategory::Boring,
             type_op::custom::CustomTypeOp::new(
                 |ocx| {
+                    // The `AscribeUserType` query would normally emit a wf
+                    // obligation for the unnormalized user_ty here. This is
+                    // where the "incorrectly skips the WF checks we normally do"
+                    // happens
                     let user_ty = ocx.normalize(&cause, param_env, user_ty);
                     ocx.eq(&cause, param_env, user_ty, mir_ty)?;
                     Ok(())

compiler/rustc_borrowck/src/type_check/input_output.rs

@@ -126,6 +126,31 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
         );
     }
 
+    //  FIXME(BoxyUwU): This should probably be part of a larger borrowck dev-guide chapter
+    //
+    /// Enforce that the types of the locals corresponding to the inputs and output of
+    /// the body are equal to those of the (normalized) signature.
+    ///
+    /// This is necessary for two reasons:
+    /// - Locals in the MIR all start out with `'erased` regions and then are replaced
+    ///    with unconstrained nll vars. If we have a function returning `&'a u32` then
+    ///    the local `_0: &'?10 u32` needs to have its region var equated with the nll
+    ///    var representing `'a`. i.e. borrow check must uphold that `'?10 = 'a`.
+    /// - When computing the normalized signature we may introduce new unconstrained nll
+    ///    vars due to higher ranked where clauses ([#136547]). We then wind up with implied
+    ///    bounds involving these vars.
+    ///
+    ///    For this reason it is important that we equate with the *normalized* signature
+    ///    which was produced when computing implied bounds. If we do not do so then we will
+    ///    wind up with implied bounds on nll vars which cannot actually be used as the nll
+    ///    var never gets related to anything.
+    ///
+    /// For 'closure-like' bodies this function effectively relates the *inferred* signature
+    /// of the closure against the locals corresponding to the closure's inputs/output. It *does
+    /// not* relate the user provided types for the signature to the locals, this is handled
+    /// separately by: [`TypeChecker::check_signature_annotation`].
+    ///
+    /// [#136547]: <https://www.github.com/rust-lang/rust/issues/136547>
     #[instrument(skip(self), level = "debug")]
     pub(super) fn equate_inputs_and_outputs(&mut self, normalized_inputs_and_output: &[Ty<'tcx>]) {
         let (&normalized_output_ty, normalized_input_tys) =
@@ -173,38 +198,43 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
             );
         }
 
-        // Return types are a bit more complex. They may contain opaque `impl Trait` types.
-        let mir_output_ty = self.body.local_decls[RETURN_PLACE].ty;
+        // Equate expected output ty with the type of the RETURN_PLACE in MIR
+        let mir_output_ty = self.body.return_ty();
         let output_span = self.body.local_decls[RETURN_PLACE].source_info.span;
         self.equate_normalized_input_or_output(normalized_output_ty, mir_output_ty, output_span);
     }
 
     #[instrument(skip(self), level = "debug")]
     fn equate_normalized_input_or_output(&mut self, a: Ty<'tcx>, b: Ty<'tcx>, span: Span) {
+        if self.infcx.next_trait_solver() {
+            self.eq_types(a, b, Locations::All(span), ConstraintCategory::BoringNoLocation)
+                .unwrap_or_else(|terr| {
+                    span_mirbug!(
+                        self,
+                        Location::START,
+                        "equate_normalized_input_or_output: `{a:?}=={b:?}` failed with `{terr:?}`",
+                    );
+                });
+        }
+
+        // This is a hack. `body.local_decls` are not necessarily normalized in the old
+        // solver due to not deeply normalizing in writeback. So we must re-normalize here.
+        //
+        // However, in most cases normalizing is unnecessary so we only do so if it may be
+        // necessary for type equality to hold. This leads to some (very minor) performance
+        // wins.
         if let Err(_) =
             self.eq_types(a, b, Locations::All(span), ConstraintCategory::BoringNoLocation)
         {
-            // FIXME(jackh726): This is a hack. It's somewhat like
-            // `rustc_traits::normalize_after_erasing_regions`. Ideally, we'd
-            // like to normalize *before* inserting into `local_decls`, but
-            // doing so ends up causing some other trouble.
             let b = self.normalize(b, Locations::All(span));
-
-            // Note: if we have to introduce new placeholders during normalization above, then we
-            // won't have added those universes to the universe info, which we would want in
-            // `relate_tys`.
-            if let Err(terr) =
-                self.eq_types(a, b, Locations::All(span), ConstraintCategory::BoringNoLocation)
-            {
-                span_mirbug!(
-                    self,
-                    Location::START,
-                    "equate_normalized_input_or_output: `{:?}=={:?}` failed with `{:?}`",
-                    a,
-                    b,
-                    terr
-                );
-            }
-        }
+            self.eq_types(a, b, Locations::All(span), ConstraintCategory::BoringNoLocation)
+                .unwrap_or_else(|terr| {
+                    span_mirbug!(
+                        self,
+                        Location::START,
+                        "equate_normalized_input_or_output: `{a:?}=={b:?}` failed with `{terr:?}`",
+                    );
+                });
+        };
     }
 }

compiler/rustc_borrowck/src/type_check/mod.rs

@@ -123,16 +123,19 @@ pub(crate) fn type_check<'tcx>(
         known_type_outlives_obligations,
     } = free_region_relations::create(infcx, universal_regions, &mut constraints);
 
-    let pre_obligations = infcx.take_registered_region_obligations();
-    assert!(
-        pre_obligations.is_empty(),
-        "there should be no incoming region obligations = {pre_obligations:#?}",
-    );
-    let pre_assumptions = infcx.take_registered_region_assumptions();
-    assert!(
-        pre_assumptions.is_empty(),
-        "there should be no incoming region assumptions = {pre_assumptions:#?}",
-    );
+    {
+        // Scope these variables so it's clear they're not used later
+        let pre_obligations = infcx.take_registered_region_obligations();
+        assert!(
+            pre_obligations.is_empty(),
+            "there should be no incoming region obligations = {pre_obligations:#?}",
+        );
+        let pre_assumptions = infcx.take_registered_region_assumptions();
+        assert!(
+            pre_assumptions.is_empty(),
+            "there should be no incoming region assumptions = {pre_assumptions:#?}",
+        );
+    }
 
     debug!(?normalized_inputs_and_output);
 

compiler/rustc_middle/src/ty/util.rs

@@ -609,9 +609,8 @@ impl<'tcx> TyCtxt<'tcx> {
     /// have the same `DefKind`.
     ///
     /// Note that closures have a `DefId`, but the closure *expression* also has a
-    /// `HirId` that is located within the context where the closure appears (and, sadly,
-    /// a corresponding `NodeId`, since those are not yet phased out). The parent of
-    /// the closure's `DefId` will also be the context where it appears.
+    /// `HirId` that is located within the context where the closure appears. The
+    /// parent of the closure's `DefId` will also be the context where it appears.
     pub fn is_closure_like(self, def_id: DefId) -> bool {
         matches!(self.def_kind(def_id), DefKind::Closure)
     }

tests/ui/associated-types/normalization-generality-2.rs

@@ -1,4 +1,7 @@
 //@ build-pass
+//@ revisions: current next
+//@ ignore-compare-mode-next-solver (explicit revisions)
+//@[next] compile-flags: -Znext-solver
 
 // Ensures that we don't regress on "implementation is not general enough" when
 // normalizating under binders. Unlike `normalization-generality.rs`, this also produces

tests/ui/associated-types/normalization-generality.rs

@@ -1,4 +1,7 @@
 //@ build-pass
+//@ revisions: current next
+//@ ignore-compare-mode-next-solver (explicit revisions)
+//@[next] compile-flags: -Znext-solver
 
 // Ensures that we don't regress on "implementation is not general enough" when
 // normalizating under binders.

tests/ui/higher-ranked/trait-bounds/issue-88446.rs

@@ -1,4 +1,7 @@
 //@ check-pass
+//@ revisions: current next
+//@ ignore-compare-mode-next-solver (explicit revisions)
+//@[next] compile-flags: -Znext-solver
 
 trait Yokeable<'a> {
     type Output: 'a;

tests/ui/higher-ranked/trait-bounds/normalize-under-binder/issue-89436.rs

@@ -1,4 +1,7 @@
 //@ check-pass
+//@ revisions: current next
+//@ ignore-compare-mode-next-solver (explicit revisions)
+//@[next] compile-flags: -Znext-solver
 
 #![allow(unused)]
 

tests/ui/higher-ranked/trait-bounds/normalize-under-binder/issue-90638.rs

@@ -1,4 +1,7 @@
 //@check-pass
+//@ revisions: current next
+//@ ignore-compare-mode-next-solver (explicit revisions)
+//@[next] compile-flags: -Znext-solver
 
 trait Yokeable<'a>: 'static {
     type Output: 'a;

tests/ui/nll/issue-112604-closure-output-normalize.rs

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