diff --git a/compiler/rustc_typeck/src/check/generator_interior.rs b/compiler/rustc_typeck/src/check/generator_interior.rs index 6ee989070b429..a0c256bb83dc9 100644 --- a/compiler/rustc_typeck/src/check/generator_interior.rs +++ b/compiler/rustc_typeck/src/check/generator_interior.rs @@ -14,7 +14,7 @@ use rustc_hir::hir_id::HirIdSet; use rustc_hir::intravisit::{self, Visitor}; use rustc_hir::{Arm, Expr, ExprKind, Guard, HirId, Pat, PatKind}; use rustc_middle::middle::region::{self, Scope, ScopeData, YieldData}; -use rustc_middle::ty::{self, RvalueScopes, Ty, TyCtxt}; +use rustc_middle::ty::{self, RvalueScopes, Ty, TyCtxt, TypeVisitable}; use rustc_span::symbol::sym; use rustc_span::Span; use tracing::debug; @@ -376,6 +376,17 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> { debug!("is_borrowed_temporary: {:?}", self.drop_ranges.is_borrowed_temporary(expr)); + let ty = self.fcx.typeck_results.borrow().expr_ty_adjusted_opt(expr); + let may_need_drop = |ty: Ty<'tcx>| { + // Avoid ICEs in needs_drop. + let ty = self.fcx.resolve_vars_if_possible(ty); + let ty = self.fcx.tcx.erase_regions(ty); + if ty.needs_infer() { + return true; + } + ty.needs_drop(self.fcx.tcx, self.fcx.param_env) + }; + // Typically, the value produced by an expression is consumed by its parent in some way, // so we only have to check if the parent contains a yield (note that the parent may, for // example, store the value into a local variable, but then we already consider local @@ -384,7 +395,18 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> { // However, in the case of temporary values, we are going to store the value into a // temporary on the stack that is live for the current temporary scope and then return a // reference to it. That value may be live across the entire temporary scope. - let scope = if self.drop_ranges.is_borrowed_temporary(expr) { + // + // There's another subtlety: if the type has an observable drop, it must be dropped after + // the yield, even if it's not borrowed or referenced after the yield. Ideally this would + // *only* happen for types with observable drop, not all types which wrap them, but that + // doesn't match the behavior of MIR borrowck and causes ICEs. See the FIXME comment in + // src/test/ui/generator/drop-tracking-parent-expression.rs. + let scope = if self.drop_ranges.is_borrowed_temporary(expr) + || ty.map_or(true, |ty| { + let needs_drop = may_need_drop(ty); + debug!(?needs_drop, ?ty); + needs_drop + }) { self.rvalue_scopes.temporary_scope(self.region_scope_tree, expr.hir_id.local_id) } else { debug!("parent_node: {:?}", self.fcx.tcx.hir().find_parent_node(expr.hir_id)); @@ -398,7 +420,7 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> { // If there are adjustments, then record the final type -- // this is the actual value that is being produced. - if let Some(adjusted_ty) = self.fcx.typeck_results.borrow().expr_ty_adjusted_opt(expr) { + if let Some(adjusted_ty) = ty { self.record(adjusted_ty, expr.hir_id, scope, Some(expr), expr.span); } diff --git a/src/test/ui/async-await/default-struct-update.rs b/src/test/ui/async-await/default-struct-update.rs new file mode 100644 index 0000000000000..64fb6280dd7bb --- /dev/null +++ b/src/test/ui/async-await/default-struct-update.rs @@ -0,0 +1,22 @@ +// build-pass +// edition:2018 +// compile-flags: -Zdrop-tracking=y + +fn main() { + let _ = foo(); +} + +async fn from_config(_: Config) {} + +async fn foo() { + from_config(Config { + nickname: None, + ..Default::default() + }) + .await; +} + +#[derive(Default)] +struct Config { + nickname: Option>, +} diff --git a/src/test/ui/async-await/issue-70935-complex-spans.drop_tracking.stderr b/src/test/ui/async-await/issue-70935-complex-spans.drop_tracking.stderr index 9b6917df45d2f..2ce7309e1de63 100644 --- a/src/test/ui/async-await/issue-70935-complex-spans.drop_tracking.stderr +++ b/src/test/ui/async-await/issue-70935-complex-spans.drop_tracking.stderr @@ -1,5 +1,5 @@ error[E0277]: `Sender` cannot be shared between threads safely - --> $DIR/issue-70935-complex-spans.rs:13:45 + --> $DIR/issue-70935-complex-spans.rs:12:45 | LL | fn foo(tx: std::sync::mpsc::Sender) -> impl Future + Send { | ^^^^^^^^^^^^^^^^^^ `Sender` cannot be shared between threads safely @@ -7,7 +7,7 @@ LL | fn foo(tx: std::sync::mpsc::Sender) -> impl Future + Send { = help: the trait `Sync` is not implemented for `Sender` = note: required because of the requirements on the impl of `Send` for `&Sender` note: required because it's used within this closure - --> $DIR/issue-70935-complex-spans.rs:25:13 + --> $DIR/issue-70935-complex-spans.rs:16:13 | LL | baz(|| async{ | ^^ @@ -16,16 +16,14 @@ note: required because it's used within this `async fn` body | LL | async fn baz(_c: impl FnMut() -> T) where T: Future { | ___________________________________________________________________^ -LL | | LL | | } | |_^ = note: required because it captures the following types: `ResumeTy`, `impl for<'r, 's, 't0> Future`, `()` note: required because it's used within this `async` block - --> $DIR/issue-70935-complex-spans.rs:23:16 + --> $DIR/issue-70935-complex-spans.rs:15:16 | LL | async move { | ________________^ -LL | | LL | | baz(|| async{ LL | | foo(tx.clone()); LL | | }).await; diff --git a/src/test/ui/async-await/issue-70935-complex-spans.normal.stderr b/src/test/ui/async-await/issue-70935-complex-spans.normal.stderr index 88b646d2792db..2b81b400099f4 100644 --- a/src/test/ui/async-await/issue-70935-complex-spans.normal.stderr +++ b/src/test/ui/async-await/issue-70935-complex-spans.normal.stderr @@ -1,12 +1,12 @@ error: future cannot be sent between threads safely - --> $DIR/issue-70935-complex-spans.rs:13:45 + --> $DIR/issue-70935-complex-spans.rs:12:45 | LL | fn foo(tx: std::sync::mpsc::Sender) -> impl Future + Send { | ^^^^^^^^^^^^^^^^^^ future created by async block is not `Send` | = help: the trait `Sync` is not implemented for `Sender` note: future is not `Send` as this value is used across an await - --> $DIR/issue-70935-complex-spans.rs:27:11 + --> $DIR/issue-70935-complex-spans.rs:18:11 | LL | baz(|| async{ | _____________- @@ -14,9 +14,9 @@ LL | | foo(tx.clone()); LL | | }).await; | | - ^^^^^^ await occurs here, with the value maybe used later | |_________| - | has type `[closure@$DIR/issue-70935-complex-spans.rs:25:13: 25:15]` which is not `Send` + | has type `[closure@$DIR/issue-70935-complex-spans.rs:16:13: 16:15]` which is not `Send` note: the value is later dropped here - --> $DIR/issue-70935-complex-spans.rs:27:17 + --> $DIR/issue-70935-complex-spans.rs:18:17 | LL | }).await; | ^ diff --git a/src/test/ui/async-await/issue-70935-complex-spans.rs b/src/test/ui/async-await/issue-70935-complex-spans.rs index f45ce1f25efa0..48847cdf974bd 100644 --- a/src/test/ui/async-await/issue-70935-complex-spans.rs +++ b/src/test/ui/async-await/issue-70935-complex-spans.rs @@ -7,22 +7,13 @@ use std::future::Future; async fn baz(_c: impl FnMut() -> T) where T: Future { -//[drop_tracking]~^ within this `async fn` body } fn foo(tx: std::sync::mpsc::Sender) -> impl Future + Send { - //[normal]~^ ERROR: future cannot be sent between threads safely - //[drop_tracking]~^^ ERROR: `Sender` cannot be shared - //[drop_tracking]~| NOTE: cannot be shared - //[drop_tracking]~| NOTE: requirements on the impl of `Send` - //[drop_tracking]~| NOTE: captures the following types - //[drop_tracking]~| NOTE: in this expansion - //[drop_tracking]~| NOTE: in this expansion - //[drop_tracking]~| NOTE: in this expansion - //[drop_tracking]~| NOTE: in this expansion + //[normal]~^ ERROR future cannot be sent between threads safely + //[drop_tracking]~^^ ERROR `Sender` cannot be shared between threads async move { - //[drop_tracking]~^ within this `async` block - baz(|| async{ //[drop_tracking]~ NOTE: used within this closure + baz(|| async{ foo(tx.clone()); }).await; } diff --git a/src/test/ui/async-await/non-trivial-drop.rs b/src/test/ui/async-await/non-trivial-drop.rs new file mode 100644 index 0000000000000..a3167215dc32b --- /dev/null +++ b/src/test/ui/async-await/non-trivial-drop.rs @@ -0,0 +1,36 @@ +// build-pass +// edition:2018 +// compile-flags: -Zdrop-tracking=y + +#![feature(generators)] + +fn main() { + let _ = foo(); +} + +fn foo() { + || { + yield drop(Config { + nickname: NonCopy, + b: NonCopy2, + }.nickname); + }; +} + +#[derive(Default)] +struct NonCopy; +impl Drop for NonCopy { + fn drop(&mut self) {} +} + +#[derive(Default)] +struct NonCopy2; +impl Drop for NonCopy2 { + fn drop(&mut self) {} +} + +#[derive(Default)] +struct Config { + nickname: NonCopy, + b: NonCopy2, +} diff --git a/src/test/ui/async-await/type-parameter-send.rs b/src/test/ui/async-await/type-parameter-send.rs new file mode 100644 index 0000000000000..ab2b62aa5aa1c --- /dev/null +++ b/src/test/ui/async-await/type-parameter-send.rs @@ -0,0 +1,18 @@ +// check-pass +// compile-flags: --crate-type lib +// edition:2018 + +fn assert_send(_: F) {} + +async fn __post() -> T { + if false { + todo!() + } else { + async {}.await; + todo!() + } +} + +fn foo() { + assert_send(__post::()); +} diff --git a/src/test/ui/generator/derived-drop-parent-expr.rs b/src/test/ui/generator/derived-drop-parent-expr.rs new file mode 100644 index 0000000000000..4bd34346a1815 --- /dev/null +++ b/src/test/ui/generator/derived-drop-parent-expr.rs @@ -0,0 +1,17 @@ +// build-pass +// compile-flags:-Zdrop-tracking + +//! Like drop-tracking-parent-expression, but also tests that this doesn't ICE when building MIR +#![feature(generators)] + +fn assert_send(_thing: T) {} + +#[derive(Default)] +pub struct Client { pub nickname: String } + +fn main() { + let g = move || match drop(Client { ..Client::default() }) { + _status => yield, + }; + assert_send(g); +} diff --git a/src/test/ui/generator/drop-tracking-parent-expression.rs b/src/test/ui/generator/drop-tracking-parent-expression.rs new file mode 100644 index 0000000000000..d40f1b8f64d4a --- /dev/null +++ b/src/test/ui/generator/drop-tracking-parent-expression.rs @@ -0,0 +1,69 @@ +// compile-flags: -Zdrop-tracking +#![feature(generators, negative_impls, rustc_attrs)] + +macro_rules! type_combinations { + ( + $( $name:ident => { $( $tt:tt )* } );* $(;)? + ) => { $( + mod $name { + $( $tt )* + + impl !Sync for Client {} + impl !Send for Client {} + } + + // Struct update syntax. This fails because the Client used in the update is considered + // dropped *after* the yield. + { + let g = move || match drop($name::Client { ..$name::Client::default() }) { + //~^ `significant_drop::Client` which is not `Send` + //~| `insignificant_dtor::Client` which is not `Send` + //~| `derived_drop::Client` which is not `Send` + _ => yield, + }; + assert_send(g); + //~^ ERROR cannot be sent between threads + //~| ERROR cannot be sent between threads + //~| ERROR cannot be sent between threads + } + + // Simple owned value. This works because the Client is considered moved into `drop`, + // even though the temporary expression doesn't end until after the yield. + { + let g = move || match drop($name::Client::default()) { + _ => yield, + }; + assert_send(g); + } + )* } +} + +fn assert_send(_thing: T) {} + +fn main() { + type_combinations!( + // OK + copy => { #[derive(Copy, Clone, Default)] pub struct Client; }; + // NOT OK: MIR borrowck thinks that this is used after the yield, even though + // this has no `Drop` impl and only the drops of the fields are observable. + // FIXME: this should compile. + derived_drop => { #[derive(Default)] pub struct Client { pub nickname: String } }; + // NOT OK + significant_drop => { + #[derive(Default)] + pub struct Client; + impl Drop for Client { + fn drop(&mut self) {} + } + }; + // NOT OK (we need to agree with MIR borrowck) + insignificant_dtor => { + #[derive(Default)] + #[rustc_insignificant_dtor] + pub struct Client; + impl Drop for Client { + fn drop(&mut self) {} + } + }; + ); +} diff --git a/src/test/ui/generator/drop-tracking-parent-expression.stderr b/src/test/ui/generator/drop-tracking-parent-expression.stderr new file mode 100644 index 0000000000000..522a300b3ed79 --- /dev/null +++ b/src/test/ui/generator/drop-tracking-parent-expression.stderr @@ -0,0 +1,128 @@ +error: generator cannot be sent between threads safely + --> $DIR/drop-tracking-parent-expression.rs:24:13 + | +LL | assert_send(g); + | ^^^^^^^^^^^ generator is not `Send` +... +LL | / type_combinations!( +LL | | // OK +LL | | copy => { #[derive(Copy, Clone, Default)] pub struct Client; }; +LL | | // NOT OK: MIR borrowck thinks that this is used after the yield, even though +... | +LL | | }; +LL | | ); + | |_____- in this macro invocation + | + = help: within `[generator@$DIR/drop-tracking-parent-expression.rs:18:21: 18:28]`, the trait `Send` is not implemented for `derived_drop::Client` +note: generator is not `Send` as this value is used across a yield + --> $DIR/drop-tracking-parent-expression.rs:22:22 + | +LL | let g = move || match drop($name::Client { ..$name::Client::default() }) { + | ------------------------ has type `derived_drop::Client` which is not `Send` +... +LL | _ => yield, + | ^^^^^ yield occurs here, with `$name::Client::default()` maybe used later +LL | }; + | - `$name::Client::default()` is later dropped here +... +LL | / type_combinations!( +LL | | // OK +LL | | copy => { #[derive(Copy, Clone, Default)] pub struct Client; }; +LL | | // NOT OK: MIR borrowck thinks that this is used after the yield, even though +... | +LL | | }; +LL | | ); + | |_____- in this macro invocation +note: required by a bound in `assert_send` + --> $DIR/drop-tracking-parent-expression.rs:41:19 + | +LL | fn assert_send(_thing: T) {} + | ^^^^ required by this bound in `assert_send` + = note: this error originates in the macro `type_combinations` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: generator cannot be sent between threads safely + --> $DIR/drop-tracking-parent-expression.rs:24:13 + | +LL | assert_send(g); + | ^^^^^^^^^^^ generator is not `Send` +... +LL | / type_combinations!( +LL | | // OK +LL | | copy => { #[derive(Copy, Clone, Default)] pub struct Client; }; +LL | | // NOT OK: MIR borrowck thinks that this is used after the yield, even though +... | +LL | | }; +LL | | ); + | |_____- in this macro invocation + | + = help: within `[generator@$DIR/drop-tracking-parent-expression.rs:18:21: 18:28]`, the trait `Send` is not implemented for `significant_drop::Client` +note: generator is not `Send` as this value is used across a yield + --> $DIR/drop-tracking-parent-expression.rs:22:22 + | +LL | let g = move || match drop($name::Client { ..$name::Client::default() }) { + | ------------------------ has type `significant_drop::Client` which is not `Send` +... +LL | _ => yield, + | ^^^^^ yield occurs here, with `$name::Client::default()` maybe used later +LL | }; + | - `$name::Client::default()` is later dropped here +... +LL | / type_combinations!( +LL | | // OK +LL | | copy => { #[derive(Copy, Clone, Default)] pub struct Client; }; +LL | | // NOT OK: MIR borrowck thinks that this is used after the yield, even though +... | +LL | | }; +LL | | ); + | |_____- in this macro invocation +note: required by a bound in `assert_send` + --> $DIR/drop-tracking-parent-expression.rs:41:19 + | +LL | fn assert_send(_thing: T) {} + | ^^^^ required by this bound in `assert_send` + = note: this error originates in the macro `type_combinations` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: generator cannot be sent between threads safely + --> $DIR/drop-tracking-parent-expression.rs:24:13 + | +LL | assert_send(g); + | ^^^^^^^^^^^ generator is not `Send` +... +LL | / type_combinations!( +LL | | // OK +LL | | copy => { #[derive(Copy, Clone, Default)] pub struct Client; }; +LL | | // NOT OK: MIR borrowck thinks that this is used after the yield, even though +... | +LL | | }; +LL | | ); + | |_____- in this macro invocation + | + = help: within `[generator@$DIR/drop-tracking-parent-expression.rs:18:21: 18:28]`, the trait `Send` is not implemented for `insignificant_dtor::Client` +note: generator is not `Send` as this value is used across a yield + --> $DIR/drop-tracking-parent-expression.rs:22:22 + | +LL | let g = move || match drop($name::Client { ..$name::Client::default() }) { + | ------------------------ has type `insignificant_dtor::Client` which is not `Send` +... +LL | _ => yield, + | ^^^^^ yield occurs here, with `$name::Client::default()` maybe used later +LL | }; + | - `$name::Client::default()` is later dropped here +... +LL | / type_combinations!( +LL | | // OK +LL | | copy => { #[derive(Copy, Clone, Default)] pub struct Client; }; +LL | | // NOT OK: MIR borrowck thinks that this is used after the yield, even though +... | +LL | | }; +LL | | ); + | |_____- in this macro invocation +note: required by a bound in `assert_send` + --> $DIR/drop-tracking-parent-expression.rs:41:19 + | +LL | fn assert_send(_thing: T) {} + | ^^^^ required by this bound in `assert_send` + = note: this error originates in the macro `type_combinations` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 3 previous errors + diff --git a/src/test/ui/generator/issue-57017.rs b/src/test/ui/generator/issue-57017.rs index 1223a3037abc7..c0bde3b4473e1 100644 --- a/src/test/ui/generator/issue-57017.rs +++ b/src/test/ui/generator/issue-57017.rs @@ -1,22 +1,55 @@ -// check-pass +// build-pass // compile-flags: -Zdrop-tracking #![feature(generators, negative_impls)] -struct Client; +macro_rules! type_combinations { + ( + $( $name:ident => { $( $tt:tt )* } );* + ) => { $( + mod $name { + pub mod unsync { + $( $tt )* -impl !Sync for Client {} + impl !Sync for Client {} + } + pub mod unsend { + $( $tt )* -fn status(_client_status: &Client) -> i16 { - 200 + impl !Send for Client {} + } + } + + // This is the same bug as issue 57017, but using yield instead of await + { + let g = move || match drop(&$name::unsync::Client::default()) { + _status => yield, + }; + assert_send(g); + } + + // This tests that `Client` is properly considered to be dropped after moving it into the + // function. + { + let g = move || match drop($name::unsend::Client::default()) { + _status => yield, + }; + assert_send(g); + } + )* } } fn assert_send(_thing: T) {} -// This is the same bug as issue 57017, but using yield instead of await fn main() { - let client = Client; - let g = move || match status(&client) { - _status => yield, - }; - assert_send(g); + type_combinations!( + copy => { #[derive(Copy, Clone, Default)] pub struct Client; }; + derived_drop => { #[derive(Default)] pub struct Client { pub nickname: String } }; + significant_drop => { + #[derive(Default)] + pub struct Client; + impl Drop for Client { + fn drop(&mut self) {} + } + } + ); }