Skip to content

Commit

Permalink
Feature gate and make must_not_suspend allow-by-default
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
guswynn authored and Mark-Simulacrum committed Nov 1, 2021
1 parent ff0e148 commit 185fa56
Show file tree
Hide file tree
Showing 13 changed files with 114 additions and 16 deletions.
1 change: 0 additions & 1 deletion compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
);

Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ declare_lint! {
///
/// ```rust
/// #![feature(must_not_suspend)]
/// #![warn(must_not_suspend)]
///
/// #[must_not_suspend]
/// struct SyncThing {}
Expand All @@ -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! {
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/async-await/issue-64130-non-send-future-diags.rs
Original file line number Diff line number Diff line change
@@ -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.

Expand Down
Original file line number Diff line number Diff line change
@@ -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`
Expand All @@ -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: Send>(t: T) { }
| ^^^^ required by this bound in `is_send`
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/async-await/issue-71137.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// edition:2018
#![feature(must_not_suspend)]
#![allow(must_not_suspend)]

use std::future::Future;
use std::sync::Mutex;
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/async-await/issue-71137.stderr
Original file line number Diff line number Diff line change
@@ -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`
Expand All @@ -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: Future + Send + 'static>(f: F) { }
| ^^^^ required by this bound in `fake_spawn`
Expand Down
14 changes: 14 additions & 0 deletions src/test/ui/lint/must_not_suspend/gated.rs
Original file line number Diff line number Diff line change
@@ -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() {
}
54 changes: 54 additions & 0 deletions src/test/ui/lint/must_not_suspend/gated.stderr
Original file line number Diff line number Diff line change
@@ -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 <https://github.com/rust-lang/rust/issues/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 <https://github.com/rust-lang/rust/issues/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 <https://github.com/rust-lang/rust/issues/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`.
19 changes: 19 additions & 0 deletions src/test/ui/lint/must_not_suspend/issue-89562.rs
Original file line number Diff line number Diff line change
@@ -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() {}
1 change: 1 addition & 0 deletions src/test/ui/lint/must_not_suspend/mutex.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// edition:2018
#![feature(must_not_suspend)]
#![deny(must_not_suspend)]

async fn other() {}
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/lint/must_not_suspend/mutex.stderr
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
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();
| ^^^^^^
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();
| ^^^^^^
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/lint/must_not_suspend/warn.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
12 changes: 8 additions & 4 deletions src/test/ui/lint/must_not_suspend/warn.stderr
Original file line number Diff line number Diff line change
@@ -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();
| ^^^^^^
Expand Down

0 comments on commit 185fa56

Please sign in to comment.