Auto merge of rust-lang#121679 - lcnr:opaque-wf-check-2, r=<try>

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, I add 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>(
@@ -1551,14 +1541,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

@@ -605,8 +605,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;