Rollup merge of #125259 - compiler-errors:fn-mut-as-a-treat, r=oli-obk

An async closure may implement `FnMut`/`Fn` if it has no self-borrows

There's no reason that async closures may not implement `FnMut` or `Fn` if they don't actually borrow anything with the closure's env lifetime. Specifically, #123660 made it so that we don't always need to borrow captures from the closure's env.

See the doc comment on `should_reborrow_from_env_of_parent_coroutine_closure`:

https://github.com/rust-lang/rust/blob/c00957a3e269219413041a4e3565f33b1f9d0779/compiler/rustc_hir_typeck/src/upvar.rs#L1777-L1823

If there are no such borrows, then we are free to implement `FnMut` and `Fn` as permitted by our closure's inferred `ClosureKind`.

As far as I can tell, this change makes `async || {}` work in precisely the set of places they used to work before #120361.
Fixes #125247.

r? oli-obk

compiler/rustc_middle/src/ty/sty.rs

@@ -401,6 +401,45 @@ impl<'tcx> CoroutineClosureArgs<'tcx> {
     pub fn coroutine_witness_ty(self) -> Ty<'tcx> {
         self.split().coroutine_witness_ty
     }
+
+    pub fn has_self_borrows(&self) -> bool {
+        match self.coroutine_captures_by_ref_ty().kind() {
+            ty::FnPtr(sig) => sig
+                .skip_binder()
+                .visit_with(&mut HasRegionsBoundAt { binder: ty::INNERMOST })
+                .is_break(),
+            ty::Error(_) => true,
+            _ => bug!(),
+        }
+    }
+}
+/// Unlike `has_escaping_bound_vars` or `outermost_exclusive_binder`, this will
+/// detect only regions bound *at* the debruijn index.
+struct HasRegionsBoundAt {
+    binder: ty::DebruijnIndex,
+}
+// FIXME: Could be optimized to not walk into components with no escaping bound vars.
+impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for HasRegionsBoundAt {
+    type Result = ControlFlow<()>;
+    fn visit_binder<T: TypeVisitable<TyCtxt<'tcx>>>(
+        &mut self,
+        t: &ty::Binder<'tcx, T>,
+    ) -> Self::Result {
+        self.binder.shift_in(1);
+        t.super_visit_with(self)?;
+        self.binder.shift_out(1);
+        ControlFlow::Continue(())
+    }
+
+    fn visit_region(&mut self, r: ty::Region<'tcx>) -> Self::Result {
+        if let ty::ReBound(binder, _) = *r
+            && self.binder == binder
+        {
+            ControlFlow::Break(())
+        } else {
+            ControlFlow::Continue(())
+        }
+    }
 }
 
 #[derive(Copy, Clone, PartialEq, Eq, Debug, TypeFoldable, TypeVisitable)]

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

@@ -300,14 +300,11 @@ pub(in crate::solve) fn extract_tupled_inputs_and_output_from_callable<'tcx>(
                     return Err(NoSolution);
                 }
 
-                // If `Fn`/`FnMut`, we only implement this goal if we
-                // have no captures.
-                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"),
-                };
-                if closure_kind != ty::ClosureKind::FnOnce && !no_borrows {
+                // A coroutine-closure implements `FnOnce` *always*, since it may
+                // always be called once. It additionally implements `Fn`/`FnMut`
+                // only if it has no upvars referencing the closure-env lifetime,
+                // and if the closure kind permits it.
+                if closure_kind != ty::ClosureKind::FnOnce && args.has_self_borrows() {
                     return Err(NoSolution);
                 }
 

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

@@ -418,20 +418,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                     // 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) {
+                    // only if it has no upvars referencing the closure-env lifetime,
+                    // and if the closure kind permits it.
+                    if closure_kind.extends(kind) && !args.has_self_borrows() {
                         candidates.vec.push(ClosureCandidate { is_const });
                     } else if kind == ty::ClosureKind::FnOnce {
                         candidates.vec.push(ClosureCandidate { is_const });

tests/ui/async-await/async-closures/implements-fnmut.rs

@@ -0,0 +1,23 @@
+//@ check-pass
+//@ edition: 2021
+
+// Demonstrates that an async closure may implement `FnMut` (not just `async FnMut`!)
+// if it has no self-borrows. In this case, `&Ty` is not borrowed from the closure env,
+// since it's fine to reborrow it with its original lifetime. See the doc comment on
+// `should_reborrow_from_env_of_parent_coroutine_closure` for more detail for when we
+// must borrow from the closure env.
+
+#![feature(async_closure)]
+
+fn main() {}
+
+fn needs_fn_mut<T>(x: impl FnMut() -> T) {}
+
+fn hello(x: &Ty) {
+    needs_fn_mut(async || { x.hello(); });
+}
+
+struct Ty;
+impl Ty {
+    fn hello(&self) {}
+}