Skip to content

Commit

Permalink
Auto merge of #119258 - compiler-errors:closure-kind, r=eholk
Browse files Browse the repository at this point in the history 
Make closures carry their own ClosureKind

Right now, we use the "`movability`" field of `hir::Closure` to distinguish a closure and a coroutine. This is paired together with the `CoroutineKind`, which is located not in the `hir::Closure`, but the `hir::Body`. This is strange and redundant.

This PR introduces `ClosureKind` with two variants -- `Closure` and `Coroutine`, which is put into `hir::Closure`. The `CoroutineKind` is thus removed from `hir::Body`, and `Option<Movability>` no longer needs to be a stand-in for "is this a closure or a coroutine".

r? eholk
  • Loading branch information
@bors
bors committed Dec 26, 2023
2 parents 2271c26 + ba91285 commit 1ab7831
Show file tree
Hide file tree
Showing 47 changed files with 570 additions and 488 deletions.
65 changes: 38 additions & 27 deletions compiler/rustc_ast_lowering/src/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -668,11 +668,10 @@ impl<'hir> LoweringContext<'_, 'hir> {
};
let params = arena_vec![self; param];

let coroutine_kind =
hir::CoroutineKind::Desugared(hir::CoroutineDesugaring::Async, async_coroutine_source);
let body = self.lower_body(move |this| {
this.coroutine_kind = Some(hir::CoroutineKind::Desugared(
hir::CoroutineDesugaring::Async,
async_coroutine_source,
));
this.coroutine_kind = Some(coroutine_kind);

let old_ctx = this.task_context;
this.task_context = Some(task_context_hid);
Expand All @@ -691,7 +690,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
body,
fn_decl_span: self.lower_span(span),
fn_arg_span: None,
movability: Some(hir::Movability::Static),
kind: hir::ClosureKind::Coroutine(coroutine_kind),
constness: hir::Constness::NotConst,
}))
}
Expand Down Expand Up @@ -725,11 +724,10 @@ impl<'hir> LoweringContext<'_, 'hir> {
lifetime_elision_allowed: false,
});

let coroutine_kind =
hir::CoroutineKind::Desugared(hir::CoroutineDesugaring::Gen, coroutine_source);
let body = self.lower_body(move |this| {
this.coroutine_kind = Some(hir::CoroutineKind::Desugared(
hir::CoroutineDesugaring::Gen,
coroutine_source,
));
this.coroutine_kind = Some(coroutine_kind);

let res = body(this);
(&[], res)
Expand All @@ -745,7 +743,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
body,
fn_decl_span: self.lower_span(span),
fn_arg_span: None,
movability: Some(Movability::Movable),
kind: hir::ClosureKind::Coroutine(coroutine_kind),
constness: hir::Constness::NotConst,
}))
}
Expand Down Expand Up @@ -806,11 +804,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
};
let params = arena_vec![self; param];

let coroutine_kind = hir::CoroutineKind::Desugared(
hir::CoroutineDesugaring::AsyncGen,
async_coroutine_source,
);
let body = self.lower_body(move |this| {
this.coroutine_kind = Some(hir::CoroutineKind::Desugared(
hir::CoroutineDesugaring::AsyncGen,
async_coroutine_source,
));
this.coroutine_kind = Some(coroutine_kind);

let old_ctx = this.task_context;
this.task_context = Some(task_context_hid);
Expand All @@ -829,7 +828,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
body,
fn_decl_span: self.lower_span(span),
fn_arg_span: None,
movability: Some(hir::Movability::Static),
kind: hir::ClosureKind::Coroutine(coroutine_kind),
constness: hir::Constness::NotConst,
}))
}
Expand Down Expand Up @@ -898,7 +897,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
let is_async_gen = match self.coroutine_kind {
Some(hir::CoroutineKind::Desugared(hir::CoroutineDesugaring::Async, _)) => false,
Some(hir::CoroutineKind::Desugared(hir::CoroutineDesugaring::AsyncGen, _)) => true,
Some(hir::CoroutineKind::Coroutine)
Some(hir::CoroutineKind::Coroutine(_))
| Some(hir::CoroutineKind::Desugared(hir::CoroutineDesugaring::Gen, _))
| None => {
return hir::ExprKind::Err(self.dcx().emit_err(AwaitOnlyInAsyncFnAndBlocks {
Expand Down Expand Up @@ -1086,15 +1085,15 @@ impl<'hir> LoweringContext<'_, 'hir> {
) -> hir::ExprKind<'hir> {
let (binder_clause, generic_params) = self.lower_closure_binder(binder);

let (body_id, coroutine_option) = self.with_new_scopes(fn_decl_span, move |this| {
let (body_id, closure_kind) = self.with_new_scopes(fn_decl_span, move |this| {
let mut coroutine_kind = None;
let body_id = this.lower_fn_body(decl, |this| {
let e = this.lower_expr_mut(body);
coroutine_kind = this.coroutine_kind;
e
});
let coroutine_option =
this.coroutine_movability_for_fn(decl, fn_decl_span, coroutine_kind, movability);
this.closure_movability_for_fn(decl, fn_decl_span, coroutine_kind, movability);
(body_id, coroutine_option)
});

Expand All @@ -1111,26 +1110,26 @@ impl<'hir> LoweringContext<'_, 'hir> {
body: body_id,
fn_decl_span: self.lower_span(fn_decl_span),
fn_arg_span: Some(self.lower_span(fn_arg_span)),
movability: coroutine_option,
kind: closure_kind,
constness: self.lower_constness(constness),
});

hir::ExprKind::Closure(c)
}

fn coroutine_movability_for_fn(
fn closure_movability_for_fn(
&mut self,
decl: &FnDecl,
fn_decl_span: Span,
coroutine_kind: Option<hir::CoroutineKind>,
movability: Movability,
) -> Option<hir::Movability> {
) -> hir::ClosureKind {
match coroutine_kind {
Some(hir::CoroutineKind::Coroutine) => {
Some(hir::CoroutineKind::Coroutine(_)) => {
if decl.inputs.len() > 1 {
self.dcx().emit_err(CoroutineTooManyParameters { fn_decl_span });
}
Some(movability)
hir::ClosureKind::Coroutine(hir::CoroutineKind::Coroutine(movability))
}
Some(
hir::CoroutineKind::Desugared(hir::CoroutineDesugaring::Gen, _)
Expand All @@ -1143,7 +1142,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
if movability == Movability::Static {
self.dcx().emit_err(ClosureCannotBeStatic { fn_decl_span });
}
None
hir::ClosureKind::Closure
}
}
}
Expand Down Expand Up @@ -1235,7 +1234,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
body,
fn_decl_span: self.lower_span(fn_decl_span),
fn_arg_span: Some(self.lower_span(fn_arg_span)),
movability: None,
kind: hir::ClosureKind::Closure,
constness: hir::Constness::NotConst,
});
hir::ExprKind::Closure(c)
Expand Down Expand Up @@ -1655,7 +1654,19 @@ impl<'hir> LoweringContext<'_, 'hir> {
self.dcx().emit_err(AsyncCoroutinesNotSupported { span }),
);
}
Some(hir::CoroutineKind::Coroutine) | None => {
Some(hir::CoroutineKind::Coroutine(_)) => {
if !self.tcx.features().coroutines {
rustc_session::parse::feature_err(
&self.tcx.sess.parse_sess,
sym::coroutines,
span,
"yield syntax is experimental",
)
.emit();
}
false
}
None => {
if !self.tcx.features().coroutines {
rustc_session::parse::feature_err(
&self.tcx.sess.parse_sess,
Expand All @@ -1665,7 +1676,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
)
.emit();
}
self.coroutine_kind = Some(hir::CoroutineKind::Coroutine);
self.coroutine_kind = Some(hir::CoroutineKind::Coroutine(Movability::Movable));
false
}
};
Expand Down
6 changes: 1 addition & 5 deletions compiler/rustc_ast_lowering/src/item.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -952,11 +952,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
params: &'hir [hir::Param<'hir>],
value: hir::Expr<'hir>,
) -> hir::BodyId {
let body = hir::Body {
coroutine_kind: self.coroutine_kind,
params,
value: self.arena.alloc(value),
};
let body = hir::Body { params, value: self.arena.alloc(value) };
let id = body.id();
debug_assert_eq!(id.hir_id.owner, self.current_hir_id_owner);
self.bodies.push((id.hir_id.local_id, self.arena.alloc(body)));
Expand Down
50 changes: 31 additions & 19 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -848,8 +848,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
move_spans.var_subdiag(None, &mut err, None, |kind, var_span| {
use crate::session_diagnostics::CaptureVarCause::*;
match kind {
Some(_) => MoveUseInCoroutine { var_span },
None => MoveUseInClosure { var_span },
hir::ClosureKind::Coroutine(_) => MoveUseInCoroutine { var_span },
hir::ClosureKind::Closure => MoveUseInClosure { var_span },
}
});

Expand Down Expand Up @@ -893,10 +893,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let place = &borrow.borrowed_place;
let desc_place = self.describe_any_place(place.as_ref());
match kind {
Some(_) => {
hir::ClosureKind::Coroutine(_) => {
BorrowUsePlaceCoroutine { place: desc_place, var_span, is_single_var: true }
}
None => BorrowUsePlaceClosure { place: desc_place, var_span, is_single_var: true },
hir::ClosureKind::Closure => {
BorrowUsePlaceClosure { place: desc_place, var_span, is_single_var: true }
}
}
});

Expand Down Expand Up @@ -1040,12 +1042,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
|kind, var_span| {
use crate::session_diagnostics::CaptureVarCause::*;
match kind {
Some(_) => BorrowUsePlaceCoroutine {
hir::ClosureKind::Coroutine(_) => BorrowUsePlaceCoroutine {
place: desc_place,
var_span,
is_single_var: true,
},
None => BorrowUsePlaceClosure {
hir::ClosureKind::Closure => BorrowUsePlaceClosure {
place: desc_place,
var_span,
is_single_var: true,
Expand Down Expand Up @@ -1124,12 +1126,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
borrow_spans.var_subdiag(None, &mut err, Some(gen_borrow_kind), |kind, var_span| {
use crate::session_diagnostics::CaptureVarCause::*;
match kind {
Some(_) => BorrowUsePlaceCoroutine {
hir::ClosureKind::Coroutine(_) => BorrowUsePlaceCoroutine {
place: desc_place,
var_span,
is_single_var: false,
},
None => {
hir::ClosureKind::Closure => {
BorrowUsePlaceClosure { place: desc_place, var_span, is_single_var: false }
}
}
Expand All @@ -1144,10 +1146,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let borrow_place = &issued_borrow.borrowed_place;
let borrow_place_desc = self.describe_any_place(borrow_place.as_ref());
match kind {
Some(_) => {
hir::ClosureKind::Coroutine(_) => {
FirstBorrowUsePlaceCoroutine { place: borrow_place_desc, var_span }
}
None => FirstBorrowUsePlaceClosure { place: borrow_place_desc, var_span },
hir::ClosureKind::Closure => {
FirstBorrowUsePlaceClosure { place: borrow_place_desc, var_span }
}
}
},
);
Expand All @@ -1159,8 +1163,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
|kind, var_span| {
use crate::session_diagnostics::CaptureVarCause::*;
match kind {
Some(_) => SecondBorrowUsePlaceCoroutine { place: desc_place, var_span },
None => SecondBorrowUsePlaceClosure { place: desc_place, var_span },
hir::ClosureKind::Coroutine(_) => {
SecondBorrowUsePlaceCoroutine { place: desc_place, var_span }
}
hir::ClosureKind::Closure => {
SecondBorrowUsePlaceClosure { place: desc_place, var_span }
}
}
},
);
Expand Down Expand Up @@ -1651,7 +1659,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
fn visit_expr(&mut self, e: &'hir hir::Expr<'hir>) {
if e.span.contains(self.capture_span) {
if let hir::ExprKind::Closure(&hir::Closure {
movability: None,
kind: hir::ClosureKind::Closure,
body,
fn_arg_span,
fn_decl: hir::FnDecl { inputs, .. },
Expand Down Expand Up @@ -1686,7 +1694,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
&& let Some(init) = local.init
{
if let hir::Expr {
kind: hir::ExprKind::Closure(&hir::Closure { movability: None, .. }),
kind:
hir::ExprKind::Closure(&hir::Closure {
kind: hir::ClosureKind::Closure,
..
}),
..
} = init
&& init.span.contains(self.capture_span)
Expand Down Expand Up @@ -2537,7 +2549,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}
}
CoroutineKind::Coroutine => "coroutine",
CoroutineKind::Coroutine(_) => "coroutine",
},
None => "closure",
};
Expand Down Expand Up @@ -2838,8 +2850,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
loan_spans.var_subdiag(None, &mut err, Some(loan.kind), |kind, var_span| {
use crate::session_diagnostics::CaptureVarCause::*;
match kind {
Some(_) => BorrowUseInCoroutine { var_span },
None => BorrowUseInClosure { var_span },
hir::ClosureKind::Coroutine(_) => BorrowUseInCoroutine { var_span },
hir::ClosureKind::Closure => BorrowUseInClosure { var_span },
}
});

Expand All @@ -2854,8 +2866,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
loan_spans.var_subdiag(None, &mut err, Some(loan.kind), |kind, var_span| {
use crate::session_diagnostics::CaptureVarCause::*;
match kind {
Some(_) => BorrowUseInCoroutine { var_span },
None => BorrowUseInClosure { var_span },
hir::ClosureKind::Coroutine(_) => BorrowUseInCoroutine { var_span },
hir::ClosureKind::Closure => BorrowUseInClosure { var_span },
}
});

Expand Down
Loading

0 comments on commit 1ab7831

Please sign in to comment.