Skip to content

Commit

Permalink
Auto merge of rust-lang#120361 - compiler-errors:async-closures, r=ol…
Browse files Browse the repository at this point in the history
…i-obk

Rework support for async closures; allow them to return futures that borrow from the closure's captures

This PR implements a new lowering for async closures via `TyKind::CoroutineClosure` which handles the curious relationship between the closure and the coroutine that it returns.

I wrote up a bunch in [this hackmd](https://hackmd.io/`@compiler-errors/S1HvqQxca)` which will be copied to the dev guide after this PR lands, and hopefully left sufficient comments in the source code explaining why this change is as large as it is.

This also necessitates that they begin implementing the `AsyncFn`-family of traits, rather than the `Fn`-family of traits -- if you need `Fn` implementations, you should probably use the non-sugar `|| async {}` syntax instead.

Notably this PR does not yet implement `async Fn()` syntax sugar for bounds, but I expect to add those soon (**edit:** rust-lang#120392). For now, users must use `AsyncFn()` traits directly, which necessitates adding the `async_fn_traits` feature gate as well. I will add this as a follow-up very soon.

r? oli-obk

This is based on top of rust-lang#120322, but that PR is minimal.
  • Loading branch information
bors committed Feb 6, 2024
2 parents f067fd6 + b53905e commit 8d6080a
Show file tree
Hide file tree
Showing 182 changed files with 3,613 additions and 366 deletions.
3 changes: 0 additions & 3 deletions compiler/rustc_ast_lowering/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,6 @@ ast_lowering_never_pattern_with_guard =
a guard on a never pattern will never be run
.suggestion = remove this guard
ast_lowering_not_supported_for_lifetime_binder_async_closure =
`for<...>` binders on `async` closures are not currently supported
ast_lowering_previously_used_here = previously used here
ast_lowering_register1 = register `{$reg1_name}`
Expand Down
7 changes: 0 additions & 7 deletions compiler/rustc_ast_lowering/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,13 +326,6 @@ pub struct MisplacedRelaxTraitBound {
pub span: Span,
}

#[derive(Diagnostic, Clone, Copy)]
#[diag(ast_lowering_not_supported_for_lifetime_binder_async_closure)]
pub struct NotSupportedForLifetimeBinderAsyncClosure {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(ast_lowering_match_arm_with_no_body)]
pub struct MatchArmWithNoBody {
Expand Down
38 changes: 18 additions & 20 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use std::assert_matches::assert_matches;

use super::errors::{
AsyncCoroutinesNotSupported, AwaitOnlyInAsyncFnAndBlocks, BaseExpressionDoubleDot,
ClosureCannotBeStatic, CoroutineTooManyParameters,
FunctionalRecordUpdateDestructuringAssignment, InclusiveRangeWithNoEnd, MatchArmWithNoBody,
NeverPatternWithBody, NeverPatternWithGuard, NotSupportedForLifetimeBinderAsyncClosure,
UnderscoreExprLhsAssign,
NeverPatternWithBody, NeverPatternWithGuard, UnderscoreExprLhsAssign,
};
use super::ResolverAstLoweringExt;
use super::{ImplTraitContext, LoweringContext, ParamMode, ParenthesizedGenericArgs};
Expand Down Expand Up @@ -1026,30 +1027,27 @@ impl<'hir> LoweringContext<'_, 'hir> {
fn_decl_span: Span,
fn_arg_span: Span,
) -> hir::ExprKind<'hir> {
if let &ClosureBinder::For { span, .. } = binder {
self.dcx().emit_err(NotSupportedForLifetimeBinderAsyncClosure { span });
}

let (binder_clause, generic_params) = self.lower_closure_binder(binder);

assert_matches!(
coroutine_kind,
CoroutineKind::Async { .. },
"only async closures are supported currently"
);

let body = self.with_new_scopes(fn_decl_span, |this| {
let inner_decl =
FnDecl { inputs: decl.inputs.clone(), output: FnRetTy::Default(fn_decl_span) };

// Transform `async |x: u8| -> X { ... }` into
// `|x: u8| || -> X { ... }`.
let body_id = this.lower_body(|this| {
let async_ret_ty = if let FnRetTy::Ty(ty) = &decl.output {
let itctx = ImplTraitContext::Disallowed(ImplTraitPosition::AsyncBlock);
Some(hir::FnRetTy::Return(this.lower_ty(ty, &itctx)))
} else {
None
};

let (parameters, expr) = this.lower_coroutine_body_with_moved_arguments(
decl,
&inner_decl,
|this| this.with_new_scopes(fn_decl_span, |this| this.lower_expr_mut(body)),
body.span,
coroutine_kind,
hir::CoroutineSource::Closure,
async_ret_ty,
);

let hir_id = this.lower_node_id(coroutine_kind.closure_id());
Expand All @@ -1060,15 +1058,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
body_id
});

let outer_decl =
FnDecl { inputs: decl.inputs.clone(), output: FnRetTy::Default(fn_decl_span) };

let bound_generic_params = self.lower_lifetime_binder(closure_id, generic_params);
// We need to lower the declaration outside the new scope, because we
// have to conserve the state of being inside a loop condition for the
// closure argument types.
let fn_decl =
self.lower_fn_decl(&outer_decl, closure_id, fn_decl_span, FnDeclKind::Closure, None);
self.lower_fn_decl(&decl, closure_id, fn_decl_span, FnDeclKind::Closure, None);

let c = self.arena.alloc(hir::Closure {
def_id: self.local_def_id(closure_id),
Expand All @@ -1079,7 +1074,10 @@ impl<'hir> LoweringContext<'_, 'hir> {
body,
fn_decl_span: self.lower_span(fn_decl_span),
fn_arg_span: Some(self.lower_span(fn_arg_span)),
kind: hir::ClosureKind::Closure,
// Lower this as a `CoroutineClosure`. That will ensure that HIR typeck
// knows that a `FnDecl` output type like `-> &str` actually means
// "coroutine that returns &str", rather than directly returning a `&str`.
kind: hir::ClosureKind::CoroutineClosure(hir::CoroutineDesugaring::Async),
constness: hir::Constness::NotConst,
});
hir::ExprKind::Closure(c)
Expand Down
13 changes: 6 additions & 7 deletions compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1086,7 +1086,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
body.span,
coroutine_kind,
hir::CoroutineSource::Fn,
None,
);

// FIXME(async_fn_track_caller): Can this be moved above?
Expand All @@ -1108,7 +1107,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
body_span: Span,
coroutine_kind: CoroutineKind,
coroutine_source: hir::CoroutineSource,
return_type_hint: Option<hir::FnRetTy<'hir>>,
) -> (&'hir [hir::Param<'hir>], hir::Expr<'hir>) {
let mut parameters: Vec<hir::Param<'_>> = Vec::new();
let mut statements: Vec<hir::Stmt<'_>> = Vec::new();
Expand Down Expand Up @@ -1278,12 +1276,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
};
let closure_id = coroutine_kind.closure_id();
let coroutine_expr = self.make_desugared_coroutine_expr(
// FIXME(async_closures): This should only move locals,
// and not upvars. Capturing closure upvars by ref doesn't
// work right now anyways, so whatever.
CaptureBy::Value { move_kw: rustc_span::DUMMY_SP },
// The default capture mode here is by-ref. Later on during upvar analysis,
// we will force the captured arguments to by-move, but for async closures,
// we want to make sure that we avoid unnecessarily moving captures, or else
// all async closures would default to `FnOnce` as their calling mode.
CaptureBy::Ref,
closure_id,
return_type_hint,
None,
body_span,
desugaring_kind,
coroutine_source,
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#![allow(internal_features)]
#![feature(rustdoc_internals)]
#![doc(rust_logo)]
#![feature(assert_matches)]
#![feature(box_patterns)]
#![feature(let_chains)]
#![deny(rustc::untranslatable_diagnostic)]
Expand Down Expand Up @@ -296,7 +297,6 @@ enum ImplTraitPosition {
Path,
Variable,
Trait,
AsyncBlock,
Bound,
Generic,
ExternFnParam,
Expand All @@ -323,7 +323,6 @@ impl std::fmt::Display for ImplTraitPosition {
ImplTraitPosition::Path => "paths",
ImplTraitPosition::Variable => "the type of variable bindings",
ImplTraitPosition::Trait => "traits",
ImplTraitPosition::AsyncBlock => "async blocks",
ImplTraitPosition::Bound => "bounds",
ImplTraitPosition::Generic => "generics",
ImplTraitPosition::ExternFnParam => "`extern fn` parameters",
Expand Down
23 changes: 15 additions & 8 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
use crate::session_diagnostics::CaptureVarCause::*;
match kind {
hir::ClosureKind::Coroutine(_) => MoveUseInCoroutine { var_span },
hir::ClosureKind::Closure => MoveUseInClosure { var_span },
hir::ClosureKind::Closure | hir::ClosureKind::CoroutineClosure(_) => {
MoveUseInClosure { var_span }
}
}
});

Expand Down Expand Up @@ -905,7 +907,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
hir::ClosureKind::Coroutine(_) => {
BorrowUsePlaceCoroutine { place: desc_place, var_span, is_single_var: true }
}
hir::ClosureKind::Closure => {
hir::ClosureKind::Closure | hir::ClosureKind::CoroutineClosure(_) => {
BorrowUsePlaceClosure { place: desc_place, var_span, is_single_var: true }
}
}
Expand Down Expand Up @@ -1056,7 +1058,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
var_span,
is_single_var: true,
},
hir::ClosureKind::Closure => BorrowUsePlaceClosure {
hir::ClosureKind::Closure
| hir::ClosureKind::CoroutineClosure(_) => BorrowUsePlaceClosure {
place: desc_place,
var_span,
is_single_var: true,
Expand Down Expand Up @@ -1140,7 +1143,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
var_span,
is_single_var: false,
},
hir::ClosureKind::Closure => {
hir::ClosureKind::Closure | hir::ClosureKind::CoroutineClosure(_) => {
BorrowUsePlaceClosure { place: desc_place, var_span, is_single_var: false }
}
}
Expand All @@ -1158,7 +1161,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
hir::ClosureKind::Coroutine(_) => {
FirstBorrowUsePlaceCoroutine { place: borrow_place_desc, var_span }
}
hir::ClosureKind::Closure => {
hir::ClosureKind::Closure | hir::ClosureKind::CoroutineClosure(_) => {
FirstBorrowUsePlaceClosure { place: borrow_place_desc, var_span }
}
}
Expand All @@ -1175,7 +1178,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
hir::ClosureKind::Coroutine(_) => {
SecondBorrowUsePlaceCoroutine { place: desc_place, var_span }
}
hir::ClosureKind::Closure => {
hir::ClosureKind::Closure | hir::ClosureKind::CoroutineClosure(_) => {
SecondBorrowUsePlaceClosure { place: desc_place, var_span }
}
}
Expand Down Expand Up @@ -2942,7 +2945,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
use crate::session_diagnostics::CaptureVarCause::*;
match kind {
hir::ClosureKind::Coroutine(_) => BorrowUseInCoroutine { var_span },
hir::ClosureKind::Closure => BorrowUseInClosure { var_span },
hir::ClosureKind::Closure | hir::ClosureKind::CoroutineClosure(_) => {
BorrowUseInClosure { var_span }
}
}
});

Expand All @@ -2958,7 +2963,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
use crate::session_diagnostics::CaptureVarCause::*;
match kind {
hir::ClosureKind::Coroutine(_) => BorrowUseInCoroutine { var_span },
hir::ClosureKind::Closure => BorrowUseInClosure { var_span },
hir::ClosureKind::Closure | hir::ClosureKind::CoroutineClosure(_) => {
BorrowUseInClosure { var_span }
}
}
});

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ impl UseSpans<'_> {
PartialAssignment => AssignPartInCoroutine { path_span },
});
}
hir::ClosureKind::Closure => {
hir::ClosureKind::Closure | hir::ClosureKind::CoroutineClosure(_) => {
err.subdiagnostic(match action {
Borrow => BorrowInClosure { path_span },
MatchOn | Use => UseInClosure { path_span },
Expand Down Expand Up @@ -1253,7 +1253,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
hir::ClosureKind::Coroutine(_) => {
CaptureVarCause::PartialMoveUseInCoroutine { var_span, is_partial }
}
hir::ClosureKind::Closure => {
hir::ClosureKind::Closure | hir::ClosureKind::CoroutineClosure(_) => {
CaptureVarCause::PartialMoveUseInClosure { var_span, is_partial }
}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1472,7 +1472,7 @@ fn suggest_ampmut<'tcx>(
}

fn is_closure_or_coroutine(ty: Ty<'_>) -> bool {
ty.is_closure() || ty.is_coroutine()
ty.is_closure() || ty.is_coroutine() || ty.is_coroutine_closure()
}

/// Given a field that needs to be mutable, returns a span where the " mut " could go.
Expand Down
36 changes: 23 additions & 13 deletions compiler/rustc_borrowck/src/diagnostics/region_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,31 +324,32 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> {
ty::BoundRegionKind::BrEnv => {
let def_ty = self.regioncx.universal_regions().defining_ty;

let DefiningTy::Closure(_, args) = def_ty else {
// Can't have BrEnv in functions, constants or coroutines.
bug!("BrEnv outside of closure.");
let closure_kind = match def_ty {
DefiningTy::Closure(_, args) => args.as_closure().kind(),
DefiningTy::CoroutineClosure(_, args) => args.as_coroutine_closure().kind(),
_ => {
// Can't have BrEnv in functions, constants or coroutines.
bug!("BrEnv outside of closure.");
}
};
let hir::ExprKind::Closure(&hir::Closure { fn_decl_span, .. }) =
tcx.hir().expect_expr(self.mir_hir_id()).kind
else {
bug!("Closure is not defined by a closure expr");
};
let region_name = self.synthesize_region_name();

let closure_kind_ty = args.as_closure().kind_ty();
let note = match closure_kind_ty.to_opt_closure_kind() {
Some(ty::ClosureKind::Fn) => {
let note = match closure_kind {
ty::ClosureKind::Fn => {
"closure implements `Fn`, so references to captured variables \
can't escape the closure"
}
Some(ty::ClosureKind::FnMut) => {
ty::ClosureKind::FnMut => {
"closure implements `FnMut`, so references to captured variables \
can't escape the closure"
}
Some(ty::ClosureKind::FnOnce) => {
ty::ClosureKind::FnOnce => {
bug!("BrEnv in a `FnOnce` closure");
}
None => bug!("Closure kind not inferred in borrow check"),
};

Some(RegionName {
Expand Down Expand Up @@ -692,7 +693,10 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> {
hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared(
hir::CoroutineDesugaring::Async,
hir::CoroutineSource::Closure,
)) => " of async closure",
))
| hir::ClosureKind::CoroutineClosure(hir::CoroutineDesugaring::Async) => {
" of async closure"
}

hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared(
hir::CoroutineDesugaring::Async,
Expand All @@ -719,7 +723,10 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> {
hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared(
hir::CoroutineDesugaring::Gen,
hir::CoroutineSource::Closure,
)) => " of gen closure",
))
| hir::ClosureKind::CoroutineClosure(hir::CoroutineDesugaring::Gen) => {
" of gen closure"
}

hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared(
hir::CoroutineDesugaring::Gen,
Expand All @@ -743,7 +750,10 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> {
hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared(
hir::CoroutineDesugaring::AsyncGen,
hir::CoroutineSource::Closure,
)) => " of async gen closure",
))
| hir::ClosureKind::CoroutineClosure(hir::CoroutineDesugaring::AsyncGen) => {
" of async gen closure"
}

hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared(
hir::CoroutineDesugaring::AsyncGen,
Expand Down
11 changes: 9 additions & 2 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#![allow(internal_features)]
#![feature(rustdoc_internals)]
#![doc(rust_logo)]
#![feature(assert_matches)]
#![feature(associated_type_bounds)]
#![feature(box_patterns)]
#![feature(let_chains)]
Expand Down Expand Up @@ -1303,7 +1304,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
// moved into the closure and subsequently used by the closure,
// in order to populate our used_mut set.
match **aggregate_kind {
AggregateKind::Closure(def_id, _) | AggregateKind::Coroutine(def_id, _) => {
AggregateKind::Closure(def_id, _)
| AggregateKind::CoroutineClosure(def_id, _)
| AggregateKind::Coroutine(def_id, _) => {
let def_id = def_id.expect_local();
let BorrowCheckResult { used_mut_upvars, .. } =
self.infcx.tcx.mir_borrowck(def_id);
Expand Down Expand Up @@ -1609,6 +1612,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
| ty::FnPtr(_)
| ty::Dynamic(_, _, _)
| ty::Closure(_, _)
| ty::CoroutineClosure(_, _)
| ty::Coroutine(_, _)
| ty::CoroutineWitness(..)
| ty::Never
Expand All @@ -1633,7 +1637,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
return;
}
}
ty::Closure(_, _) | ty::Coroutine(_, _) | ty::Tuple(_) => (),
ty::Closure(..)
| ty::CoroutineClosure(..)
| ty::Coroutine(_, _)
| ty::Tuple(_) => (),
ty::Bool
| ty::Char
| ty::Int(_)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/path_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ pub(crate) fn is_upvar_field_projection<'tcx>(
match place_ref.last_projection() {
Some((place_base, ProjectionElem::Field(field, _ty))) => {
let base_ty = place_base.ty(body, tcx).ty;
if (base_ty.is_closure() || base_ty.is_coroutine())
if (base_ty.is_closure() || base_ty.is_coroutine() || base_ty.is_coroutine_closure())
&& (!by_ref || upvars[field.index()].is_by_ref())
{
Some(field)
Expand Down
Loading

0 comments on commit 8d6080a

Please sign in to comment.