Auto merge of rust-lang#121679 - lcnr:opaque-wf-check-2, r=oli-obk

stricter hidden type wf-check [based on rust-lang#115008]

Original work by `@aliemjay` in rust-lang#115008. A huge thanks to them for originally figuring out this approach ❤️

Fixes rust-lang#114728
Fixes rust-lang#114572

Instead of adding the `WellFormed` obligations when relating opaque types, we now always emit such an obligation when defining the hidden type.

This causes nested opaque types which aren't wf to error, see the comment below for the described impact. I believe this change to be desirable as it significantly reduces complexity by removing special-cases.

It also caused an issue with RPITIT: in defaulted trait methods, we add a `Projection(synthetic_assoc, rpit_of_trait_method)` clause to the `param_env`. This clause is not added to the `ParamEnv` of the nested coroutines. This caused a normalization failure in `fn check_coroutine_obligations` with the new solver. I fixed that by using the env of the typeck root instead.

r? `@oli-obk`

compiler/rustc_hir_analysis/src/check/check.rs

@@ -402,39 +402,29 @@ fn check_opaque_meets_bounds<'tcx>(
         let guar = infcx.err_ctxt().report_fulfillment_errors(errors);
         return Err(guar);
     }
-    match origin {
-        // Checked when type checking the function containing them.
-        hir::OpaqueTyOrigin::FnReturn(..) | hir::OpaqueTyOrigin::AsyncFn(..) => {
-            // HACK: this should also fall through to the hidden type check below, but the original
-            // implementation had a bug where equivalent lifetimes are not identical. This caused us
-            // to reject existing stable code that is otherwise completely fine. The real fix is to
-            // compare the hidden types via our type equivalence/relation infra instead of doing an
-            // identity check.
-            let _ = infcx.take_opaque_types();
-            return Ok(());
-        }
-        // Nested opaque types occur only in associated types:
-        // ` type Opaque<T> = impl Trait<&'static T, AssocTy = impl Nested>; `
-        // They can only be referenced as `<Opaque<T> as Trait<&'static T>>::AssocTy`.
-        // We don't have to check them here because their well-formedness follows from the WF of
-        // the projection input types in the defining- and use-sites.
-        hir::OpaqueTyOrigin::TyAlias { .. }
-            if tcx.def_kind(tcx.parent(def_id.to_def_id())) == DefKind::OpaqueTy => {}
-        // Can have different predicates to their defining use
-        hir::OpaqueTyOrigin::TyAlias { .. } => {
-            let wf_tys = ocx.assumed_wf_types_and_report_errors(param_env, def_id)?;
-            let implied_bounds = infcx.implied_bounds_tys(param_env, def_id, &wf_tys);
-            let outlives_env = OutlivesEnvironment::with_bounds(param_env, implied_bounds);
-            ocx.resolve_regions_and_report_errors(defining_use_anchor, &outlives_env)?;
+
+    let wf_tys = ocx.assumed_wf_types_and_report_errors(param_env, defining_use_anchor)?;
+    let implied_bounds = infcx.implied_bounds_tys(param_env, def_id, &wf_tys);
+    let outlives_env = OutlivesEnvironment::with_bounds(param_env, implied_bounds);
+    ocx.resolve_regions_and_report_errors(defining_use_anchor, &outlives_env)?;
+
+    if let hir::OpaqueTyOrigin::FnReturn(..) | hir::OpaqueTyOrigin::AsyncFn(..) = origin {
+        // HACK: this should also fall through to the hidden type check below, but the original
+        // implementation had a bug where equivalent lifetimes are not identical. This caused us
+        // to reject existing stable code that is otherwise completely fine. The real fix is to
+        // compare the hidden types via our type equivalence/relation infra instead of doing an
+        // identity check.
+        let _ = infcx.take_opaque_types();
+        Ok(())
+    } else {
+        // Check that any hidden types found during wf checking match the hidden types that `type_of` sees.
+        for (mut key, mut ty) in infcx.take_opaque_types() {
+            ty.hidden_type.ty = infcx.resolve_vars_if_possible(ty.hidden_type.ty);
+            key = infcx.resolve_vars_if_possible(key);
+            sanity_check_found_hidden_type(tcx, key, ty.hidden_type)?;
         }
+        Ok(())
     }
-    // Check that any hidden types found during wf checking match the hidden types that `type_of` sees.
-    for (mut key, mut ty) in infcx.take_opaque_types() {
-        ty.hidden_type.ty = infcx.resolve_vars_if_possible(ty.hidden_type.ty);
-        key = infcx.resolve_vars_if_possible(key);
-        sanity_check_found_hidden_type(tcx, key, ty.hidden_type)?;
-    }
-    Ok(())
 }
 
 fn sanity_check_found_hidden_type<'tcx>(
@@ -1549,14 +1539,16 @@ fn opaque_type_cycle_error(
     err.emit()
 }
 
+// FIXME(@lcnr): This should not be computed per coroutine, but instead once for
+// each typeck root.
 pub(super) fn check_coroutine_obligations(
     tcx: TyCtxt<'_>,
     def_id: LocalDefId,
 ) -> Result<(), ErrorGuaranteed> {
     debug_assert!(tcx.is_coroutine(def_id.to_def_id()));
 
     let typeck = tcx.typeck(def_id);
-    let param_env = tcx.param_env(def_id);
+    let param_env = tcx.param_env(typeck.hir_owner.def_id);
 
     let coroutine_interior_predicates = &typeck.coroutine_interior_predicates[&def_id];
     debug!(?coroutine_interior_predicates);

compiler/rustc_infer/src/infer/opaque_types.rs

@@ -592,8 +592,25 @@ impl<'tcx> InferCtxt<'tcx> {
         obligations: &mut Vec<PredicateObligation<'tcx>>,
     ) {
         let tcx = self.tcx;
-        let item_bounds = tcx.explicit_item_bounds(def_id);
+        // Require that the hidden type is well-formed. We have to
+        // make sure we wf-check the hidden type to fix #114728.
+        //
+        // However, we don't check that all types are well-formed.
+        // We only do so for types provided by the user or if they are
+        // "used", e.g. for method selection.
+        //
+        // This means we never check the wf requirements of the hidden
+        // type during MIR borrowck, causing us to infer the wrong
+        // lifetime for its member constraints which then results in
+        // unexpected region errors.
+        obligations.push(traits::Obligation::new(
+            tcx,
+            cause.clone(),
+            param_env,
+            ty::ClauseKind::WellFormed(hidden_ty.into()),
+        ));
 
+        let item_bounds = tcx.explicit_item_bounds(def_id);
         for (predicate, _) in item_bounds.iter_instantiated_copied(tcx, args) {
             let predicate = predicate.fold_with(&mut BottomUpFolder {
                 tcx,

compiler/rustc_ty_utils/src/implied_bounds.rs

@@ -121,18 +121,7 @@ fn assumed_wf_types<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> &'tcx [(Ty<'
             }
         }
         DefKind::AssocConst | DefKind::AssocTy => tcx.assumed_wf_types(tcx.local_parent(def_id)),
-        DefKind::OpaqueTy => match tcx.def_kind(tcx.local_parent(def_id)) {
-            DefKind::TyAlias => ty::List::empty(),
-            DefKind::AssocTy => tcx.assumed_wf_types(tcx.local_parent(def_id)),
-            // Nested opaque types only occur in associated types:
-            // ` type Opaque<T> = impl Trait<&'static T, AssocTy = impl Nested>; `
-            // assumed_wf_types should include those of `Opaque<T>`, `Opaque<T>` itself
-            // and `&'static T`.
-            DefKind::OpaqueTy => bug!("unimplemented implied bounds for nested opaque types"),
-            def_kind => {
-                bug!("unimplemented implied bounds for opaque types with parent {def_kind:?}")
-            }
-        },
+        DefKind::OpaqueTy => bug!("implied bounds are not defined for opaques"),
         DefKind::Mod
         | DefKind::Struct
         | DefKind::Union

tests/ui/closures/issue-78720.rs

@@ -1,7 +1,7 @@
 fn server() -> impl {
-//~^ ERROR at least one trait must be specified
+    //~^ ERROR at least one trait must be specified
+    //~| ERROR type annotations needed
     ().map2(|| "")
-    //~^ ERROR type annotations needed
 }
 
 trait FilterBase2 {

tests/ui/closures/issue-78720.stderr

@@ -23,10 +23,10 @@ LL | struct Map2<Segment2, F> {
    |                     +++
 
 error[E0282]: type annotations needed
-  --> $DIR/issue-78720.rs:3:5
+  --> $DIR/issue-78720.rs:1:16
    |
-LL |     ().map2(|| "")
-   |     ^^^^^^^^^^^^^^ cannot infer type
+LL | fn server() -> impl {
+   |                ^^^^ cannot infer type
 
 error[E0308]: mismatched types
   --> $DIR/issue-78720.rs:8:39

tests/ui/impl-trait/issues/issue-86800.rs

@@ -1,12 +1,6 @@
 #![feature(type_alias_impl_trait)]
 
 //@ edition:2021
-//@ compile-flags:-Z treat-err-as-bug=2
-//@ error-pattern: due to `-Z treat-err-as-bug=2
-//@ failure-status:101
-//@ normalize-stderr-test ".*note: .*\n\n" -> ""
-//@ normalize-stderr-test "thread 'rustc' panicked.*:\n.*\n" -> ""
-//@ rustc-env:RUST_BACKTRACE=0
 
 use std::future::Future;
 
@@ -29,6 +23,7 @@ struct Context {
 type TransactionResult<O> = Result<O, ()>;
 
 type TransactionFuture<'__, O> = impl '__ + Future<Output = TransactionResult<O>>;
+//~^ ERROR unconstrained opaque type
 
 fn execute_transaction_fut<'f, F, O>(
     f: F,
@@ -37,13 +32,15 @@ where
     F: FnOnce(&mut dyn Transaction) -> TransactionFuture<'_, O> + 'f
 {
     f
+    //~^ ERROR expected generic lifetime parameter, found `'_`
 }
 
 impl Context {
     async fn do_transaction<O>(
         &self, f: impl FnOnce(&mut dyn Transaction) -> TransactionFuture<'_, O>
     ) -> TransactionResult<O>
     {
+        //~^ ERROR expected generic lifetime parameter, found `'_`
         let mut conn = Connection {};
         let mut transaction = TestTransaction { conn: &mut conn };
         f(&mut transaction).await

tests/ui/impl-trait/issues/issue-86800.stderr

@@ -1,21 +1,34 @@
 error: unconstrained opaque type
-  --> $DIR/issue-86800.rs:31:34
+  --> $DIR/issue-86800.rs:25:34
    |
 LL | type TransactionFuture<'__, O> = impl '__ + Future<Output = TransactionResult<O>>;
    |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
-error: internal compiler error[E0792]: expected generic lifetime parameter, found `'_`
-  --> $DIR/issue-86800.rs:39:5
+   = note: `TransactionFuture` must be used in combination with a concrete type within the same module
+
+error[E0792]: expected generic lifetime parameter, found `'_`
+  --> $DIR/issue-86800.rs:34:5
    |
 LL | type TransactionFuture<'__, O> = impl '__ + Future<Output = TransactionResult<O>>;
    |                        --- this generic parameter must be used with a generic lifetime parameter
 ...
 LL |     f
    |     ^
 
-error: the compiler unexpectedly panicked. this is a bug.
+error[E0792]: expected generic lifetime parameter, found `'_`
+  --> $DIR/issue-86800.rs:42:5
+   |
+LL |   type TransactionFuture<'__, O> = impl '__ + Future<Output = TransactionResult<O>>;
+   |                          --- this generic parameter must be used with a generic lifetime parameter
+...
+LL | /     {
+LL | |
+LL | |         let mut conn = Connection {};
+LL | |         let mut transaction = TestTransaction { conn: &mut conn };
+LL | |         f(&mut transaction).await
+LL | |     }
+   | |_____^
+
+error: aborting due to 3 previous errors
 
-query stack during panic:
-#0 [mir_borrowck] borrow-checking `execute_transaction_fut`
-#1 [type_of_opaque] computing type of opaque `execute_transaction_fut::{opaque#0}`
-end of query stack
+For more information about this error, try `rustc --explain E0792`.

tests/ui/impl-trait/recursive-coroutine-boxed.next.stderr

@@ -1,9 +1,9 @@
 error[E0282]: type annotations needed
-  --> $DIR/recursive-coroutine-boxed.rs:10:23
+  --> $DIR/recursive-coroutine-boxed.rs:11:23
    |
 LL |         let mut gen = Box::pin(foo());
    |                       ^^^^^^^^ cannot infer type of the type parameter `T` declared on the struct `Box`
-...
+LL |
 LL |         let mut r = gen.as_mut().resume(());
    |                         ------ type must be known at this point
    |
@@ -13,10 +13,10 @@ LL |         let mut gen = Box::<T>::pin(foo());
    |                          +++++
 
 error[E0282]: type annotations needed
-  --> $DIR/recursive-coroutine-boxed.rs:10:32
+  --> $DIR/recursive-coroutine-boxed.rs:8:13
    |
-LL |         let mut gen = Box::pin(foo());
-   |                                ^^^^^ cannot infer type for opaque type `impl Coroutine<Yield = (), Return = ()>`
+LL | fn foo() -> impl Coroutine<Yield = (), Return = ()> {
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot infer type for opaque type `impl Coroutine<Yield = (), Return = ()>`
 
 error: aborting due to 2 previous errors
 

tests/ui/impl-trait/recursive-coroutine-boxed.rs

@@ -6,10 +6,10 @@
 use std::ops::{Coroutine, CoroutineState};
 
 fn foo() -> impl Coroutine<Yield = (), Return = ()> {
+    //[next]~^ ERROR type annotations needed
     || {
         let mut gen = Box::pin(foo());
         //[next]~^ ERROR type annotations needed
-        //[next]~| ERROR type annotations needed
         let mut r = gen.as_mut().resume(());
         while let CoroutineState::Yielded(v) = r {
             yield v;

tests/ui/impl-trait/wf-check-hidden-type.rs

@@ -0,0 +1,20 @@
+//! Regression test for #114728.
+
+trait Extend<'a, 'b> {
+    fn extend(self, _: &'a str) -> &'b str;
+}
+
+impl<'a, 'b> Extend<'a, 'b> for Option<&'b &'a ()> {
+    fn extend(self, s: &'a str) -> &'b str {
+        s
+    }
+}
+
+fn boom<'a, 'b>() -> impl Extend<'a, 'b> {
+    None::<&'_ &'_ ()> //~ ERROR lifetime may not live long enough
+}
+
+fn main() {
+    let y = boom().extend(&String::from("temporary"));
+    println!("{}", y);
+}

tests/ui/impl-trait/wf-check-hidden-type.stderr

@@ -0,0 +1,14 @@
+error: lifetime may not live long enough
+  --> $DIR/wf-check-hidden-type.rs:14:5
+   |
+LL | fn boom<'a, 'b>() -> impl Extend<'a, 'b> {
+   |         --  -- lifetime `'b` defined here
+   |         |
+   |         lifetime `'a` defined here
+LL |     None::<&'_ &'_ ()>
+   |     ^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a`
+   |
+   = help: consider adding the following bound: `'a: 'b`
+
+error: aborting due to 1 previous error
+

tests/ui/lifetimes/issue-76168-hr-outlives-3.rs

@@ -6,6 +6,7 @@ use std::future::Future;
 async fn wrapper<F>(f: F)
 //~^ ERROR: expected a `FnOnce(&'a mut i32)` closure, found `i32`
 //~| ERROR: expected a `FnOnce(&'a mut i32)` closure, found `i32`
+//~| ERROR: expected a `FnOnce(&'a mut i32)` closure, found `i32`
 where
 F:,
 for<'a> <i32 as FnOnce<(&'a mut i32,)>>::Output: Future<Output = ()> + 'a,

tests/ui/lifetimes/issue-76168-hr-outlives-3.stderr

@@ -4,6 +4,7 @@ error[E0277]: expected a `FnOnce(&'a mut i32)` closure, found `i32`
 LL | / async fn wrapper<F>(f: F)
 LL | |
 LL | |
+LL | |
 LL | | where
 LL | | F:,
 LL | | for<'a> <i32 as FnOnce<(&'a mut i32,)>>::Output: Future<Output = ()> + 'a,
@@ -20,7 +21,21 @@ LL | async fn wrapper<F>(f: F)
    = help: the trait `for<'a> FnOnce<(&'a mut i32,)>` is not implemented for `i32`
 
 error[E0277]: expected a `FnOnce(&'a mut i32)` closure, found `i32`
-  --> $DIR/issue-76168-hr-outlives-3.rs:12:1
+  --> $DIR/issue-76168-hr-outlives-3.rs:6:1
+   |
+LL | / async fn wrapper<F>(f: F)
+LL | |
+LL | |
+LL | |
+LL | | where
+LL | | F:,
+LL | | for<'a> <i32 as FnOnce<(&'a mut i32,)>>::Output: Future<Output = ()> + 'a,
+   | |__________________________________________________________________________^ expected an `FnOnce(&'a mut i32)` closure, found `i32`
+   |
+   = help: the trait `for<'a> FnOnce<(&'a mut i32,)>` is not implemented for `i32`
+
+error[E0277]: expected a `FnOnce(&'a mut i32)` closure, found `i32`
+  --> $DIR/issue-76168-hr-outlives-3.rs:13:1
    |
 LL | / {
 LL | |
@@ -31,6 +46,6 @@ LL | | }
    |
    = help: the trait `for<'a> FnOnce<(&'a mut i32,)>` is not implemented for `i32`
 
-error: aborting due to 3 previous errors
+error: aborting due to 4 previous errors
 
 For more information about this error, try `rustc --explain E0277`.

tests/ui/traits/non_lifetime_binders/type-match-with-late-bound.stderr

@@ -21,6 +21,20 @@ help: consider adding an explicit lifetime bound
 LL |     for<F> F: 'a, !1_"F": 'a
    |                 ~~~~~~~~~~~~
 
-error: aborting due to 1 previous error; 1 warning emitted
+error[E0309]: the placeholder type `!2_"F"` may not live long enough
+  --> $DIR/type-match-with-late-bound.rs:11:1
+   |
+LL | async fn walk2<'a, T: 'a>(_: T)
+   |                -- the placeholder type `!2_"F"` must be valid for the lifetime `'a` as defined here...
+...
+LL | {}
+   | ^^ ...so that the type `F` will meet its required lifetime bounds
+   |
+help: consider adding an explicit lifetime bound
+   |
+LL |     for<F> F: 'a, !2_"F": 'a
+   |                 ~~~~~~~~~~~~
+
+error: aborting due to 2 previous errors; 1 warning emitted
 
 For more information about this error, try `rustc --explain E0309`.

tests/ui/type-alias-impl-trait/issue-90400-2.stderr

@@ -2,15 +2,13 @@ error[E0277]: the trait bound `B: Bar` is not satisfied
   --> $DIR/issue-90400-2.rs:25:9
    |
 LL |         MyBaz(bar)
-   |         ^^^^^^^^^^ the trait `Bar` is not implemented for `B`, which is required by `MyBaz<B>: Baz`
+   |         ^^^^^^^^^^ the trait `Bar` is not implemented for `B`
    |
-note: required for `MyBaz<B>` to implement `Baz`
-  --> $DIR/issue-90400-2.rs:30:14
+note: required by a bound in `MyBaz`
+  --> $DIR/issue-90400-2.rs:29:17
    |
-LL | impl<B: Bar> Baz for MyBaz<B> {
-   |         ---  ^^^     ^^^^^^^^
-   |         |
-   |         unsatisfied trait bound introduced here
+LL | struct MyBaz<B: Bar>(B);
+   |                 ^^^ required by this bound in `MyBaz`
 help: consider restricting type parameter `B`
    |
 LL |     type FooFn<B: Bar> = impl Baz;