Skip to content

Commit

Permalink
Error on using yield without also using #[coroutine] on the closure
Browse files Browse the repository at this point in the history
And suggest adding the `#[coroutine]` to the closure
  • Loading branch information
oli-obk committed Apr 12, 2024
1 parent 2a3779b commit 37af228
Show file tree
Hide file tree
Showing 266 changed files with 1,228 additions and 829 deletions.
3 changes: 3 additions & 0 deletions compiler/rustc_ast_lowering/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -161,3 +161,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 @@ -414,3 +414,12 @@ pub(crate) struct AsyncBoundOnlyForFnTraits {
#[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>,
}
5 changes: 5 additions & 0 deletions 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 @@ -979,6 +980,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
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 @@ -1577,7 +1579,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 @@ -122,9 +123,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
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(generic_nonzero)]
#![feature(if_let_guard)]
#![feature(inline_const)]
Expand Down
82 changes: 43 additions & 39 deletions compiler/rustc_middle/src/ty/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,49 +421,53 @@ pub fn analyze_coroutine_closure_captures<'a, 'tcx: 'a, T>(
child_captures: impl IntoIterator<Item = &'a CapturedPlace<'tcx>>,
mut for_each: impl FnMut((usize, &'a CapturedPlace<'tcx>), (usize, &'a CapturedPlace<'tcx>)) -> T,
) -> impl Iterator<Item = T> + Captures<'a> + Captures<'tcx> {
std::iter::from_coroutine(move || {
let mut child_captures = child_captures.into_iter().enumerate().peekable();

// One parent capture may correspond to several child captures if we end up
// refining the set of captures via edition-2021 precise captures. We want to
// match up any number of child captures with one parent capture, so we keep
// peeking off this `Peekable` until the child doesn't match anymore.
for (parent_field_idx, parent_capture) in parent_captures.into_iter().enumerate() {
// Make sure we use every field at least once, b/c why are we capturing something
// if it's not used in the inner coroutine.
let mut field_used_at_least_once = false;

// A parent matches a child if they share the same prefix of projections.
// The child may have more, if it is capturing sub-fields out of
// something that is captured by-move in the parent closure.
while child_captures.peek().map_or(false, |(_, child_capture)| {
child_prefix_matches_parent_projections(parent_capture, child_capture)
}) {
let (child_field_idx, child_capture) = child_captures.next().unwrap();
// This analysis only makes sense if the parent capture is a
// prefix of the child capture.
assert!(
child_capture.place.projections.len() >= parent_capture.place.projections.len(),
"parent capture ({parent_capture:#?}) expected to be prefix of \
std::iter::from_coroutine(
#[cfg_attr(not(bootstrap), coroutine)]
move || {
let mut child_captures = child_captures.into_iter().enumerate().peekable();

// One parent capture may correspond to several child captures if we end up
// refining the set of captures via edition-2021 precise captures. We want to
// match up any number of child captures with one parent capture, so we keep
// peeking off this `Peekable` until the child doesn't match anymore.
for (parent_field_idx, parent_capture) in parent_captures.into_iter().enumerate() {
// Make sure we use every field at least once, b/c why are we capturing something
// if it's not used in the inner coroutine.
let mut field_used_at_least_once = false;

// A parent matches a child if they share the same prefix of projections.
// The child may have more, if it is capturing sub-fields out of
// something that is captured by-move in the parent closure.
while child_captures.peek().map_or(false, |(_, child_capture)| {
child_prefix_matches_parent_projections(parent_capture, child_capture)
}) {
let (child_field_idx, child_capture) = child_captures.next().unwrap();
// This analysis only makes sense if the parent capture is a
// prefix of the child capture.
assert!(
child_capture.place.projections.len()
>= parent_capture.place.projections.len(),
"parent capture ({parent_capture:#?}) expected to be prefix of \
child capture ({child_capture:#?})"
);
);

yield for_each(
(parent_field_idx, parent_capture),
(child_field_idx, child_capture),
);
yield for_each(
(parent_field_idx, parent_capture),
(child_field_idx, child_capture),
);

field_used_at_least_once = true;
}
field_used_at_least_once = true;
}

// Make sure the field was used at least once.
assert!(
field_used_at_least_once,
"we captured {parent_capture:#?} but it was not used in the child coroutine?"
);
}
assert_eq!(child_captures.next(), None, "leftover child captures?");
})
// Make sure the field was used at least once.
assert!(
field_used_at_least_once,
"we captured {parent_capture:#?} but it was not used in the child coroutine?"
);
}
assert_eq!(child_captures.next(), None, "leftover child captures?");
},
)
}

fn child_prefix_matches_parent_projections(
Expand Down
29 changes: 16 additions & 13 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1262,20 +1262,23 @@ impl<'tcx> TyCtxt<'tcx> {
self.dep_graph.read_index(DepNodeIndex::FOREVER_RED_NODE);

let definitions = &self.untracked.definitions;
std::iter::from_coroutine(|| {
let mut i = 0;

// Recompute the number of definitions each time, because our caller may be creating
// new ones.
while i < { definitions.read().num_definitions() } {
let local_def_index = rustc_span::def_id::DefIndex::from_usize(i);
yield LocalDefId { local_def_index };
i += 1;
}
std::iter::from_coroutine(
#[cfg_attr(not(bootstrap), coroutine)]
|| {
let mut i = 0;

// Recompute the number of definitions each time, because our caller may be creating
// new ones.
while i < { definitions.read().num_definitions() } {
let local_def_index = rustc_span::def_id::DefIndex::from_usize(i);
yield LocalDefId { local_def_index };
i += 1;
}

// Freeze definitions once we finish iterating on them, to prevent adding new ones.
definitions.freeze();
})
// Freeze definitions once we finish iterating on them, to prevent adding new ones.
definitions.freeze();
},
)
}

pub fn def_path_table(self) -> &'tcx rustc_hir::definitions::DefPathTable {
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/tests/ui/crashes/ice-5238.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// Regression test for #5238 / https://github.com/rust-lang/rust/pull/69562

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

fn main() {
let _ = || {
let _ = #[coroutine] || {
yield;
};
}
1 change: 0 additions & 1 deletion src/tools/clippy/tests/ui/large_futures.fixed
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#![feature(coroutines)]
#![warn(clippy::large_futures)]
#![allow(clippy::never_loop)]
#![allow(clippy::future_not_send)]
Expand Down
1 change: 0 additions & 1 deletion src/tools/clippy/tests/ui/large_futures.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#![feature(coroutines)]
#![warn(clippy::large_futures)]
#![allow(clippy::never_loop)]
#![allow(clippy::future_not_send)]
Expand Down
16 changes: 8 additions & 8 deletions src/tools/clippy/tests/ui/large_futures.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: large future with a size of 16385 bytes
--> tests/ui/large_futures.rs:11:9
--> tests/ui/large_futures.rs:10:9
|
LL | big_fut([0u8; 1024 * 16]).await;
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider `Box::pin` on it: `Box::pin(big_fut([0u8; 1024 * 16]))`
Expand All @@ -8,37 +8,37 @@ LL | big_fut([0u8; 1024 * 16]).await;
= help: to override `-D warnings` add `#[allow(clippy::large_futures)]`

error: large future with a size of 16386 bytes
--> tests/ui/large_futures.rs:15:5
--> tests/ui/large_futures.rs:14:5
|
LL | f.await
| ^ help: consider `Box::pin` on it: `Box::pin(f)`

error: large future with a size of 16387 bytes
--> tests/ui/large_futures.rs:20:9
--> tests/ui/large_futures.rs:19:9
|
LL | wait().await;
| ^^^^^^ help: consider `Box::pin` on it: `Box::pin(wait())`

error: large future with a size of 16387 bytes
--> tests/ui/large_futures.rs:25:13
--> tests/ui/large_futures.rs:24:13
|
LL | wait().await;
| ^^^^^^ help: consider `Box::pin` on it: `Box::pin(wait())`

error: large future with a size of 65540 bytes
--> tests/ui/large_futures.rs:33:5
--> tests/ui/large_futures.rs:32:5
|
LL | foo().await;
| ^^^^^ help: consider `Box::pin` on it: `Box::pin(foo())`

error: large future with a size of 49159 bytes
--> tests/ui/large_futures.rs:35:5
--> tests/ui/large_futures.rs:34:5
|
LL | calls_fut(fut).await;
| ^^^^^^^^^^^^^^ help: consider `Box::pin` on it: `Box::pin(calls_fut(fut))`

error: large future with a size of 65540 bytes
--> tests/ui/large_futures.rs:48:5
--> tests/ui/large_futures.rs:47:5
|
LL | / async {
LL | |
Expand All @@ -59,7 +59,7 @@ LL + })
|

error: large future with a size of 65540 bytes
--> tests/ui/large_futures.rs:60:13
--> tests/ui/large_futures.rs:59:13
|
LL | / async {
LL | | let x = [0i32; 1024 * 16];
Expand Down
Loading

0 comments on commit 37af228

Please sign in to comment.