From 1dd02e32a360f64c6fedbd5f1eecaced9f9f08a1 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 29 Jan 2022 11:44:30 -0800 Subject: [PATCH 1/3] Add regression test for issue 93274 Currently fails with: error: future cannot be sent between threads safely --> $DIR/src/test/ui/fmt/format-with-yield-point.rs:21:17 | LL | assert_send(with_await()); | ^^^^^^^^^^^^ future returned by `with_await` is not `Send` | = help: the trait `Sync` is not implemented for `core::fmt::Opaque` note: future is not `Send` as this value is used across an await --> $DIR/src/test/ui/fmt/format-with-yield-point.rs:11:37 | LL | println!("{} {:?}", "", async {}.await); | --------------------------------^^^^^^- | | | | | await occurs here, with `$crate::format_args_nl!($($arg)*)` maybe used later | has type `ArgumentV1<'_>` which is not `Send` | `$crate::format_args_nl!($($arg)*)` is later dropped here note: required by a bound in `assert_send` --> $DIR/src/test/ui/fmt/format-with-yield-point.rs:18:24 | LL | fn assert_send(_: impl Send) {} | ^^^^ required by this bound in `assert_send` error: future cannot be sent between threads safely --> $DIR/src/test/ui/fmt/format-with-yield-point.rs:22:17 | LL | assert_send(with_macro_call()); | ^^^^^^^^^^^^^^^^^ future returned by `with_macro_call` is not `Send` | = help: the trait `Sync` is not implemented for `core::fmt::Opaque` note: future is not `Send` as this value is used across an await --> $DIR/src/test/ui/fmt/format-with-yield-point.rs:6:17 | LL | async {}.await | ^^^^^^ await occurs here, with `$crate::format_args_nl!($($arg)*)` maybe used later ... LL | println!("{} {:?}", "", m!()); | ----------------------------- | | | | | in this macro invocation | has type `ArgumentV1<'_>` which is not `Send` | `$crate::format_args_nl!($($arg)*)` is later dropped here note: required by a bound in `assert_send` --> $DIR/src/test/ui/fmt/format-with-yield-point.rs:18:24 | LL | fn assert_send(_: impl Send) {} | ^^^^ required by this bound in `assert_send` = note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info) error: aborting due to 2 previous errors --- src/test/ui/fmt/format-with-yield-point.rs | 23 ++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 src/test/ui/fmt/format-with-yield-point.rs diff --git a/src/test/ui/fmt/format-with-yield-point.rs b/src/test/ui/fmt/format-with-yield-point.rs new file mode 100644 index 0000000000000..0bd9062f95e78 --- /dev/null +++ b/src/test/ui/fmt/format-with-yield-point.rs @@ -0,0 +1,23 @@ +// check-pass +// edition:2021 + +macro_rules! m { + () => { + async {}.await + }; +} + +async fn with_await() { + println!("{} {:?}", "", async {}.await); +} + +async fn with_macro_call() { + println!("{} {:?}", "", m!()); +} + +fn assert_send(_: impl Send) {} + +fn main() { + assert_send(with_await()); + assert_send(with_macro_call()); +} From 47f92a58a412fed5555eb120f963bcf6a35a9da8 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 29 Jan 2022 12:05:19 -0800 Subject: [PATCH 2/3] Accommodate yield points in the format_args expansion --- compiler/rustc_builtin_macros/src/format.rs | 53 +++++++++++++++++++-- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index bdc92a2c096a4..d1696816666a2 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -4,6 +4,7 @@ use Position::*; use rustc_ast as ast; use rustc_ast::ptr::P; use rustc_ast::tokenstream::TokenStream; +use rustc_ast::visit::{self, Visitor}; use rustc_ast::{token, BlockCheckMode, UnsafeSource}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::{pluralize, Applicability, DiagnosticBuilder}; @@ -788,17 +789,31 @@ impl<'a, 'b> Context<'a, 'b> { // the order provided to fmt::Arguments. When arguments are repeated, we // want the expression evaluated only once. // - // Thus in the not nicely ordered case we emit the following instead: + // Further, if any arg _after the first one_ contains a yield point such + // as `await` or `yield`, the above short form is inconvenient for the + // caller because it would keep a temporary of type ArgumentV1 alive + // across the yield point. ArgumentV1 can't implement Send since it + // holds a type-erased arbitrary type. + // + // Thus in the not nicely ordered case, and in the yielding case, we + // emit the following instead: // // match (&$arg0, &$arg1, …) { // args => [ArgumentV1::new(args.$i, …), ArgumentV1::new(args.$j, …), …] // } // // for the sequence of indices $i, $j, … governed by fmt_arg_index_and_ty. + // This more verbose representation ensures that all arguments are + // evaluated a single time each, in the order written by the programmer, + // and that the surrounding future/generator (if any) is Send whenever + // possible. + let no_need_for_match = + nicely_ordered && !original_args.iter().skip(1).any(|e| may_contain_yield_point(e)); + for (arg_index, arg_ty) in fmt_arg_index_and_ty { let e = &mut original_args[arg_index]; let span = e.span; - let arg = if nicely_ordered { + let arg = if no_need_for_match { let expansion_span = e.span.with_ctxt(self.macsp.ctxt()); // The indices are strictly ordered so e has not been taken yet. self.ecx.expr_addr_of(expansion_span, P(e.take())) @@ -814,10 +829,10 @@ impl<'a, 'b> Context<'a, 'b> { let args_array = self.ecx.expr_vec(self.macsp, fmt_args); let args_slice = self.ecx.expr_addr_of( self.macsp, - if nicely_ordered { + if no_need_for_match { args_array } else { - // In the !nicely_ordered case, none of the exprs were moved + // In the !no_need_for_match case, none of the exprs were moved // away in the previous loop. // // This uses the arg span for `&arg` so that borrowck errors @@ -1216,3 +1231,33 @@ pub fn expand_preparsed_format_args( cx.into_expr() } + +fn may_contain_yield_point(e: &ast::Expr) -> bool { + struct MayContainYieldPoint(bool); + + impl Visitor<'_> for MayContainYieldPoint { + fn visit_expr(&mut self, e: &ast::Expr) { + if let ast::ExprKind::Await(_) | ast::ExprKind::Yield(_) | ast::ExprKind::MacCall(_) = + e.kind + { + self.0 = true; + } else { + visit::walk_expr(self, e); + } + } + + fn visit_attribute(&mut self, _: &ast::Attribute) { + // Conservatively assume this may be a proc macro attribute in + // expression position. + self.0 = true; + } + + fn visit_item(&mut self, _: &ast::Item) { + // Do not recurse into nested items. + } + } + + let mut visitor = MayContainYieldPoint(false); + visitor.visit_expr(e); + visitor.0 +} From 858d6a071199b30ea95c744895f61f93ad188ffb Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 29 Jan 2022 22:16:35 -0800 Subject: [PATCH 3/3] Mac calls --- compiler/rustc_builtin_macros/src/format.rs | 8 +++++--- src/test/ui/fmt/format-with-yield-point.rs | 14 ++++++++++++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index d1696816666a2..f08aa8f95ff4c 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -1237,15 +1237,17 @@ fn may_contain_yield_point(e: &ast::Expr) -> bool { impl Visitor<'_> for MayContainYieldPoint { fn visit_expr(&mut self, e: &ast::Expr) { - if let ast::ExprKind::Await(_) | ast::ExprKind::Yield(_) | ast::ExprKind::MacCall(_) = - e.kind - { + if let ast::ExprKind::Await(_) | ast::ExprKind::Yield(_) = e.kind { self.0 = true; } else { visit::walk_expr(self, e); } } + fn visit_mac_call(&mut self, _: &ast::MacCall) { + self.0 = true; + } + fn visit_attribute(&mut self, _: &ast::Attribute) { // Conservatively assume this may be a proc macro attribute in // expression position. diff --git a/src/test/ui/fmt/format-with-yield-point.rs b/src/test/ui/fmt/format-with-yield-point.rs index 0bd9062f95e78..e484074cc9a55 100644 --- a/src/test/ui/fmt/format-with-yield-point.rs +++ b/src/test/ui/fmt/format-with-yield-point.rs @@ -11,13 +11,23 @@ async fn with_await() { println!("{} {:?}", "", async {}.await); } -async fn with_macro_call() { +async fn with_macro_call_expr() { println!("{} {:?}", "", m!()); } +async fn with_macro_call_stmt_semi() { + println!("{} {:?}", "", { m!(); }); +} + +async fn with_macro_call_stmt_braced() { + println!("{} {:?}", "", { m!{} }); +} + fn assert_send(_: impl Send) {} fn main() { assert_send(with_await()); - assert_send(with_macro_call()); + assert_send(with_macro_call_expr()); + assert_send(with_macro_call_stmt_semi()); + assert_send(with_macro_call_stmt_braced()); }