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

Mark let_underscore_lock and let_underscore_drop as uplifted #9697

Merged
merged 2 commits into from
Oct 23, 2022

Conversation

Alexendoo
Copy link
Member

@Alexendoo Alexendoo commented Oct 23, 2022

Here I've renamed both the uplifted lints, however rustc's let_underscore_lock is slightly less capable than the clippy lint as it doesn't catch parking_lot types or Result<Guard, ..>, should we still remove it? The Result change looks like it was unintentional to me so that could probably be fixed upstream

changelog: Uplift [let_underscore_drop] to rustc rust-lang/rust#97739
changelog: Remove overlap between rustc's let_underscore_lock and Clippy's [let_underscore_lock]

Closes #8003
Closes #9314

r? @flip1995

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 23, 2022
@Alexendoo Alexendoo force-pushed the let-underscore-uplift branch from 122a332 to 822f882 Compare October 23, 2022 13:31
@flip1995
Copy link
Member

Should we limit the let_underscore_lock lint to just lint on parking_lot locks? I wouldn't completely deprecate it.

@Alexendoo Alexendoo force-pushed the let-underscore-uplift branch from 822f882 to 5dba80b Compare October 23, 2022 14:05
@Alexendoo Alexendoo force-pushed the let-underscore-uplift branch from 5dba80b to 9306540 Compare October 23, 2022 14:07
@Alexendoo
Copy link
Member Author

Yeah I think that's a good plan

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.

Thanks!

@flip1995
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Oct 23, 2022

📌 Commit 9306540 has been approved by flip1995

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Oct 23, 2022

⌛ Testing commit 9306540 with merge 3f4287c...

@bors
Copy link
Contributor

bors commented Oct 23, 2022

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

1 similar comment
@bors
Copy link
Contributor

bors commented Oct 23, 2022

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

@bors bors merged commit 3f4287c into rust-lang:master Oct 23, 2022
@Alexendoo Alexendoo deleted the let-underscore-uplift branch October 23, 2022 16:51
akoshelev added a commit to akoshelev/raw-ipa that referenced this pull request Oct 27, 2022
Since yesterday, clippy seems unhappy with two `prss` tests with no obvious reason why.

```
error: non-binding `let` on a type that implements `Drop`
   --> src/protocol/prss.rs:571:9
    |
571 |         let _ = p1.sequential(&step);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `-D clippy::let-underscore-drop` implied by `-D clippy::pedantic`
    = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_underscore_drop
```

It either sees what I don't see, but I wasn't able to find `Drop` implementation for that type. I also wasn't able to reproduce it locally neither on MacOS nor Linux, despite the fact that I am using the same cargo version as our GH action

```
~/workspace/raw-ipa% ~/.cargo/bin/cargo -V
cargo 1.64.0 (387270bc7 2022-09-16)
```

I wish I could learn clippy version as well, but reading the output did not reveal it.

There was a recent change in Clippy code that touched that lint: rust-lang/rust-clippy#9697 but I can't see how it could trigger this behaviour. To unblock us, lets appease Clippy for now
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
4 participants