Rollup merge of rust-lang#123662 - compiler-errors:no-upvars-yet, r=oli-obk

Don't rely on upvars being assigned just because coroutine-closure kind is assigned

Previously, code relied on the implicit assumption that if a coroutine-closure's kind variable was constrained, then its upvars were also constrained. This is because we assign all of them at once at the end up upvar analysis.

However, there's another way that a coroutine-closure's kind can be constrained: from a signature hint in closure signature deduction. After rust-lang#123350, we use these hints, which means the implicit assumption above no longer holds.

This PR adds the necessary checks so that we don't ICE.

r? oli-obk

Author: GuillaumeGomez

compiler/rustc_middle/src/ty/sty.rs

@@ -2231,7 +2231,7 @@ impl<'tcx> Ty<'tcx> {
     pub fn tuple_fields(self) -> &'tcx List<Ty<'tcx>> {
         match self.kind() {
             Tuple(args) => args,
-            _ => bug!("tuple_fields called on non-tuple"),
+            _ => bug!("tuple_fields called on non-tuple: {self:?}"),
         }
     }
 

compiler/rustc_trait_selection/src/solve/assembly/structural_traits.rs

@@ -292,7 +292,9 @@ pub(in crate::solve) fn extract_tupled_inputs_and_output_from_callable<'tcx>(
             let kind_ty = args.kind_ty();
             let sig = args.coroutine_closure_sig().skip_binder();
 
-            let coroutine_ty = if let Some(closure_kind) = kind_ty.to_opt_closure_kind() {
+            let coroutine_ty = if let Some(closure_kind) = kind_ty.to_opt_closure_kind()
+                && !args.tupled_upvars_ty().is_ty_var()
+            {
                 if !closure_kind.extends(goal_kind) {
                     return Err(NoSolution);
                 }
@@ -401,7 +403,9 @@ pub(in crate::solve) fn extract_tupled_inputs_and_output_from_async_callable<'tc
             let kind_ty = args.kind_ty();
             let sig = args.coroutine_closure_sig().skip_binder();
             let mut nested = vec![];
-            let coroutine_ty = if let Some(closure_kind) = kind_ty.to_opt_closure_kind() {
+            let coroutine_ty = if let Some(closure_kind) = kind_ty.to_opt_closure_kind()
+                && !args.tupled_upvars_ty().is_ty_var()
+            {
                 if !closure_kind.extends(goal_kind) {
                     return Err(NoSolution);
                 }

compiler/rustc_trait_selection/src/solve/normalizes_to/mod.rs

@@ -487,6 +487,11 @@ impl<'tcx> assembly::GoalKind<'tcx> for NormalizesTo<'tcx> {
             bug!();
         };
 
+        // Bail if the upvars haven't been constrained.
+        if tupled_upvars_ty.expect_ty().is_ty_var() {
+            return ecx.evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS);
+        }
+
         let Some(closure_kind) = closure_fn_kind_ty.expect_ty().to_opt_closure_kind() else {
             // We don't need to worry about the self type being an infer var.
             return Err(NoSolution);

compiler/rustc_trait_selection/src/traits/project.rs

@@ -1601,7 +1601,10 @@ fn confirm_closure_candidate<'cx, 'tcx>(
                 // If we know the kind and upvars, use that directly.
                 // Otherwise, defer to `AsyncFnKindHelper::Upvars` to delay
                 // the projection, like the `AsyncFn*` traits do.
-                let output_ty = if let Some(_) = kind_ty.to_opt_closure_kind() {
+                let output_ty = if let Some(_) = kind_ty.to_opt_closure_kind()
+                    // Fall back to projection if upvars aren't constrained
+                    && !args.tupled_upvars_ty().is_ty_var()
+                {
                     sig.to_coroutine_given_kind_and_upvars(
                         tcx,
                         args.parent_args(),
@@ -1731,7 +1734,10 @@ fn confirm_async_closure_candidate<'cx, 'tcx>(
 
             let term = match item_name {
                 sym::CallOnceFuture | sym::CallRefFuture => {
-                    if let Some(closure_kind) = kind_ty.to_opt_closure_kind() {
+                    if let Some(closure_kind) = kind_ty.to_opt_closure_kind()
+                        // Fall back to projection if upvars aren't constrained
+                        && !args.tupled_upvars_ty().is_ty_var()
+                    {
                         if !closure_kind.extends(goal_kind) {
                             bug!("we should not be confirming if the closure kind is not met");
                         }

compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs

@@ -400,39 +400,36 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 }
             }
             ty::CoroutineClosure(def_id, args) => {
+                let args = args.as_coroutine_closure();
                 let is_const = self.tcx().is_const_fn_raw(def_id);
-                match self.infcx.closure_kind(self_ty) {
-                    Some(closure_kind) => {
-                        let no_borrows = match self
-                            .infcx
-                            .shallow_resolve(args.as_coroutine_closure().tupled_upvars_ty())
-                            .kind()
-                        {
-                            ty::Tuple(tys) => tys.is_empty(),
-                            ty::Error(_) => false,
-                            _ => bug!("tuple_fields called on non-tuple"),
-                        };
-                        // A coroutine-closure implements `FnOnce` *always*, since it may
-                        // always be called once. It additionally implements `Fn`/`FnMut`
-                        // only if it has no upvars (therefore no borrows from the closure
-                        // that would need to be represented with a lifetime) and if the
-                        // closure kind permits it.
-                        // FIXME(async_closures): Actually, it could also implement `Fn`/`FnMut`
-                        // if it takes all of its upvars by copy, and none by ref. This would
-                        // require us to record a bit more information during upvar analysis.
-                        if no_borrows && closure_kind.extends(kind) {
-                            candidates.vec.push(ClosureCandidate { is_const });
-                        } else if kind == ty::ClosureKind::FnOnce {
-                            candidates.vec.push(ClosureCandidate { is_const });
-                        }
+                if let Some(closure_kind) = self.infcx.closure_kind(self_ty)
+                    // Ambiguity if upvars haven't been constrained yet
+                    && !args.tupled_upvars_ty().is_ty_var()
+                {
+                    let no_borrows = match args.tupled_upvars_ty().kind() {
+                        ty::Tuple(tys) => tys.is_empty(),
+                        ty::Error(_) => false,
+                        _ => bug!("tuple_fields called on non-tuple"),
+                    };
+                    // A coroutine-closure implements `FnOnce` *always*, since it may
+                    // always be called once. It additionally implements `Fn`/`FnMut`
+                    // only if it has no upvars (therefore no borrows from the closure
+                    // that would need to be represented with a lifetime) and if the
+                    // closure kind permits it.
+                    // FIXME(async_closures): Actually, it could also implement `Fn`/`FnMut`
+                    // if it takes all of its upvars by copy, and none by ref. This would
+                    // require us to record a bit more information during upvar analysis.
+                    if no_borrows && closure_kind.extends(kind) {
+                        candidates.vec.push(ClosureCandidate { is_const });
+                    } else if kind == ty::ClosureKind::FnOnce {
+                        candidates.vec.push(ClosureCandidate { is_const });
                     }
-                    None => {
-                        if kind == ty::ClosureKind::FnOnce {
-                            candidates.vec.push(ClosureCandidate { is_const });
-                        } else {
-                            // This stays ambiguous until kind+upvars are determined.
-                            candidates.ambiguous = true;
-                        }
+                } else {
+                    if kind == ty::ClosureKind::FnOnce {
+                        candidates.vec.push(ClosureCandidate { is_const });
+                    } else {
+                        // This stays ambiguous until kind+upvars are determined.
+                        candidates.ambiguous = true;
                     }
                 }
             }

tests/ui/async-await/async-closures/constrained-but-no-upvars-yet.rs

@@ -0,0 +1,27 @@
+//@ edition: 2021
+//@ check-pass
+//@ revisions: current next
+//@ ignore-compare-mode-next-solver (explicit revisions)
+//@[next] compile-flags: -Znext-solver
+
+#![feature(async_closure)]
+
+fn constrain<T: async FnOnce()>(t: T) -> T {
+    t
+}
+
+fn call_once<T>(f: impl FnOnce() -> T) -> T {
+    f()
+}
+
+async fn async_call_once<T>(f: impl async FnOnce() -> T) -> T {
+    f().await
+}
+
+fn main() {
+    let c = constrain(async || {});
+    call_once(c);
+
+    let c = constrain(async || {});
+    async_call_once(c);
+}