Skip to content

Commit

Permalink
Rollup merge of #97962 - eholk:drop-tracking-must-not-suspend, r=cjgi…
Browse files Browse the repository at this point in the history
…llot

Make must_not_suspend lint see through references when drop tracking is enabled

See #97333.

With drop tracking enabled, sometimes values that were previously linted are now considered dropped and not linted. This change makes must_not_suspend traverse through references to still catch these values.

Unfortunately, this leads to duplicate warnings in some cases (e.g. [dedup.rs](https://cs.github.com/rust-lang/rust/blob/9a74608543d499bcc7dd505e195e8bfab9447315/src/test/ui/lint/must_not_suspend/dedup.rs#L4)), so we only use the new behavior when drop tracking is enabled.

cc ``@guswynn``
  • Loading branch information
matthiaskrgr authored Aug 18, 2022
2 parents 9c20b2a + 0da8199 commit 370bf15
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 2 deletions.
12 changes: 10 additions & 2 deletions compiler/rustc_typeck/src/check/generator_interior.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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();
Expand Down Expand Up @@ -580,6 +582,12 @@ pub 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.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 })
}
_ => false,
}
}
Expand Down
30 changes: 30 additions & 0 deletions src/test/ui/lint/must_not_suspend/ref-drop-tracking.rs
Original file line number Diff line number Diff line change
@@ -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() {
}
27 changes: 27 additions & 0 deletions src/test/ui/lint/must_not_suspend/ref-drop-tracking.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 370bf15

Please sign in to comment.