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

Add #[must_not_suspend] to some types in std #89303

Merged
merged 3 commits into from
Oct 1, 2021

Conversation

guswynn
Copy link
Contributor

@guswynn guswynn commented Sep 27, 2021

I am not sure what else should have it? Ref?

@rust-highfive
Copy link
Collaborator

r? @dtolnay

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 27, 2021
not(bootstrap),
must_not_suspend = "Holding a MutexGuard across suspend \
points can cause deadlocks, delays, \
and cause Future's to not implement `Send`"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A plural-s does not go with an apostrophe (same issue rwlock.rs).

@Kobzol
Copy link
Contributor

Kobzol commented Sep 28, 2021

I would personally really like to see it on Ref and RefMut. When single-threaded async executors are used with Refcells, holding a Ref across await is a pretty common source of BorrowMut errors in my experience. There's a corresponding clippy lint for this.

Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The RFC explicitly calls out MutexGuard as a motivation for the lint, so that one is a no-brainer. The other types here are all very clearly in the same category as MutexGuard — LGTM.

@dtolnay
Copy link
Member

dtolnay commented Sep 29, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Sep 29, 2021

📌 Commit cb8e83c has been approved by dtolnay

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 29, 2021
@dtolnay
Copy link
Member

dtolnay commented Sep 29, 2021

Cross linking tracking issue. #83310

Manishearth added a commit to Manishearth/rust that referenced this pull request Sep 30, 2021
Add `#[must_not_suspend]` to some types in std

I am not sure what else should have it? `Ref`?
Manishearth added a commit to Manishearth/rust that referenced this pull request Sep 30, 2021
Add `#[must_not_suspend]` to some types in std

I am not sure what else should have it? `Ref`?
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 1, 2021
…arth

Rollup of 8 pull requests

Successful merges:

 - rust-lang#88782 (Fix ICE when `start` lang item has wrong generics)
 - rust-lang#89202 (Resolve infered types when complaining about unexpected call type )
 - rust-lang#89248 (Suggest similarly named associated items in trait impls)
 - rust-lang#89303 (Add `#[must_not_suspend]` to some types in std)
 - rust-lang#89306 (thread: implements available_concurrency on haiku)
 - rust-lang#89314 (fix(lint): don't suggest refutable patterns to "fix" irrefutable bind)
 - rust-lang#89370 (CTFE: tweak aggregate rvalue handling)
 - rust-lang#89392 (bootstrap: Update comment in config.library.toml.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7b40d42 into rust-lang:master Oct 1, 2021
@rustbot rustbot added this to the 1.57.0 milestone Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants