Auto merge of rust-lang#113636 - compiler-errors:opaque-recursive-check-bad, r=oli-obk

Restrict recursive opaque type check

We have a recursive opaque check in writeback to avoid inferring the hidden of an opaque type to be itself:

https://github.com/rust-lang/rust/blob/33a2c2487ac5d9927830ea4c1844335c6b9f77db/compiler/rustc_hir_typeck/src/writeback.rs#L556-L575

Issue rust-lang#113619 treats `make_option2` as not defining the TAIT `TestImpl` since it is inferred to have the definition `TestImpl := B<TestImpl>`, which fails this check. This regressed in rust-lang#102700 (5d15beb), I think due to the refactoring that made us record the hidden types of TAITs during writeback.

However, nothing actually seems to go bad if we relax this recursion checker to only check for directly recursive definitions. This PR fixes rust-lang#113619 by changing this recursive check from being a visitor to just checking that the hidden type is exactly the same as the opaque being inferred.

Alternatively, we may be able to fix rust-lang#113619 by restricting this recursion check only to RPITs/async fns. It seems to only be possible to use misuse the recursion check to cause ICEs for TAITs (though I didn't try too hard to create a bad RPIT example... may be possible, actually.)

r? `@oli-obk`

--

Fixes rust-lang#113314

Author: bors

compiler/rustc_hir_typeck/src/writeback.rs

@@ -3,21 +3,19 @@
 // substitutions.
 
 use crate::FnCtxt;
-use hir::def_id::LocalDefId;
 use rustc_data_structures::unord::ExtendUnord;
 use rustc_errors::{ErrorGuaranteed, StashKey};
 use rustc_hir as hir;
 use rustc_hir::intravisit::{self, Visitor};
 use rustc_infer::infer::error_reporting::TypeAnnotationNeeded::E0282;
 use rustc_middle::ty::adjustment::{Adjust, Adjustment, PointerCoercion};
 use rustc_middle::ty::fold::{TypeFoldable, TypeFolder, TypeSuperFoldable};
-use rustc_middle::ty::visit::{TypeSuperVisitable, TypeVisitable, TypeVisitableExt};
+use rustc_middle::ty::visit::TypeVisitableExt;
 use rustc_middle::ty::{self, Ty, TyCtxt};
 use rustc_span::symbol::sym;
 use rustc_span::Span;
 
 use std::mem;
-use std::ops::ControlFlow;
 
 ///////////////////////////////////////////////////////////////////////////
 // Entry point
@@ -565,23 +563,9 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
             let hidden_type = self.resolve(decl.hidden_type, &decl.hidden_type.span);
             let opaque_type_key = self.resolve(opaque_type_key, &decl.hidden_type.span);
 
-            struct RecursionChecker {
-                def_id: LocalDefId,
-            }
-            impl<'tcx> ty::TypeVisitor<TyCtxt<'tcx>> for RecursionChecker {
-                type BreakTy = ();
-                fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
-                    if let ty::Alias(ty::Opaque, ty::AliasTy { def_id, .. }) = *t.kind() {
-                        if def_id == self.def_id.to_def_id() {
-                            return ControlFlow::Break(());
-                        }
-                    }
-                    t.super_visit_with(self)
-                }
-            }
-            if hidden_type
-                .visit_with(&mut RecursionChecker { def_id: opaque_type_key.def_id })
-                .is_break()
+            if let ty::Alias(ty::Opaque, alias_ty) = hidden_type.ty.kind()
+                && alias_ty.def_id == opaque_type_key.def_id.to_def_id()
+                && alias_ty.args == opaque_type_key.args
             {
                 continue;
             }

tests/ui/type-alias-impl-trait/recursive-tait-conflicting-defn-2.rs

@@ -0,0 +1,21 @@
+// issue: 113314
+
+#![feature(type_alias_impl_trait)]
+
+type Op = impl std::fmt::Display;
+fn foo() -> Op { &"hello world" }
+
+fn transform<S>() -> impl std::fmt::Display {
+    &0usize
+}
+fn bad() -> Op {
+    transform::<Op>()
+    //~^ ERROR concrete type differs from previous defining opaque type use
+}
+
+fn main() {
+    let mut x = foo();
+    println!("{x}");
+    x = bad();
+    println!("{x}");
+}

tests/ui/type-alias-impl-trait/recursive-tait-conflicting-defn-2.stderr

@@ -0,0 +1,14 @@
+error: concrete type differs from previous defining opaque type use
+  --> $DIR/recursive-tait-conflicting-defn-2.rs:12:5
+   |
+LL |     transform::<Op>()
+   |     ^^^^^^^^^^^^^^^^^ expected `&'static &'static str`, got `impl std::fmt::Display`
+   |
+note: previous use here
+  --> $DIR/recursive-tait-conflicting-defn-2.rs:6:18
+   |
+LL | fn foo() -> Op { &"hello world" }
+   |                  ^^^^^^^^^^^^^^
+
+error: aborting due to previous error
+

tests/ui/type-alias-impl-trait/recursive-tait-conflicting-defn.rs

@@ -0,0 +1,34 @@
+// issue: 113596
+
+#![feature(type_alias_impl_trait)]
+
+trait Test {}
+
+struct A;
+
+impl Test for A {}
+
+struct B<T> {
+  inner: T,
+}
+
+impl<T: Test> Test for B<T> {}
+
+type TestImpl = impl Test;
+
+fn test() -> TestImpl {
+  A
+}
+
+fn make_option() -> Option<TestImpl> {
+  Some(test())
+}
+
+fn make_option2() -> Option<TestImpl> {
+  let inner = make_option().unwrap();
+
+  Some(B { inner })
+  //~^ ERROR concrete type differs from previous defining opaque type use
+}
+
+fn main() {}

tests/ui/type-alias-impl-trait/recursive-tait-conflicting-defn.stderr

@@ -0,0 +1,14 @@
+error: concrete type differs from previous defining opaque type use
+  --> $DIR/recursive-tait-conflicting-defn.rs:30:3
+   |
+LL |   Some(B { inner })
+   |   ^^^^^^^^^^^^^^^^^ expected `A`, got `B<TestImpl>`
+   |
+note: previous use here
+  --> $DIR/recursive-tait-conflicting-defn.rs:20:3
+   |
+LL |   A
+   |   ^
+
+error: aborting due to previous error
+