From df1c1afdafcf8c9c46b18f274fc7f01f13ff05da Mon Sep 17 00:00:00 2001
From: Michael Goulet <michael@errs.io>
Date: Wed, 31 May 2023 17:45:45 +0000
Subject: [PATCH] Check that RPITs are compatible with the opaques inferred
 during HIR typeck too

---
 .../src/collect/type_of/opaque.rs             | 65 +++++++++--------
 .../rustc_middle/src/ty/typeck_results.rs     |  4 --
 tests/ui/chalkify/bugs/async.stderr           | 13 +---
 tests/ui/dyn-star/param-env-infer.next.stderr | 35 ----------
 tests/ui/impl-trait/auto-trait-leak.stderr    | 70 -------------------
 .../multiple-defining-usages-in-body.rs       |  2 +-
 .../multiple-defining-usages-in-body.stderr   | 12 ++--
 7 files changed, 46 insertions(+), 155 deletions(-)

diff --git a/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs b/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs
index da82ce1d523..4d96a7ff4c3 100644
--- a/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs
+++ b/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs
@@ -1,3 +1,4 @@
+use rustc_errors::StashKey;
 use rustc_hir::def_id::LocalDefId;
 use rustc_hir::intravisit::{self, Visitor};
 use rustc_hir::{self as hir, Expr, ImplItem, Item, Node, TraitItem};
@@ -210,12 +211,40 @@ pub(super) fn find_opaque_ty_constraints_for_rpit<'tcx>(
     def_id: LocalDefId,
     owner_def_id: LocalDefId,
 ) -> Ty<'_> {
-    let concrete = tcx.mir_borrowck(owner_def_id).concrete_opaque_types.get(&def_id).copied();
+    let tables = tcx.typeck(owner_def_id);
 
-    if let Some(concrete) = concrete {
+    // Check that all of the opaques we inferred during HIR are compatible.
+    // FIXME: We explicitly don't check that the types inferred during HIR
+    // typeck are compatible with the one that we infer during borrowck,
+    // because that one actually sometimes has consts evaluated eagerly so
+    // using strict type equality will fail.
+    let mut hir_opaque_ty: Option<ty::OpaqueHiddenType<'tcx>> = None;
+    if tables.tainted_by_errors.is_none() {
+        for (&opaque_type_key, &hidden_type) in &tables.concrete_opaque_types {
+            if opaque_type_key.def_id != def_id {
+                continue;
+            }
+            let concrete_type = tcx.erase_regions(
+                hidden_type.remap_generic_params_to_declaration_params(opaque_type_key, tcx, true),
+            );
+            if let Some(prev) = &mut hir_opaque_ty {
+                if concrete_type.ty != prev.ty && !(concrete_type, prev.ty).references_error() {
+                    prev.report_mismatch(&concrete_type, def_id, tcx).stash(
+                        tcx.def_span(opaque_type_key.def_id),
+                        StashKey::OpaqueHiddenTypeMismatch,
+                    );
+                }
+            } else {
+                hir_opaque_ty = Some(concrete_type);
+            }
+        }
+    }
+
+    let mir_opaque_ty = tcx.mir_borrowck(owner_def_id).concrete_opaque_types.get(&def_id).copied();
+    if let Some(mir_opaque_ty) = mir_opaque_ty {
         let scope = tcx.hir().local_def_id_to_hir_id(owner_def_id);
         debug!(?scope);
-        let mut locator = RpitConstraintChecker { def_id, tcx, found: concrete };
+        let mut locator = RpitConstraintChecker { def_id, tcx, found: mir_opaque_ty };
 
         match tcx.hir().get(scope) {
             Node::Item(it) => intravisit::walk_item(&mut locator, it),
@@ -224,38 +253,16 @@ pub(super) fn find_opaque_ty_constraints_for_rpit<'tcx>(
             other => bug!("{:?} is not a valid scope for an opaque type item", other),
         }
 
-        concrete.ty
+        mir_opaque_ty.ty
     } else {
-        let tables = tcx.typeck(owner_def_id);
         if let Some(guar) = tables.tainted_by_errors {
-            // Some error in the
-            // owner fn prevented us from populating
+            // Some error in the owner fn prevented us from populating
             // the `concrete_opaque_types` table.
             tcx.ty_error(guar)
         } else {
             // Fall back to the RPIT we inferred during HIR typeck
-            let mut opaque_ty: Option<ty::OpaqueHiddenType<'tcx>> = None;
-            for (&opaque_type_key, &hidden_type) in &tables.concrete_opaque_types {
-                if opaque_type_key.def_id != def_id {
-                    continue;
-                }
-                let concrete_type =
-                    tcx.erase_regions(hidden_type.remap_generic_params_to_declaration_params(
-                        opaque_type_key,
-                        tcx,
-                        true,
-                    ));
-                if let Some(prev) = &mut opaque_ty {
-                    if concrete_type.ty != prev.ty && !(concrete_type, prev.ty).references_error() {
-                        prev.report_mismatch(&concrete_type, def_id, tcx).emit();
-                    }
-                } else {
-                    opaque_ty = Some(concrete_type);
-                }
-            }
-
-            if let Some(opaque_ty) = opaque_ty {
-                opaque_ty.ty
+            if let Some(hir_opaque_ty) = hir_opaque_ty {
+                hir_opaque_ty.ty
             } else {
                 // We failed to resolve the opaque type or it
                 // resolves to itself. We interpret this as the
diff --git a/compiler/rustc_middle/src/ty/typeck_results.rs b/compiler/rustc_middle/src/ty/typeck_results.rs
index 68bae5828c5..8cbffa14850 100644
--- a/compiler/rustc_middle/src/ty/typeck_results.rs
+++ b/compiler/rustc_middle/src/ty/typeck_results.rs
@@ -155,10 +155,6 @@ pub struct TypeckResults<'tcx> {
     /// We also store the type here, so that the compiler can use it as a hint
     /// for figuring out hidden types, even if they are only set in dead code
     /// (which doesn't show up in MIR).
-    ///
-    /// These types are mapped back to the opaque's identity substitutions
-    /// (with erased regions), which is why we don't associated substs with any
-    /// of these usages.
     pub concrete_opaque_types: FxIndexMap<ty::OpaqueTypeKey<'tcx>, ty::OpaqueHiddenType<'tcx>>,
 
     /// Tracks the minimum captures required for a closure;
diff --git a/tests/ui/chalkify/bugs/async.stderr b/tests/ui/chalkify/bugs/async.stderr
index 7e64e67f24c..e6d46b02706 100644
--- a/tests/ui/chalkify/bugs/async.stderr
+++ b/tests/ui/chalkify/bugs/async.stderr
@@ -13,16 +13,9 @@ error: internal compiler error: projection clauses should be implied from elsewh
 LL | async fn foo(x: u32) -> u32 {
    |                         ^^^query stack during panic:
 #0 [typeck] type-checking `foo`
-#1 [thir_body] building THIR for `foo`
-#2 [check_match] match-checking `foo`
-#3 [mir_built] building MIR for `foo`
-#4 [unsafety_check_result] unsafety-checking `foo`
-#5 [mir_const] preparing `foo` for borrow checking
-#6 [mir_promoted] promoting constants in MIR for `foo`
-#7 [mir_borrowck] borrow-checking `foo`
-#8 [type_of] computing type of `foo::{opaque#0}`
-#9 [check_mod_item_types] checking item types in top-level module
-#10 [analysis] running analysis passes on this crate
+#1 [type_of] computing type of `foo::{opaque#0}`
+#2 [check_mod_item_types] checking item types in top-level module
+#3 [analysis] running analysis passes on this crate
 end of query stack
 error: aborting due to 2 previous errors
 
diff --git a/tests/ui/dyn-star/param-env-infer.next.stderr b/tests/ui/dyn-star/param-env-infer.next.stderr
index 64d76bb04b1..408abecc30d 100644
--- a/tests/ui/dyn-star/param-env-infer.next.stderr
+++ b/tests/ui/dyn-star/param-env-infer.next.stderr
@@ -13,41 +13,6 @@ error[E0391]: cycle detected when computing type of `make_dyn_star::{opaque#0}`
 LL | fn make_dyn_star<'a, T: PointerLike + Debug + 'a>(t: T) -> impl PointerLike + Debug + 'a {
    |                                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
-note: ...which requires borrow-checking `make_dyn_star`...
-  --> $DIR/param-env-infer.rs:11:1
-   |
-LL | fn make_dyn_star<'a, T: PointerLike + Debug + 'a>(t: T) -> impl PointerLike + Debug + 'a {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-note: ...which requires promoting constants in MIR for `make_dyn_star`...
-  --> $DIR/param-env-infer.rs:11:1
-   |
-LL | fn make_dyn_star<'a, T: PointerLike + Debug + 'a>(t: T) -> impl PointerLike + Debug + 'a {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-note: ...which requires preparing `make_dyn_star` for borrow checking...
-  --> $DIR/param-env-infer.rs:11:1
-   |
-LL | fn make_dyn_star<'a, T: PointerLike + Debug + 'a>(t: T) -> impl PointerLike + Debug + 'a {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-note: ...which requires unsafety-checking `make_dyn_star`...
-  --> $DIR/param-env-infer.rs:11:1
-   |
-LL | fn make_dyn_star<'a, T: PointerLike + Debug + 'a>(t: T) -> impl PointerLike + Debug + 'a {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-note: ...which requires building MIR for `make_dyn_star`...
-  --> $DIR/param-env-infer.rs:11:1
-   |
-LL | fn make_dyn_star<'a, T: PointerLike + Debug + 'a>(t: T) -> impl PointerLike + Debug + 'a {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-note: ...which requires match-checking `make_dyn_star`...
-  --> $DIR/param-env-infer.rs:11:1
-   |
-LL | fn make_dyn_star<'a, T: PointerLike + Debug + 'a>(t: T) -> impl PointerLike + Debug + 'a {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-note: ...which requires building THIR for `make_dyn_star`...
-  --> $DIR/param-env-infer.rs:11:1
-   |
-LL | fn make_dyn_star<'a, T: PointerLike + Debug + 'a>(t: T) -> impl PointerLike + Debug + 'a {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 note: ...which requires type-checking `make_dyn_star`...
   --> $DIR/param-env-infer.rs:11:1
    |
diff --git a/tests/ui/impl-trait/auto-trait-leak.stderr b/tests/ui/impl-trait/auto-trait-leak.stderr
index aa4ee75bb75..c0c4cd5013e 100644
--- a/tests/ui/impl-trait/auto-trait-leak.stderr
+++ b/tests/ui/impl-trait/auto-trait-leak.stderr
@@ -4,41 +4,6 @@ error[E0391]: cycle detected when computing type of `cycle1::{opaque#0}`
 LL | fn cycle1() -> impl Clone {
    |                ^^^^^^^^^^
    |
-note: ...which requires borrow-checking `cycle1`...
-  --> $DIR/auto-trait-leak.rs:12:1
-   |
-LL | fn cycle1() -> impl Clone {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^
-note: ...which requires promoting constants in MIR for `cycle1`...
-  --> $DIR/auto-trait-leak.rs:12:1
-   |
-LL | fn cycle1() -> impl Clone {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^
-note: ...which requires preparing `cycle1` for borrow checking...
-  --> $DIR/auto-trait-leak.rs:12:1
-   |
-LL | fn cycle1() -> impl Clone {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^
-note: ...which requires unsafety-checking `cycle1`...
-  --> $DIR/auto-trait-leak.rs:12:1
-   |
-LL | fn cycle1() -> impl Clone {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^
-note: ...which requires building MIR for `cycle1`...
-  --> $DIR/auto-trait-leak.rs:12:1
-   |
-LL | fn cycle1() -> impl Clone {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^
-note: ...which requires match-checking `cycle1`...
-  --> $DIR/auto-trait-leak.rs:12:1
-   |
-LL | fn cycle1() -> impl Clone {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^
-note: ...which requires building THIR for `cycle1`...
-  --> $DIR/auto-trait-leak.rs:12:1
-   |
-LL | fn cycle1() -> impl Clone {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^
 note: ...which requires type-checking `cycle1`...
   --> $DIR/auto-trait-leak.rs:14:5
    |
@@ -50,41 +15,6 @@ note: ...which requires computing type of `cycle2::{opaque#0}`...
    |
 LL | fn cycle2() -> impl Clone {
    |                ^^^^^^^^^^
-note: ...which requires borrow-checking `cycle2`...
-  --> $DIR/auto-trait-leak.rs:19:1
-   |
-LL | fn cycle2() -> impl Clone {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^
-note: ...which requires promoting constants in MIR for `cycle2`...
-  --> $DIR/auto-trait-leak.rs:19:1
-   |
-LL | fn cycle2() -> impl Clone {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^
-note: ...which requires preparing `cycle2` for borrow checking...
-  --> $DIR/auto-trait-leak.rs:19:1
-   |
-LL | fn cycle2() -> impl Clone {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^
-note: ...which requires unsafety-checking `cycle2`...
-  --> $DIR/auto-trait-leak.rs:19:1
-   |
-LL | fn cycle2() -> impl Clone {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^
-note: ...which requires building MIR for `cycle2`...
-  --> $DIR/auto-trait-leak.rs:19:1
-   |
-LL | fn cycle2() -> impl Clone {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^
-note: ...which requires match-checking `cycle2`...
-  --> $DIR/auto-trait-leak.rs:19:1
-   |
-LL | fn cycle2() -> impl Clone {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^
-note: ...which requires building THIR for `cycle2`...
-  --> $DIR/auto-trait-leak.rs:19:1
-   |
-LL | fn cycle2() -> impl Clone {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^
 note: ...which requires type-checking `cycle2`...
   --> $DIR/auto-trait-leak.rs:20:5
    |
diff --git a/tests/ui/impl-trait/multiple-defining-usages-in-body.rs b/tests/ui/impl-trait/multiple-defining-usages-in-body.rs
index c3a6f09f86d..86661153a0d 100644
--- a/tests/ui/impl-trait/multiple-defining-usages-in-body.rs
+++ b/tests/ui/impl-trait/multiple-defining-usages-in-body.rs
@@ -4,9 +4,9 @@ impl Trait for () {}
 fn foo<T: Trait, U: Trait>() -> impl Trait {
     //~^ WARN function cannot return without recursing [unconditional_recursion]
     let a: T = foo::<T, U>();
-    //~^ ERROR concrete type differs from previous defining opaque type use
     loop {}
     let _: T = foo::<U, T>();
+    //~^ ERROR concrete type differs from previous defining opaque type use
 }
 
 fn main() {}
diff --git a/tests/ui/impl-trait/multiple-defining-usages-in-body.stderr b/tests/ui/impl-trait/multiple-defining-usages-in-body.stderr
index 06991749bfa..f3c090408b4 100644
--- a/tests/ui/impl-trait/multiple-defining-usages-in-body.stderr
+++ b/tests/ui/impl-trait/multiple-defining-usages-in-body.stderr
@@ -11,15 +11,15 @@ LL |     let a: T = foo::<T, U>();
    = note: `#[warn(unconditional_recursion)]` on by default
 
 error: concrete type differs from previous defining opaque type use
+  --> $DIR/multiple-defining-usages-in-body.rs:8:16
+   |
+LL |     let _: T = foo::<U, T>();
+   |                ^^^^^^^^^^^^^ expected `T`, got `U`
+   |
+note: previous use here
   --> $DIR/multiple-defining-usages-in-body.rs:6:16
    |
 LL |     let a: T = foo::<T, U>();
-   |                ^^^^^^^^^^^^^ expected `U`, got `T`
-   |
-note: previous use here
-  --> $DIR/multiple-defining-usages-in-body.rs:9:16
-   |
-LL |     let _: T = foo::<U, T>();
    |                ^^^^^^^^^^^^^
 
 error: aborting due to previous error; 1 warning emitted