Skip to content

Commit

Permalink
Rollup merge of rust-lang#125259 - compiler-errors:fn-mut-as-a-treat,…
Browse files Browse the repository at this point in the history
… 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, rust-lang#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 rust-lang#120361.
Fixes rust-lang#125247.

r? oli-obk
  • Loading branch information
matthiaskrgr authored May 22, 2024
2 parents d0ef840 + 2e97dae commit 725a7fc
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 20 deletions.
39 changes: 39 additions & 0 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
23 changes: 23 additions & 0 deletions tests/ui/async-await/async-closures/implements-fnmut.rs
Original file line number Diff line number Diff line change
@@ -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) {}
}

0 comments on commit 725a7fc

Please sign in to comment.