Rollup merge of rust-lang#62423 - Aaron1011:fix/existential-cycle, r=oli-obk

Fix cycle error with existential types

Fixes rust-lang#61863

We now allow uses of `existential type`'s that aren't defining uses - that is, uses which don't constrain the underlying concrete type.

To make this work correctly, we also modify `eq_opaque_type_and_type` to not try to apply additional constraints to an opaque type. If we have code like this:

```rust
existential type Foo;
fn foo1() -> Foo { ... }
fn foo2() -> Foo { foo1() }
```

then `foo2` doesn't end up constraining `Foo`, which means that `foo2` will end up using the type `Foo` internally - that is, an actual `TyKind::Opaque`. We don't want to equate this to the underlying concrete type - we just need to enforce the basic equality constraint between the two types (here, the return type of `foo1` and the return type of `foo2`)

Author: Centril

src/librustc_mir/borrow_check/nll/type_check/mod.rs

@@ -1281,15 +1281,43 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
                         let opaque_defn_ty = tcx.type_of(opaque_def_id);
                         let opaque_defn_ty = opaque_defn_ty.subst(tcx, opaque_decl.substs);
                         let opaque_defn_ty = renumber::renumber_regions(infcx, &opaque_defn_ty);
+                        let concrete_is_opaque = infcx
+                            .resolve_vars_if_possible(&opaque_decl.concrete_ty).is_impl_trait();
+
                         debug!(
-                            "eq_opaque_type_and_type: concrete_ty={:?}={:?} opaque_defn_ty={:?}",
+                            "eq_opaque_type_and_type: concrete_ty={:?}={:?} opaque_defn_ty={:?} \
+                            concrete_is_opaque={}",
                             opaque_decl.concrete_ty,
                             infcx.resolve_vars_if_possible(&opaque_decl.concrete_ty),
-                            opaque_defn_ty
+                            opaque_defn_ty,
+                            concrete_is_opaque
                         );
-                        obligations.add(infcx
-                            .at(&ObligationCause::dummy(), param_env)
-                            .eq(opaque_decl.concrete_ty, opaque_defn_ty)?);
+
+                        // concrete_is_opaque is `true` when we're using an existential
+                        // type without 'revealing' it. For example, code like this:
+                        //
+                        // existential type Foo: Debug;
+                        // fn foo1() -> Foo { ... }
+                        // fn foo2() -> Foo { foo1() }
+                        //
+                        // In `foo2`, we're not revealing the type of `Foo` - we're
+                        // just treating it as the opaque type.
+                        //
+                        // When this occurs, we do *not* want to try to equate
+                        // the concrete type with the underlying defining type
+                        // of the existential type - this will always fail, since
+                        // the defining type of an existential type is always
+                        // some other type (e.g. not itself)
+                        // Essentially, none of the normal obligations apply here -
+                        // we're just passing around some unknown opaque type,
+                        // without actually looking at the underlying type it
+                        // gets 'revealed' into
+
+                        if !concrete_is_opaque {
+                            obligations.add(infcx
+                                .at(&ObligationCause::dummy(), param_env)
+                                .eq(opaque_decl.concrete_ty, opaque_defn_ty)?);
+                        }
                     }
 
                     debug!("eq_opaque_type_and_type: equated");

src/librustc_typeck/check/writeback.rs

@@ -453,36 +453,43 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
             let definition_ty = self.fcx.infer_opaque_definition_from_instantiation(
                 def_id, opaque_defn, instantiated_ty, span);
 
+            let mut skip_add = false;
+
             if let ty::Opaque(defin_ty_def_id, _substs) = definition_ty.sty {
                 if def_id == defin_ty_def_id {
-                    // Concrete type resolved to the existential type itself.
-                    // Force a cycle error.
-                    // FIXME(oli-obk): we could just not insert it into `concrete_existential_types`
-                    // which simply would make this use not a defining use.
-                    self.tcx().at(span).type_of(defin_ty_def_id);
+                    debug!("Skipping adding concrete definition for opaque type {:?} {:?}",
+                           opaque_defn, defin_ty_def_id);
+                    skip_add = true;
                 }
             }
 
             if !opaque_defn.substs.has_local_value() {
-                let new = ty::ResolvedOpaqueTy {
-                    concrete_type: definition_ty,
-                    substs: opaque_defn.substs,
-                };
-
-                let old = self.tables
-                    .concrete_existential_types
-                    .insert(def_id, new);
-                if let Some(old) = old {
-                    if old.concrete_type != definition_ty || old.substs != opaque_defn.substs {
-                        span_bug!(
-                            span,
-                            "visit_opaque_types tried to write \
-                            different types for the same existential type: {:?}, {:?}, {:?}, {:?}",
-                            def_id,
-                            definition_ty,
-                            opaque_defn,
-                            old,
-                        );
+                // We only want to add an entry into `concrete_existential_types`
+                // if we actually found a defining usage of this existential type.
+                // Otherwise, we do nothing - we'll either find a defining usage
+                // in some other location, or we'll end up emitting an error due
+                // to the lack of defining usage
+                if !skip_add {
+                    let new = ty::ResolvedOpaqueTy {
+                        concrete_type: definition_ty,
+                        substs: opaque_defn.substs,
+                    };
+
+                    let old = self.tables
+                        .concrete_existential_types
+                        .insert(def_id, new);
+                    if let Some(old) = old {
+                        if old.concrete_type != definition_ty || old.substs != opaque_defn.substs {
+                            span_bug!(
+                                span,
+                                "visit_opaque_types tried to write different types for the same \
+                                existential type: {:?}, {:?}, {:?}, {:?}",
+                                def_id,
+                                definition_ty,
+                                opaque_defn,
+                                old,
+                            );
+                        }
                     }
                 }
             } else {

src/test/ui/existential_types/existential-types-with-cycle-error.rs

@@ -1,7 +1,7 @@
 #![feature(existential_type)]
 
 existential type Foo: Fn() -> Foo;
-//~^ ERROR: cycle detected when processing `Foo`
+//~^ ERROR: could not find defining uses
 
 fn crash(x: Foo) -> Foo {
     x

src/test/ui/existential_types/existential-types-with-cycle-error.stderr

@@ -1,30 +1,8 @@
-error[E0391]: cycle detected when processing `Foo`
+error: could not find defining uses
   --> $DIR/existential-types-with-cycle-error.rs:3:1
    |
 LL | existential type Foo: Fn() -> Foo;
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-note: ...which requires processing `crash`...
-  --> $DIR/existential-types-with-cycle-error.rs:6:25
-   |
-LL |   fn crash(x: Foo) -> Foo {
-   |  _________________________^
-LL | |     x
-LL | | }
-   | |_^
-   = note: ...which again requires processing `Foo`, completing the cycle
-note: cycle used when collecting item types in top-level module
-  --> $DIR/existential-types-with-cycle-error.rs:1:1
-   |
-LL | / #![feature(existential_type)]
-LL | |
-LL | | existential type Foo: Fn() -> Foo;
-LL | |
-...  |
-LL | |
-LL | | }
-   | |_^
 
 error: aborting due to previous error
 
-For more information about this error, try `rustc --explain E0391`.

src/test/ui/existential_types/existential-types-with-cycle-error2.rs

@@ -5,7 +5,7 @@ pub trait Bar<T> {
 }
 
 existential type Foo: Bar<Foo, Item = Foo>;
-//~^ ERROR: cycle detected when processing `Foo`
+//~^ ERROR: could not find defining uses
 
 fn crash(x: Foo) -> Foo {
     x

src/test/ui/existential_types/existential-types-with-cycle-error2.stderr

@@ -1,30 +1,8 @@
-error[E0391]: cycle detected when processing `Foo`
+error: could not find defining uses
   --> $DIR/existential-types-with-cycle-error2.rs:7:1
    |
 LL | existential type Foo: Bar<Foo, Item = Foo>;
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-note: ...which requires processing `crash`...
-  --> $DIR/existential-types-with-cycle-error2.rs:10:25
-   |
-LL |   fn crash(x: Foo) -> Foo {
-   |  _________________________^
-LL | |     x
-LL | | }
-   | |_^
-   = note: ...which again requires processing `Foo`, completing the cycle
-note: cycle used when collecting item types in top-level module
-  --> $DIR/existential-types-with-cycle-error2.rs:1:1
-   |
-LL | / #![feature(existential_type)]
-LL | |
-LL | | pub trait Bar<T> {
-LL | |     type Item;
-...  |
-LL | |
-LL | | }
-   | |_^
 
 error: aborting due to previous error
 
-For more information about this error, try `rustc --explain E0391`.

src/test/ui/existential_types/existential_type_const.rs

@@ -0,0 +1,20 @@
+// check-pass
+
+#![feature(existential_type)]
+// Currently, the `existential_type` feature implicitly
+// depends on `impl_trait_in_bindings` in order to work properly.
+// Specifically, this line requires `impl_trait_in_bindings` to be enabled:
+// https://github.com/rust-lang/rust/blob/481068a707679257e2a738b40987246e0420e787/src/librustc_typeck/check/mod.rs#L856
+#![feature(impl_trait_in_bindings)]
+//~^ WARN the feature `impl_trait_in_bindings` is incomplete and may cause the compiler to crash
+
+// Ensures that `const` items can constrain an `existential type`.
+
+use std::fmt::Debug;
+
+pub existential type Foo: Debug;
+
+const _FOO: Foo = 5;
+
+fn main() {
+}

src/test/ui/existential_types/existential_type_const.stderr

@@ -0,0 +1,6 @@
+warning: the feature `impl_trait_in_bindings` is incomplete and may cause the compiler to crash
+  --> $DIR/existential_type_const.rs:8:12
+   |
+LL | #![feature(impl_trait_in_bindings)]
+   |            ^^^^^^^^^^^^^^^^^^^^^^
+

src/test/ui/existential_types/existential_type_fns.rs

@@ -0,0 +1,27 @@
+// check-pass
+
+#![feature(existential_type)]
+
+// Regression test for issue #61863
+
+pub trait MyTrait {}
+
+#[derive(Debug)]
+pub struct MyStruct {
+  v: u64
+}
+
+impl MyTrait for MyStruct {}
+
+pub fn bla() -> TE {
+    return MyStruct {v:1}
+}
+
+pub fn bla2() -> TE {
+    bla()
+}
+
+
+existential type TE: MyTrait;
+
+fn main() {}

src/test/ui/existential_types/existential_type_tuple.rs

@@ -0,0 +1,33 @@
+// check-pass
+
+#![feature(existential_type)]
+#![allow(dead_code)]
+
+pub trait MyTrait {}
+
+impl MyTrait for bool {}
+
+struct Blah {
+    my_foo: Foo,
+    my_u8: u8
+}
+
+impl Blah {
+    fn new() -> Blah {
+        Blah {
+            my_foo: make_foo(),
+            my_u8: 12
+        }
+    }
+    fn into_inner(self) -> (Foo, u8) {
+        (self.my_foo, self.my_u8)
+    }
+}
+
+fn make_foo() -> Foo {
+    true
+}
+
+existential type Foo: MyTrait;
+
+fn main() {}