Rollup merge of rust-lang#99079 - compiler-errors:issue-99073, r=oli-obk

Check that RPITs constrained by a recursive call in a closure are compatible

Fixes rust-lang#99073

Adapts a similar visitor pattern to `find_opaque_ty_constraints` (that we use to check TAITs), but with some changes:
0. Only walk the "OnlyBody" children, instead of all items in the RPIT's defining scope
1. Only walk through the body's children if we found a constraining usage
2. Don't actually do any inference, just do a comparison and error if they're mismatched

----

r? `@oli-obk` -- you know all this impl-trait stuff best... is this the right approach? I can explain the underlying issue better if you'd like, in case that might reveal a better solution. Not sure if it's possible to gather up the closure's defining usages of the RPIT while borrowck'ing the outer function, that might be a better place to put this check...

Author: JohnTitor

compiler/rustc_borrowck/src/type_check/mod.rs

@@ -21,7 +21,6 @@ use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKi
 use rustc_infer::infer::{
     InferCtxt, InferOk, LateBoundRegion, LateBoundRegionConversionTime, NllRegionVariableOrigin,
 };
-use rustc_infer::traits::ObligationCause;
 use rustc_middle::mir::tcx::PlaceTy;
 use rustc_middle::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor};
 use rustc_middle::mir::AssertKind;
@@ -225,26 +224,6 @@ pub(crate) fn type_check<'mir, 'tcx>(
                     )
                     .unwrap();
                     let mut hidden_type = infcx.resolve_vars_if_possible(decl.hidden_type);
-                    // Check that RPITs are only constrained in their outermost
-                    // function, otherwise report a mismatched types error.
-                    if let OpaqueTyOrigin::FnReturn(parent) | OpaqueTyOrigin::AsyncFn(parent)
-                            = infcx.opaque_ty_origin_unchecked(opaque_type_key.def_id, hidden_type.span)
-                        && parent.to_def_id() != body.source.def_id()
-                    {
-                        infcx
-                            .report_mismatched_types(
-                                &ObligationCause::misc(
-                                    hidden_type.span,
-                                    infcx.tcx.hir().local_def_id_to_hir_id(
-                                        body.source.def_id().expect_local(),
-                                    ),
-                                ),
-                                infcx.tcx.mk_opaque(opaque_type_key.def_id.to_def_id(), opaque_type_key.substs),
-                                hidden_type.ty,
-                                ty::error::TypeError::Mismatch,
-                            )
-                            .emit();
-                    }
                     trace!(
                         "finalized opaque type {:?} to {:#?}",
                         opaque_type_key,

compiler/rustc_infer/src/infer/opaque_types.rs

@@ -430,7 +430,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
     }
 
     #[instrument(skip(self), level = "trace")]
-    pub fn opaque_ty_origin_unchecked(&self, def_id: LocalDefId, span: Span) -> OpaqueTyOrigin {
+    fn opaque_ty_origin_unchecked(&self, def_id: LocalDefId, span: Span) -> OpaqueTyOrigin {
         let origin = match self.tcx.hir().expect_item(def_id).kind {
             hir::ItemKind::OpaqueTy(hir::OpaqueTy { origin, .. }) => origin,
             ref itemkind => {

compiler/rustc_typeck/src/collect/type_of.rs

@@ -335,37 +335,11 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> {
                     tcx.mk_adt(def, substs)
                 }
                 ItemKind::OpaqueTy(OpaqueTy { origin: hir::OpaqueTyOrigin::TyAlias, .. }) => {
-                    find_opaque_ty_constraints(tcx, def_id)
+                    find_opaque_ty_constraints_for_tait(tcx, def_id)
                 }
                 // Opaque types desugared from `impl Trait`.
                 ItemKind::OpaqueTy(OpaqueTy { origin: hir::OpaqueTyOrigin::FnReturn(owner) | hir::OpaqueTyOrigin::AsyncFn(owner), .. }) => {
-                    let concrete_ty = tcx
-                        .mir_borrowck(owner)
-                        .concrete_opaque_types
-                        .get(&def_id)
-                        .copied()
-                        .map(|concrete| concrete.ty)
-                        .unwrap_or_else(|| {
-                            let table = tcx.typeck(owner);
-                            if let Some(_) = table.tainted_by_errors {
-                                // Some error in the
-                                // owner fn prevented us from populating
-                                // the `concrete_opaque_types` table.
-                                tcx.ty_error()
-                            } else {
-                                table.concrete_opaque_types.get(&def_id).copied().unwrap_or_else(|| {
-                                    // We failed to resolve the opaque type or it
-                                    // resolves to itself. We interpret this as the
-                                    // no values of the hidden type ever being constructed,
-                                    // so we can just make the hidden type be `!`.
-                                    // For backwards compatibility reasons, we fall back to
-                                    // `()` until we the diverging default is changed.
-                                    Some(tcx.mk_diverging_default())
-                                }).expect("RPIT always have a hidden type from typeck")
-                            }
-                        });
-                    debug!("concrete_ty = {:?}", concrete_ty);
-                    concrete_ty
+                    find_opaque_ty_constraints_for_rpit(tcx, def_id, owner)
                 }
                 ItemKind::Trait(..)
                 | ItemKind::TraitAlias(..)
@@ -519,7 +493,7 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> {
 /// fn b<T>() -> Foo<T, u32> { .. }
 /// ```
 ///
-fn find_opaque_ty_constraints(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Ty<'_> {
+fn find_opaque_ty_constraints_for_tait(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Ty<'_> {
     use rustc_hir::{Expr, ImplItem, Item, TraitItem};
 
     struct ConstraintLocator<'tcx> {
@@ -660,6 +634,122 @@ fn find_opaque_ty_constraints(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Ty<'_> {
     }
 }
 
+fn find_opaque_ty_constraints_for_rpit(
+    tcx: TyCtxt<'_>,
+    def_id: LocalDefId,
+    owner_def_id: LocalDefId,
+) -> Ty<'_> {
+    use rustc_hir::{Expr, ImplItem, Item, TraitItem};
+
+    struct ConstraintChecker<'tcx> {
+        tcx: TyCtxt<'tcx>,
+
+        /// def_id of the opaque type whose defining uses are being checked
+        def_id: LocalDefId,
+
+        found: ty::OpaqueHiddenType<'tcx>,
+    }
+
+    impl ConstraintChecker<'_> {
+        #[instrument(skip(self), level = "debug")]
+        fn check(&self, def_id: LocalDefId) {
+            // Use borrowck to get the type with unerased regions.
+            let concrete_opaque_types = &self.tcx.mir_borrowck(def_id).concrete_opaque_types;
+            debug!(?concrete_opaque_types);
+            for &(def_id, concrete_type) in concrete_opaque_types {
+                if def_id != self.def_id {
+                    // Ignore constraints for other opaque types.
+                    continue;
+                }
+
+                debug!(?concrete_type, "found constraint");
+
+                if concrete_type.ty != self.found.ty
+                    && !(concrete_type, self.found).references_error()
+                {
+                    self.found.report_mismatch(&concrete_type, self.tcx);
+                }
+            }
+        }
+    }
+
+    impl<'tcx> intravisit::Visitor<'tcx> for ConstraintChecker<'tcx> {
+        type NestedFilter = nested_filter::OnlyBodies;
+
+        fn nested_visit_map(&mut self) -> Self::Map {
+            self.tcx.hir()
+        }
+        fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
+            if let hir::ExprKind::Closure { .. } = ex.kind {
+                let def_id = self.tcx.hir().local_def_id(ex.hir_id);
+                self.check(def_id);
+            }
+            intravisit::walk_expr(self, ex);
+        }
+        fn visit_item(&mut self, it: &'tcx Item<'tcx>) {
+            trace!(?it.def_id);
+            // The opaque type itself or its children are not within its reveal scope.
+            if it.def_id != self.def_id {
+                self.check(it.def_id);
+                intravisit::walk_item(self, it);
+            }
+        }
+        fn visit_impl_item(&mut self, it: &'tcx ImplItem<'tcx>) {
+            trace!(?it.def_id);
+            // The opaque type itself or its children are not within its reveal scope.
+            if it.def_id != self.def_id {
+                self.check(it.def_id);
+                intravisit::walk_impl_item(self, it);
+            }
+        }
+        fn visit_trait_item(&mut self, it: &'tcx TraitItem<'tcx>) {
+            trace!(?it.def_id);
+            self.check(it.def_id);
+            intravisit::walk_trait_item(self, it);
+        }
+    }
+
+    let concrete = tcx.mir_borrowck(owner_def_id).concrete_opaque_types.get(&def_id).copied();
+
+    if let Some(concrete) = concrete {
+        let scope = tcx.hir().local_def_id_to_hir_id(owner_def_id);
+        debug!(?scope);
+        let mut locator = ConstraintChecker { def_id: def_id, tcx, found: concrete };
+
+        match tcx.hir().get(scope) {
+            Node::Item(it) => intravisit::walk_item(&mut locator, it),
+            Node::ImplItem(it) => intravisit::walk_impl_item(&mut locator, it),
+            Node::TraitItem(it) => intravisit::walk_trait_item(&mut locator, it),
+            other => bug!("{:?} is not a valid scope for an opaque type item", other),
+        }
+    }
+
+    concrete.map(|concrete| concrete.ty).unwrap_or_else(|| {
+        let table = tcx.typeck(owner_def_id);
+        if let Some(_) = table.tainted_by_errors {
+            // Some error in the
+            // owner fn prevented us from populating
+            // the `concrete_opaque_types` table.
+            tcx.ty_error()
+        } else {
+            table
+                .concrete_opaque_types
+                .get(&def_id)
+                .copied()
+                .unwrap_or_else(|| {
+                    // We failed to resolve the opaque type or it
+                    // resolves to itself. We interpret this as the
+                    // no values of the hidden type ever being constructed,
+                    // so we can just make the hidden type be `!`.
+                    // For backwards compatibility reasons, we fall back to
+                    // `()` until we the diverging default is changed.
+                    Some(tcx.mk_diverging_default())
+                })
+                .expect("RPIT always have a hidden type from typeck")
+        }
+    })
+}
+
 fn infer_placeholder_type<'a>(
     tcx: TyCtxt<'a>,
     def_id: LocalDefId,

src/test/ui/impl-trait/issue-99073-2.rs

@@ -7,7 +7,7 @@ fn main() {
 fn test<T: Display>(t: T, recurse: bool) -> impl Display {
     let f = || {
         let i: u32 = test::<i32>(-1, false);
-        //~^ ERROR mismatched types
+        //~^ ERROR concrete type differs from previous defining opaque type use
         println!("{i}");
     };
     if recurse {

src/test/ui/impl-trait/issue-99073-2.stderr

@@ -1,15 +1,14 @@
-error[E0308]: mismatched types
+error: concrete type differs from previous defining opaque type use
   --> $DIR/issue-99073-2.rs:9:22
    |
-LL | fn test<T: Display>(t: T, recurse: bool) -> impl Display {
-   |                                             ------------ the expected opaque type
-LL |     let f = || {
 LL |         let i: u32 = test::<i32>(-1, false);
-   |                      ^^^^^^^^^^^^^^^^^^^^^^ types differ
+   |                      ^^^^^^^^^^^^^^^^^^^^^^ expected `T`, got `u32`
    |
-   = note: expected opaque type `impl std::fmt::Display`
-                     found type `u32`
+note: previous use here
+  --> $DIR/issue-99073-2.rs:16:5
+   |
+LL |     t
+   |     ^
 
 error: aborting due to previous error
 
-For more information about this error, try `rustc --explain E0308`.

src/test/ui/impl-trait/issue-99073.rs

@@ -1,8 +1,8 @@
 fn main() {
-    let _ = fix(|_: &dyn Fn()| {});
+  let _ = fix(|_: &dyn Fn()| {});
 }
 
 fn fix<F: Fn(G), G: Fn()>(f: F) -> impl Fn() {
-    move || f(fix(&f))
-    //~^ ERROR mismatched types
+  move || f(fix(&f))
+  //~^ ERROR concrete type differs from previous defining opaque type use
 }

src/test/ui/impl-trait/issue-99073.stderr

@@ -1,14 +1,14 @@
-error[E0308]: mismatched types
-  --> $DIR/issue-99073.rs:6:13
+error: concrete type differs from previous defining opaque type use
+  --> $DIR/issue-99073.rs:6:11
    |
-LL | fn fix<F: Fn(G), G: Fn()>(f: F) -> impl Fn() {
-   |                                    --------- the expected opaque type
-LL |     move || f(fix(&f))
-   |             ^^^^^^^^^^ types differ
+LL |   move || f(fix(&f))
+   |           ^^^^^^^^^^ expected `[closure@$DIR/issue-99073.rs:6:3: 6:10]`, got `G`
    |
-   = note: expected opaque type `impl Fn()`
-           found type parameter `G`
+note: previous use here
+  --> $DIR/issue-99073.rs:6:3
+   |
+LL |   move || f(fix(&f))
+   |   ^^^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 
-For more information about this error, try `rustc --explain E0308`.