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

new lint: missing-spin-loop #8174

Merged
merged 1 commit into from
Mar 2, 2022
Merged

new lint: missing-spin-loop #8174

merged 1 commit into from
Mar 2, 2022

Conversation

llogiq
Copy link
Contributor

@llogiq llogiq commented Dec 26, 2021

This fixes #7809. I went with the shorter name because the function is called std::hint::spin_loop. It doesn't yet detect while let loops. I left that for a follow-up PR.


changelog: new lint: [missing_spin_loop]

@rust-highfive
Copy link

r? @flip1995

(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 Dec 26, 2021
@llogiq llogiq force-pushed the missing-spin-loop branch 2 times, most recently from 0394b51 to d1e704e Compare December 28, 2021 15:51
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

LGTM overall, just a NIT.

clippy_lints/src/loops/missing_spin_loop.rs Outdated Show resolved Hide resolved
@flip1995 flip1995 added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Jan 12, 2022
@llogiq
Copy link
Contributor Author

llogiq commented Jan 16, 2022

This is blocked on rust/#92870.

@llogiq llogiq added the S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work label Jan 16, 2022
@flip1995 flip1995 removed the S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work label Mar 1, 2022
@llogiq
Copy link
Contributor Author

llogiq commented Mar 1, 2022

No longer blocked. No idea why the base test was cancelled, but everything seems to be in order here locally.

r? @flip1995

Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

LGTM. r=me after removing the FIXME.

if let ExprKind::MethodCall(method, [callee, ..], _) = unpack_cond(cond).kind;
if [sym::load, sym::compare_exchange, sym::compare_exchange_weak].contains(&method.ident.name);
if let ty::Adt(def, _substs) = cx.typeck_results().expr_ty(callee).kind();
//FIXME: The AtomicBool sym is apparently not correctly assigned, once it is, uncomment:
Copy link
Member

Choose a reason for hiding this comment

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

I think this fixme can now be removed.

@llogiq
Copy link
Contributor Author

llogiq commented Mar 2, 2022

@bors r=@flip1995

@bors
Copy link
Contributor

bors commented Mar 2, 2022

📌 Commit 9f1080c has been approved by flip1995

@bors
Copy link
Contributor

bors commented Mar 2, 2022

⌛ Testing commit 9f1080c with merge 27869d6...

@bors
Copy link
Contributor

bors commented Mar 2, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 27869d6 to master...

@bors bors merged commit 27869d6 into master Mar 2, 2022
@bors bors mentioned this pull request Mar 2, 2022
@flip1995 flip1995 deleted the missing-spin-loop branch March 4, 2022 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New lint: loop with atomics without other side effects
4 participants