From bdfc68855861ecea26844a66c38c53f496386b7d Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Thu, 9 Jun 2022 16:07:36 -0700 Subject: [PATCH 1/4] Add drop tracking version of must_not_suspend ref test --- .../must_not_suspend/ref-drop-tracking.rs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 src/test/ui/lint/must_not_suspend/ref-drop-tracking.rs diff --git a/src/test/ui/lint/must_not_suspend/ref-drop-tracking.rs b/src/test/ui/lint/must_not_suspend/ref-drop-tracking.rs new file mode 100644 index 0000000000000..1bc4a38125753 --- /dev/null +++ b/src/test/ui/lint/must_not_suspend/ref-drop-tracking.rs @@ -0,0 +1,30 @@ +// edition:2018 +// compile-flags: -Zdrop-tracking +#![feature(must_not_suspend)] +#![deny(must_not_suspend)] + +#[must_not_suspend = "You gotta use Umm's, ya know?"] +struct Umm { + i: i64 +} + +struct Bar { + u: Umm, +} + +async fn other() {} + +impl Bar { + async fn uhoh(&mut self) { + let guard = &mut self.u; //~ ERROR `Umm` held across + + other().await; + + *guard = Umm { + i: 2 + } + } +} + +fn main() { +} From 89d35060b900f0aa441ca16b1bde2cdebcb46263 Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Mon, 23 May 2022 12:50:55 -0700 Subject: [PATCH 2/4] Update must_not_suspend lint to traverse references --- .../rustc_typeck/src/check/generator_interior.rs | 6 ++++++ src/test/ui/lint/must_not_suspend/ref.stderr | 12 ++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_typeck/src/check/generator_interior.rs b/compiler/rustc_typeck/src/check/generator_interior.rs index d4f8001493c8b..b897d137f74ae 100644 --- a/compiler/rustc_typeck/src/check/generator_interior.rs +++ b/compiler/rustc_typeck/src/check/generator_interior.rs @@ -489,6 +489,8 @@ pub fn check_must_not_suspend_ty<'tcx>( let plural_suffix = pluralize!(data.plural_len); + debug!("Checking must_not_suspend for {}", ty); + match *ty.kind() { ty::Adt(..) if ty.is_box() => { let boxed_ty = ty.boxed_ty(); @@ -580,6 +582,10 @@ pub fn check_must_not_suspend_ty<'tcx>( }, ) } + ty::Ref(_region, ty, _mutability) => { + let descr_pre = &format!("{}reference{} to ", data.descr_pre, plural_suffix); + check_must_not_suspend_ty(fcx, ty, hir_id, SuspendCheckData { descr_pre, ..data }) + } _ => false, } } diff --git a/src/test/ui/lint/must_not_suspend/ref.stderr b/src/test/ui/lint/must_not_suspend/ref.stderr index 5f000014c7dba..8fbf3e71649ef 100644 --- a/src/test/ui/lint/must_not_suspend/ref.stderr +++ b/src/test/ui/lint/must_not_suspend/ref.stderr @@ -1,5 +1,5 @@ -error: `Umm` held across a suspend point, but should not be - --> $DIR/ref.rs:18:26 +error: reference to `Umm` held across a suspend point, but should not be + --> $DIR/ref.rs:18:13 | LL | let guard = &mut self.u; | ^^^^^^ @@ -13,15 +13,15 @@ note: the lint level is defined here LL | #![deny(must_not_suspend)] | ^^^^^^^^^^^^^^^^ note: You gotta use Umm's, ya know? - --> $DIR/ref.rs:18:26 + --> $DIR/ref.rs:18:13 | LL | let guard = &mut self.u; - | ^^^^^^ + | ^^^^^ help: consider using a block (`{ ... }`) to shrink the value's scope, ending before the suspend point - --> $DIR/ref.rs:18:26 + --> $DIR/ref.rs:18:13 | LL | let guard = &mut self.u; - | ^^^^^^ + | ^^^^^ error: aborting due to previous error From c42c77bc7b64cf9015c5b8e7f11f192e499509b3 Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Fri, 10 Jun 2022 11:44:53 -0700 Subject: [PATCH 3/4] Update to still be correct without drop tracking --- .../src/check/generator_interior.rs | 8 +++--- .../must_not_suspend/ref-drop-tracking.stderr | 27 +++++++++++++++++++ src/test/ui/lint/must_not_suspend/ref.stderr | 12 ++++----- 3 files changed, 38 insertions(+), 9 deletions(-) create mode 100644 src/test/ui/lint/must_not_suspend/ref-drop-tracking.stderr diff --git a/compiler/rustc_typeck/src/check/generator_interior.rs b/compiler/rustc_typeck/src/check/generator_interior.rs index b897d137f74ae..bfe0200101c93 100644 --- a/compiler/rustc_typeck/src/check/generator_interior.rs +++ b/compiler/rustc_typeck/src/check/generator_interior.rs @@ -457,7 +457,7 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> { } #[derive(Default)] -pub struct SuspendCheckData<'a, 'tcx> { +struct SuspendCheckData<'a, 'tcx> { expr: Option<&'tcx Expr<'tcx>>, source_span: Span, yield_span: Span, @@ -472,7 +472,7 @@ pub struct SuspendCheckData<'a, 'tcx> { // // Note that this technique was chosen over things like a `Suspend` marker trait // as it is simpler and has precedent in the compiler -pub fn check_must_not_suspend_ty<'tcx>( +fn check_must_not_suspend_ty<'tcx>( fcx: &FnCtxt<'_, 'tcx>, ty: Ty<'tcx>, hir_id: HirId, @@ -582,7 +582,9 @@ pub fn check_must_not_suspend_ty<'tcx>( }, ) } - ty::Ref(_region, ty, _mutability) => { + // If drop tracking is enabled, we want to look through references, since the referrent + // may not be considered live across the await point. + ty::Ref(_region, ty, _mutability) if fcx.sess().opts.debugging_opts.drop_tracking => { let descr_pre = &format!("{}reference{} to ", data.descr_pre, plural_suffix); check_must_not_suspend_ty(fcx, ty, hir_id, SuspendCheckData { descr_pre, ..data }) } diff --git a/src/test/ui/lint/must_not_suspend/ref-drop-tracking.stderr b/src/test/ui/lint/must_not_suspend/ref-drop-tracking.stderr new file mode 100644 index 0000000000000..33c7ff2cb3375 --- /dev/null +++ b/src/test/ui/lint/must_not_suspend/ref-drop-tracking.stderr @@ -0,0 +1,27 @@ +error: reference to `Umm` held across a suspend point, but should not be + --> $DIR/ref-drop-tracking.rs:19:13 + | +LL | let guard = &mut self.u; + | ^^^^^ +LL | +LL | other().await; + | ------ the value is held across this suspend point + | +note: the lint level is defined here + --> $DIR/ref-drop-tracking.rs:4:9 + | +LL | #![deny(must_not_suspend)] + | ^^^^^^^^^^^^^^^^ +note: You gotta use Umm's, ya know? + --> $DIR/ref-drop-tracking.rs:19:13 + | +LL | let guard = &mut self.u; + | ^^^^^ +help: consider using a block (`{ ... }`) to shrink the value's scope, ending before the suspend point + --> $DIR/ref-drop-tracking.rs:19:13 + | +LL | let guard = &mut self.u; + | ^^^^^ + +error: aborting due to previous error + diff --git a/src/test/ui/lint/must_not_suspend/ref.stderr b/src/test/ui/lint/must_not_suspend/ref.stderr index 8fbf3e71649ef..5f000014c7dba 100644 --- a/src/test/ui/lint/must_not_suspend/ref.stderr +++ b/src/test/ui/lint/must_not_suspend/ref.stderr @@ -1,5 +1,5 @@ -error: reference to `Umm` held across a suspend point, but should not be - --> $DIR/ref.rs:18:13 +error: `Umm` held across a suspend point, but should not be + --> $DIR/ref.rs:18:26 | LL | let guard = &mut self.u; | ^^^^^^ @@ -13,15 +13,15 @@ note: the lint level is defined here LL | #![deny(must_not_suspend)] | ^^^^^^^^^^^^^^^^ note: You gotta use Umm's, ya know? - --> $DIR/ref.rs:18:13 + --> $DIR/ref.rs:18:26 | LL | let guard = &mut self.u; - | ^^^^^ + | ^^^^^^ help: consider using a block (`{ ... }`) to shrink the value's scope, ending before the suspend point - --> $DIR/ref.rs:18:13 + --> $DIR/ref.rs:18:26 | LL | let guard = &mut self.u; - | ^^^^^ + | ^^^^^^ error: aborting due to previous error From 0da81997a2652b8a46506cf0a5e4d382ea1f3308 Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Fri, 29 Jul 2022 15:13:44 -0700 Subject: [PATCH 4/4] Fix after rebasing --- compiler/rustc_typeck/src/check/generator_interior.rs | 2 +- src/test/ui/lint/must_not_suspend/ref-drop-tracking.stderr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_typeck/src/check/generator_interior.rs b/compiler/rustc_typeck/src/check/generator_interior.rs index bfe0200101c93..85a0d4e449906 100644 --- a/compiler/rustc_typeck/src/check/generator_interior.rs +++ b/compiler/rustc_typeck/src/check/generator_interior.rs @@ -584,7 +584,7 @@ fn check_must_not_suspend_ty<'tcx>( } // If drop tracking is enabled, we want to look through references, since the referrent // may not be considered live across the await point. - ty::Ref(_region, ty, _mutability) if fcx.sess().opts.debugging_opts.drop_tracking => { + ty::Ref(_region, ty, _mutability) if fcx.sess().opts.unstable_opts.drop_tracking => { let descr_pre = &format!("{}reference{} to ", data.descr_pre, plural_suffix); check_must_not_suspend_ty(fcx, ty, hir_id, SuspendCheckData { descr_pre, ..data }) } diff --git a/src/test/ui/lint/must_not_suspend/ref-drop-tracking.stderr b/src/test/ui/lint/must_not_suspend/ref-drop-tracking.stderr index 33c7ff2cb3375..c49d271285371 100644 --- a/src/test/ui/lint/must_not_suspend/ref-drop-tracking.stderr +++ b/src/test/ui/lint/must_not_suspend/ref-drop-tracking.stderr @@ -3,7 +3,7 @@ error: reference to `Umm` held across a suspend point, but should not be | LL | let guard = &mut self.u; | ^^^^^ -LL | +LL | LL | other().await; | ------ the value is held across this suspend point |