Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature gate + make must_not_suspend allow-by-default #89826

Merged
merged 1 commit into from
Nov 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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`
Mark-Simulacrum marked this conversation as resolved.
Show resolved Hide resolved
//~| 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