Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 12 commits into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading