From 185fa5625656c5a5cb979397b131d1c8bbadeba9 Mon Sep 17 00:00:00 2001 From: Gus Wynn Date: Tue, 12 Oct 2021 09:10:05 -0700 Subject: [PATCH] Feature gate and make must_not_suspend allow-by-default This lint is not yet ready for stable use, primarily due to false positives in edge cases; we want to test it out more before stabilizing. --- compiler/rustc_lint/src/lib.rs | 1 - compiler/rustc_lint_defs/src/builtin.rs | 4 +- .../issue-64130-non-send-future-diags.rs | 2 + .../issue-64130-non-send-future-diags.stderr | 6 +-- src/test/ui/async-await/issue-71137.rs | 2 + src/test/ui/async-await/issue-71137.stderr | 6 +-- src/test/ui/lint/must_not_suspend/gated.rs | 14 +++++ .../ui/lint/must_not_suspend/gated.stderr | 54 +++++++++++++++++++ .../ui/lint/must_not_suspend/issue-89562.rs | 19 +++++++ src/test/ui/lint/must_not_suspend/mutex.rs | 1 + .../ui/lint/must_not_suspend/mutex.stderr | 8 +-- src/test/ui/lint/must_not_suspend/warn.rs | 1 + src/test/ui/lint/must_not_suspend/warn.stderr | 12 +++-- 13 files changed, 114 insertions(+), 16 deletions(-) create mode 100644 src/test/ui/lint/must_not_suspend/gated.rs create mode 100644 src/test/ui/lint/must_not_suspend/gated.stderr create mode 100644 src/test/ui/lint/must_not_suspend/issue-89562.rs diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 6f684a0fe5128..ee0567608f4ed 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -300,7 +300,6 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) { UNUSED_LABELS, UNUSED_PARENS, UNUSED_BRACES, - MUST_NOT_SUSPEND, REDUNDANT_SEMICOLONS ); diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index f35ca2659fd65..c42d2471ca5e6 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -323,6 +323,7 @@ declare_lint! { /// /// ```rust /// #![feature(must_not_suspend)] + /// #![warn(must_not_suspend)] /// /// #[must_not_suspend] /// struct SyncThing {} @@ -349,8 +350,9 @@ declare_lint! { /// `MutexGuard`'s) /// pub MUST_NOT_SUSPEND, - Warn, + Allow, "use of a `#[must_not_suspend]` value across a yield point", + @feature_gate = rustc_span::symbol::sym::must_not_suspend; } declare_lint! { diff --git a/src/test/ui/async-await/issue-64130-non-send-future-diags.rs b/src/test/ui/async-await/issue-64130-non-send-future-diags.rs index 656ade67c71a7..b652d23915330 100644 --- a/src/test/ui/async-await/issue-64130-non-send-future-diags.rs +++ b/src/test/ui/async-await/issue-64130-non-send-future-diags.rs @@ -1,4 +1,6 @@ // edition:2018 +#![feature(must_not_suspend)] +#![allow(must_not_suspend)] // This tests the basic example case for the async-await-specific error. diff --git a/src/test/ui/async-await/issue-64130-non-send-future-diags.stderr b/src/test/ui/async-await/issue-64130-non-send-future-diags.stderr index 7125c62dbaf9a..a373ba6aa7136 100644 --- a/src/test/ui/async-await/issue-64130-non-send-future-diags.stderr +++ b/src/test/ui/async-await/issue-64130-non-send-future-diags.stderr @@ -1,12 +1,12 @@ error: future cannot be sent between threads safely - --> $DIR/issue-64130-non-send-future-diags.rs:21:13 + --> $DIR/issue-64130-non-send-future-diags.rs:23:13 | LL | is_send(foo()); | ^^^^^ future returned by `foo` is not `Send` | = help: within `impl Future`, the trait `Send` is not implemented for `MutexGuard<'_, u32>` note: future is not `Send` as this value is used across an await - --> $DIR/issue-64130-non-send-future-diags.rs:15:5 + --> $DIR/issue-64130-non-send-future-diags.rs:17:5 | LL | let g = x.lock().unwrap(); | - has type `MutexGuard<'_, u32>` which is not `Send` @@ -15,7 +15,7 @@ LL | baz().await; LL | } | - `g` is later dropped here note: required by a bound in `is_send` - --> $DIR/issue-64130-non-send-future-diags.rs:7:15 + --> $DIR/issue-64130-non-send-future-diags.rs:9:15 | LL | fn is_send(t: T) { } | ^^^^ required by this bound in `is_send` diff --git a/src/test/ui/async-await/issue-71137.rs b/src/test/ui/async-await/issue-71137.rs index ebb392a45308e..7695e0325ff31 100644 --- a/src/test/ui/async-await/issue-71137.rs +++ b/src/test/ui/async-await/issue-71137.rs @@ -1,4 +1,6 @@ // edition:2018 +#![feature(must_not_suspend)] +#![allow(must_not_suspend)] use std::future::Future; use std::sync::Mutex; diff --git a/src/test/ui/async-await/issue-71137.stderr b/src/test/ui/async-await/issue-71137.stderr index ac254346c08a6..3cc800f96c20d 100644 --- a/src/test/ui/async-await/issue-71137.stderr +++ b/src/test/ui/async-await/issue-71137.stderr @@ -1,12 +1,12 @@ error: future cannot be sent between threads safely - --> $DIR/issue-71137.rs:20:14 + --> $DIR/issue-71137.rs:22:14 | LL | fake_spawn(wrong_mutex()); | ^^^^^^^^^^^^^ future returned by `wrong_mutex` is not `Send` | = help: within `impl Future`, the trait `Send` is not implemented for `MutexGuard<'_, i32>` note: future is not `Send` as this value is used across an await - --> $DIR/issue-71137.rs:12:5 + --> $DIR/issue-71137.rs:14:5 | LL | let mut guard = m.lock().unwrap(); | --------- has type `MutexGuard<'_, i32>` which is not `Send` @@ -16,7 +16,7 @@ LL | *guard += 1; LL | } | - `mut guard` is later dropped here note: required by a bound in `fake_spawn` - --> $DIR/issue-71137.rs:6:27 + --> $DIR/issue-71137.rs:8:27 | LL | fn fake_spawn(f: F) { } | ^^^^ required by this bound in `fake_spawn` diff --git a/src/test/ui/lint/must_not_suspend/gated.rs b/src/test/ui/lint/must_not_suspend/gated.rs new file mode 100644 index 0000000000000..acb81b0bf9def --- /dev/null +++ b/src/test/ui/lint/must_not_suspend/gated.rs @@ -0,0 +1,14 @@ +// edition:2018 +#![deny(must_not_suspend)] //~ ERROR the `must_not_suspend` +//~| ERROR the `must_not_suspend` +//~| ERROR the `must_not_suspend` + +async fn other() {} + +pub async fn uhoh(m: std::sync::Mutex<()>) { + let _guard = m.lock().unwrap(); //~ ERROR `MutexGuard` held across + other().await; +} + +fn main() { +} diff --git a/src/test/ui/lint/must_not_suspend/gated.stderr b/src/test/ui/lint/must_not_suspend/gated.stderr new file mode 100644 index 0000000000000..be077deb3f197 --- /dev/null +++ b/src/test/ui/lint/must_not_suspend/gated.stderr @@ -0,0 +1,54 @@ +error[E0658]: the `must_not_suspend` lint is unstable + --> $DIR/gated.rs:2:1 + | +LL | #![deny(must_not_suspend)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #83310 for more information + = help: add `#![feature(must_not_suspend)]` to the crate attributes to enable + +error[E0658]: the `must_not_suspend` lint is unstable + --> $DIR/gated.rs:2:1 + | +LL | #![deny(must_not_suspend)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #83310 for more information + = help: add `#![feature(must_not_suspend)]` to the crate attributes to enable + +error[E0658]: the `must_not_suspend` lint is unstable + --> $DIR/gated.rs:2:1 + | +LL | #![deny(must_not_suspend)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #83310 for more information + = help: add `#![feature(must_not_suspend)]` to the crate attributes to enable + +error: `MutexGuard` held across a suspend point, but should not be + --> $DIR/gated.rs:9:9 + | +LL | let _guard = m.lock().unwrap(); + | ^^^^^^ +LL | other().await; + | ------------- the value is held across this suspend point + | +note: the lint level is defined here + --> $DIR/gated.rs:2:9 + | +LL | #![deny(must_not_suspend)] + | ^^^^^^^^^^^^^^^^ +note: holding a MutexGuard across suspend points can cause deadlocks, delays, and cause Futures to not implement `Send` + --> $DIR/gated.rs:9:9 + | +LL | let _guard = m.lock().unwrap(); + | ^^^^^^ +help: consider using a block (`{ ... }`) to shrink the value's scope, ending before the suspend point + --> $DIR/gated.rs:9:9 + | +LL | let _guard = m.lock().unwrap(); + | ^^^^^^ + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/lint/must_not_suspend/issue-89562.rs b/src/test/ui/lint/must_not_suspend/issue-89562.rs new file mode 100644 index 0000000000000..acdb36fcdabf9 --- /dev/null +++ b/src/test/ui/lint/must_not_suspend/issue-89562.rs @@ -0,0 +1,19 @@ +// edition:2018 +// run-pass + +use std::sync::Mutex; + +// Copied from the issue. Allow-by-default for now, so run-pass +pub async fn foo() { + let foo = Mutex::new(1); + let lock = foo.lock().unwrap(); + + // Prevent mutex lock being held across `.await` point. + drop(lock); + + bar().await; +} + +async fn bar() {} + +fn main() {} diff --git a/src/test/ui/lint/must_not_suspend/mutex.rs b/src/test/ui/lint/must_not_suspend/mutex.rs index 596249b2e4e4f..7bb895e7d3643 100644 --- a/src/test/ui/lint/must_not_suspend/mutex.rs +++ b/src/test/ui/lint/must_not_suspend/mutex.rs @@ -1,4 +1,5 @@ // edition:2018 +#![feature(must_not_suspend)] #![deny(must_not_suspend)] async fn other() {} diff --git a/src/test/ui/lint/must_not_suspend/mutex.stderr b/src/test/ui/lint/must_not_suspend/mutex.stderr index 093f581264f36..dde506c19e725 100644 --- a/src/test/ui/lint/must_not_suspend/mutex.stderr +++ b/src/test/ui/lint/must_not_suspend/mutex.stderr @@ -1,5 +1,5 @@ error: `MutexGuard` held across a suspend point, but should not be - --> $DIR/mutex.rs:7:9 + --> $DIR/mutex.rs:8:9 | LL | let _guard = m.lock().unwrap(); | ^^^^^^ @@ -7,17 +7,17 @@ LL | other().await; | ------------- the value is held across this suspend point | note: the lint level is defined here - --> $DIR/mutex.rs:2:9 + --> $DIR/mutex.rs:3:9 | LL | #![deny(must_not_suspend)] | ^^^^^^^^^^^^^^^^ note: holding a MutexGuard across suspend points can cause deadlocks, delays, and cause Futures to not implement `Send` - --> $DIR/mutex.rs:7:9 + --> $DIR/mutex.rs:8:9 | LL | let _guard = m.lock().unwrap(); | ^^^^^^ help: consider using a block (`{ ... }`) to shrink the value's scope, ending before the suspend point - --> $DIR/mutex.rs:7:9 + --> $DIR/mutex.rs:8:9 | LL | let _guard = m.lock().unwrap(); | ^^^^^^ diff --git a/src/test/ui/lint/must_not_suspend/warn.rs b/src/test/ui/lint/must_not_suspend/warn.rs index 50a696ba52322..7fdea66a23517 100644 --- a/src/test/ui/lint/must_not_suspend/warn.rs +++ b/src/test/ui/lint/must_not_suspend/warn.rs @@ -1,6 +1,7 @@ // edition:2018 // run-pass #![feature(must_not_suspend)] +#![warn(must_not_suspend)] #[must_not_suspend = "You gotta use Umm's, ya know?"] struct Umm { diff --git a/src/test/ui/lint/must_not_suspend/warn.stderr b/src/test/ui/lint/must_not_suspend/warn.stderr index 24f52275b430a..42374d4acac27 100644 --- a/src/test/ui/lint/must_not_suspend/warn.stderr +++ b/src/test/ui/lint/must_not_suspend/warn.stderr @@ -1,19 +1,23 @@ warning: `Umm` held across a suspend point, but should not be - --> $DIR/warn.rs:20:9 + --> $DIR/warn.rs:21:9 | LL | let _guard = bar(); | ^^^^^^ LL | other().await; | ------------- the value is held across this suspend point | - = note: `#[warn(must_not_suspend)]` on by default +note: the lint level is defined here + --> $DIR/warn.rs:4:9 + | +LL | #![warn(must_not_suspend)] + | ^^^^^^^^^^^^^^^^ note: You gotta use Umm's, ya know? - --> $DIR/warn.rs:20:9 + --> $DIR/warn.rs:21:9 | LL | let _guard = bar(); | ^^^^^^ help: consider using a block (`{ ... }`) to shrink the value's scope, ending before the suspend point - --> $DIR/warn.rs:20:9 + --> $DIR/warn.rs:21:9 | LL | let _guard = bar(); | ^^^^^^