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

Require explicitly marking closures as coroutines #123792

Merged
merged 2 commits into from
Apr 24, 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: 3 additions & 0 deletions compiler/rustc_ast_lowering/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -163,3 +163,6 @@ ast_lowering_underscore_expr_lhs_assign =
.label = `_` not allowed here

ast_lowering_use_angle_brackets = use angle brackets instead
ast_lowering_yield_in_closure =
`yield` can only be used in `#[coroutine]` closures, or `gen` blocks
.suggestion = use `#[coroutine]` to make this closure a coroutine
9 changes: 9 additions & 0 deletions compiler/rustc_ast_lowering/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,3 +421,12 @@ pub(crate) struct NoPreciseCapturesOnApit {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(ast_lowering_yield_in_closure)]
pub(crate) struct YieldInClosure {
#[primary_span]
pub span: Span,
#[suggestion(code = "#[coroutine] ", applicability = "maybe-incorrect", style = "verbose")]
pub suggestion: Option<Span>,
}
17 changes: 16 additions & 1 deletion compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use super::errors::{
};
use super::ResolverAstLoweringExt;
use super::{ImplTraitContext, LoweringContext, ParamMode, ParenthesizedGenericArgs};
use crate::errors::YieldInClosure;
use crate::{FnDeclKind, ImplTraitPosition};
use rustc_ast::ptr::P as AstP;
use rustc_ast::*;
Expand Down Expand Up @@ -217,6 +218,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
binder,
*capture_clause,
e.id,
hir_id,
*constness,
*movability,
fn_decl,
Expand Down Expand Up @@ -955,6 +957,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
binder: &ClosureBinder,
capture_clause: CaptureBy,
closure_id: NodeId,
closure_hir_id: hir::HirId,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the same id as the closure_id below (but lowered node id -> hir id?) it feels weird and redundant to pass both in, unless one is distinct

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea it is. I copied this duplicate passing from lower_expr_coroutine_closure. I think we can move to using HirId everywhere, but it's a bit of an invasive refactoring

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, let's not do that here

constness: Const,
movability: Movability,
decl: &FnDecl,
Expand All @@ -965,8 +968,17 @@ impl<'hir> LoweringContext<'_, 'hir> {
let (binder_clause, generic_params) = self.lower_closure_binder(binder);

let (body_id, closure_kind) = self.with_new_scopes(fn_decl_span, move |this| {
let mut coroutine_kind = None;
let mut coroutine_kind = if this
.attrs
.get(&closure_hir_id.local_id)
.is_some_and(|attrs| attrs.iter().any(|attr| attr.has_name(sym::coroutine)))
{
Some(hir::CoroutineKind::Coroutine(Movability::Movable))
} else {
None
};
let body_id = this.lower_fn_body(decl, |this| {
this.coroutine_kind = coroutine_kind;
let e = this.lower_expr_mut(body);
coroutine_kind = this.coroutine_kind;
e
Expand Down Expand Up @@ -1565,7 +1577,10 @@ impl<'hir> LoweringContext<'_, 'hir> {
)
.emit();
}
let suggestion = self.current_item.map(|s| s.shrink_to_lo());
self.dcx().emit_err(YieldInClosure { span, suggestion });
self.coroutine_kind = Some(hir::CoroutineKind::Coroutine(Movability::Movable));

false
}
};
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
body,
..
}) => {
self.with_new_scopes(ident.span, |this| {
self.with_new_scopes(*fn_sig_span, |this| {
// Note: we don't need to change the return type from `T` to
// `impl Future<Output = T>` here because lower_body
// only cares about the input argument patterns in the function
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![feature(coroutines, coroutine_trait)]
#![feature(coroutines, coroutine_trait, stmt_expr_attributes)]

use std::ops::Coroutine;
use std::pin::Pin;
Expand All @@ -8,7 +8,8 @@ fn main() {
}

fn run_coroutine<T>() {
let mut coroutine = || {
let mut coroutine = #[coroutine]
|| {
yield;
return;
};
Expand Down
10 changes: 7 additions & 3 deletions compiler/rustc_codegen_cranelift/example/std_example.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![feature(
core_intrinsics,
coroutines,
stmt_expr_attributes,
coroutine_trait,
is_sorted,
repr_simd,
Expand Down Expand Up @@ -123,9 +124,12 @@ fn main() {
test_simd();
}

Box::pin(move |mut _task_context| {
yield ();
})
Box::pin(
#[coroutine]
move |mut _task_context| {
yield ();
},
)
.as_mut()
.resume(0);

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_gcc/example/std_example.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#![allow(internal_features)]
#![feature(core_intrinsics, coroutines, coroutine_trait, is_sorted)]
#![feature(core_intrinsics, coroutines, coroutine_trait, is_sorted, stmt_expr_attributes)]

#[cfg(feature="master")]
#[cfg(target_arch="x86_64")]
Expand Down Expand Up @@ -103,7 +103,7 @@ fn main() {
test_simd();
}

Box::pin(move |mut _task_context| {
Box::pin(#[coroutine] move |mut _task_context| {
yield ();
}).as_mut().resume(0);

Expand Down
20 changes: 10 additions & 10 deletions compiler/rustc_error_codes/src/error_codes/E0626.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ yield point.
Erroneous code example:

```compile_fail,E0626
# #![feature(coroutines, coroutine_trait, pin)]
# #![feature(coroutines, coroutine_trait, stmt_expr_attributes)]
# use std::ops::Coroutine;
# use std::pin::Pin;
let mut b = || {
let mut b = #[coroutine] || {
let a = &String::new(); // <-- This borrow...
yield (); // ...is still in scope here, when the yield occurs.
println!("{}", a);
Expand All @@ -23,10 +23,10 @@ resolve the previous example by removing the borrow and just storing
the integer by value:

```
# #![feature(coroutines, coroutine_trait, pin)]
# #![feature(coroutines, coroutine_trait, stmt_expr_attributes)]
# use std::ops::Coroutine;
# use std::pin::Pin;
let mut b = || {
let mut b = #[coroutine] || {
let a = 3;
yield ();
println!("{}", a);
Expand All @@ -41,10 +41,10 @@ in those cases, something like the `Rc` or `Arc` types may be useful.
This error also frequently arises with iteration:

```compile_fail,E0626
# #![feature(coroutines, coroutine_trait, pin)]
# #![feature(coroutines, coroutine_trait, stmt_expr_attributes)]
# use std::ops::Coroutine;
# use std::pin::Pin;
let mut b = || {
let mut b = #[coroutine] || {
let v = vec![1,2,3];
for &x in &v { // <-- borrow of `v` is still in scope...
yield x; // ...when this yield occurs.
Expand All @@ -57,10 +57,10 @@ Such cases can sometimes be resolved by iterating "by value" (or using
`into_iter()`) to avoid borrowing:

```
# #![feature(coroutines, coroutine_trait, pin)]
# #![feature(coroutines, coroutine_trait, stmt_expr_attributes)]
# use std::ops::Coroutine;
# use std::pin::Pin;
let mut b = || {
let mut b = #[coroutine] || {
let v = vec![1,2,3];
for x in v { // <-- Take ownership of the values instead!
yield x; // <-- Now yield is OK.
Expand All @@ -72,10 +72,10 @@ Pin::new(&mut b).resume(());
If taking ownership is not an option, using indices can work too:

```
# #![feature(coroutines, coroutine_trait, pin)]
# #![feature(coroutines, coroutine_trait, stmt_expr_attributes)]
# use std::ops::Coroutine;
# use std::pin::Pin;
let mut b = || {
let mut b = #[coroutine] || {
let v = vec![1,2,3];
let len = v.len(); // (*)
for i in 0..len {
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_error_codes/src/error_codes/E0627.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ A yield expression was used outside of the coroutine literal.
Erroneous code example:

```compile_fail,E0627
#![feature(coroutines, coroutine_trait)]
#![feature(coroutines, coroutine_trait, stmt_expr_attributes)]

fn fake_coroutine() -> &'static str {
yield 1;
Expand All @@ -19,10 +19,10 @@ The error occurs because keyword `yield` can only be used inside the coroutine
literal. This can be fixed by constructing the coroutine correctly.

```
#![feature(coroutines, coroutine_trait)]
#![feature(coroutines, coroutine_trait, stmt_expr_attributes)]

fn main() {
let mut coroutine = || {
let mut coroutine = #[coroutine] || {
yield 1;
return "foo"
};
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_error_codes/src/error_codes/E0628.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ More than one parameter was used for a coroutine.
Erroneous code example:

```compile_fail,E0628
#![feature(coroutines, coroutine_trait)]
#![feature(coroutines, coroutine_trait, stmt_expr_attributes)]

fn main() {
let coroutine = |a: i32, b: i32| {
let coroutine = #[coroutine] |a: i32, b: i32| {
// error: too many parameters for a coroutine
// Allowed only 0 or 1 parameter
yield a;
Expand All @@ -20,10 +20,10 @@ at most 1 parameter for the coroutine. For example, we might resolve
the previous example by passing only one parameter.

```
#![feature(coroutines, coroutine_trait)]
#![feature(coroutines, coroutine_trait, stmt_expr_attributes)]

fn main() {
let coroutine = |a: i32| {
let coroutine = #[coroutine] |a: i32| {
yield a;
};
}
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_error_codes/src/error_codes/E0727.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ A `yield` clause was used in an `async` context.
Erroneous code example:

```compile_fail,E0727,edition2018
#![feature(coroutines)]
#![feature(coroutines, stmt_expr_attributes)]

fn main() {
let coroutine = || {
let coroutine = #[coroutine] || {
async {
yield;
}
Expand All @@ -20,10 +20,10 @@ which is not yet supported.
To fix this error, you have to move `yield` out of the `async` block:

```edition2018
#![feature(coroutines)]
#![feature(coroutines, stmt_expr_attributes)]

fn main() {
let coroutine = || {
let coroutine = #[coroutine] || {
yield;
};
}
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,12 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
EncodeCrossCrate::Yes, experimental!(cfi_encoding)
),

// `#[coroutine]` attribute to be applied to closures to make them coroutines instead
gated!(
coroutine, Normal, template!(Word), ErrorFollowing,
EncodeCrossCrate::No, coroutines, experimental!(coroutines)
),

// ==========================================================================
// Internal attributes: Stability, deprecation, and unsafe:
// ==========================================================================
Expand Down
42 changes: 23 additions & 19 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1260,30 +1260,34 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
id: DefIndex,
sess: &'a Session,
) -> impl Iterator<Item = ModChild> + 'a {
iter::from_coroutine(move || {
if let Some(data) = &self.root.proc_macro_data {
// If we are loading as a proc macro, we want to return
// the view of this crate as a proc macro crate.
if id == CRATE_DEF_INDEX {
for child_index in data.macros.decode(self) {
iter::from_coroutine(
#[cfg_attr(not(bootstrap), coroutine)]
move || {
if let Some(data) = &self.root.proc_macro_data {
// If we are loading as a proc macro, we want to return
// the view of this crate as a proc macro crate.
if id == CRATE_DEF_INDEX {
for child_index in data.macros.decode(self) {
yield self.get_mod_child(child_index, sess);
}
}
} else {
// Iterate over all children.
let non_reexports =
self.root.tables.module_children_non_reexports.get(self, id);
for child_index in non_reexports.unwrap().decode(self) {
yield self.get_mod_child(child_index, sess);
}
}
} else {
// Iterate over all children.
let non_reexports = self.root.tables.module_children_non_reexports.get(self, id);
for child_index in non_reexports.unwrap().decode(self) {
yield self.get_mod_child(child_index, sess);
}

let reexports = self.root.tables.module_children_reexports.get(self, id);
if !reexports.is_default() {
for reexport in reexports.decode((self, sess)) {
yield reexport;
let reexports = self.root.tables.module_children_reexports.get(self, id);
if !reexports.is_default() {
for reexport in reexports.decode((self, sess)) {
yield reexport;
}
}
}
}
})
},
)
}

fn is_ctfe_mir_available(self, id: DefIndex) -> bool {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#![feature(const_type_name)]
#![feature(discriminant_kind)]
#![feature(coroutines)]
#![feature(stmt_expr_attributes)]
#![feature(if_let_guard)]
#![feature(inline_const)]
#![feature(iter_from_coroutine)]
Expand Down
Loading
Loading