Create clippy lint against unexpectedly late drop for temporaries in match scrutinee expressions by PrestonFrom · Pull Request #94206 · rust-lang/rust

PrestonFrom · 2022-02-21T03:22:11Z

A new clippy lint for issue 93883 (#93883). Relies on adding a new attribute (clippy::has_significant_drop) to MutexGuard, RwLockReadGuard, and RwLockWriteGuard, which is why this PR is for the rust-lang repo and not the clippy repo.

changelog: new lint [significant_drop_in_scrutinee]

rust-highfive · 2022-02-21T03:22:14Z

Some changes occurred in src/tools/clippy.

cc @rust-lang/clippy

rust-highfive · 2022-02-21T03:22:15Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon.

Please see the contribution instructions for more information.

oli-obk · 2022-02-21T07:48:07Z

r? @oli-obk

To other reviewers: I'm fully aware that the spans, diagnostic messages, documentation and tests are not in a state yet where the lint is ready for general usage. These will get resolved before asking to move the lint out of the nursery.

bors · 2022-02-26T20:03:25Z

☔ The latest upstream changes (presumably #94329) made this pull request unmergeable. Please resolve the merge conflicts.

xFrednet · 2022-02-27T08:49:43Z

Hey @PrestonFrom, Clippy creates its changelog from the PR description. Could you include a changelog entry to ensure that this new lint is caught when the changelog is written. You can just add the following line:

changelog: new lint [`significant_drop_in_scrutinee`]

That would be a big help 🙃

PrestonFrom · 2022-02-27T17:22:45Z

Yes, I will do that!

flip1995

Left some review comments, mostly about the code style.

It also seems like that something has gone wrong with the submodule updates. This PR shouldn't update any submodule.

PrestonFrom · 2022-03-03T07:34:43Z

Left some review comments, mostly about the code style.

It also seems like that something has gone wrong with the submodule updates. This PR shouldn't update any submodule.

Thank you for the review comments! I am not sure what happened with the submodules, but I think I have fixed it.

I'm still adding tests, but would appreciate any feed back about the matches in has_significant_drop and has_significant_drop to make sure I'm on the right track.

flip1995

Thanks for addressing all the comments! I left an explanation on how I think the visitor implementation could look like. Getting this right without implementing and testing it is tricky though. So it might take some experimentation from you to get the visitor right. If you get stuck and think that the visitor approach doesn't work at all, please let me know and I will try to give you some more pointers / rethink if a visitor is the right approach.

PrestonFrom · 2022-03-06T05:38:35Z

@flip1995 @oli-obk Thank you for all your help so far! I think I have an MVP working with error message that suggests alternatives. I would appreciate another look from you both when you have time.

flip1995

I didn't review the tests and documentation yet.

PrestonFrom · 2022-03-10T07:10:43Z

@flip1995 Sorry for the delay -- I had a bit of trouble figuring out how to get the span/suggestion working with the match, but I think it's working. Please let me know if this approach doesn't actually work though.

flip1995 · 2022-05-07T10:33:05Z

@bors r+

Thanks! Great work!

oli-obk · 2022-05-07T17:22:13Z

@bors r=flip1995

Not sure why the bot ignored you outright

oli-obk · 2022-05-07T17:22:48Z

@bors ping

flip1995 · 2022-05-07T17:52:03Z

This PR doesn't appear in the bors queue for some reason 🤔

compiler-errors · 2022-05-07T17:52:47Z

Lemme try clicking the "synchronize" button? 🤔

PrestonFrom · 2022-05-07T18:03:32Z

Oh no! Sorry, I’m away from my computer until tonight. Please let me know if there’s anything I can do to help.

Mark-Simulacrum · 2022-05-07T18:05:48Z

@bors r=flip1995

bors · 2022-05-07T18:05:49Z

📌 Commit d9e963f has been approved by flip1995

compiler-errors · 2022-05-07T18:10:01Z

Yeah, closing and reopening it is a lot simpler than what I did. Sorry haha!

bors · 2022-05-08T00:57:11Z

⌛ Testing commit d9e963f with merge 4d1076c...

bors · 2022-05-08T03:37:47Z

☀️ Test successful - checks-actions
Approved by: flip1995
Pushing 4d1076c to master...

rust-timer · 2022-05-08T04:56:23Z

Finished benchmarking commit (4d1076c): comparison url.

Summary:

Primary benchmarks: no relevant changes found
Secondary benchmarks: 😿 relevant regression found

	Regressions 😿 (primary)	Regressions 😿 (secondary)	Improvements 🎉 (primary)	Improvements 🎉 (secondary)	All 😿 🎉 (primary)
count¹	0	1	0	0	0
mean²	N/A	0.1%	N/A	N/A	N/A
max	N/A	0.1%	N/A	N/A	N/A

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

number of relevant changes ↩
the arithmetic mean of the percent change ↩

klensy · 2022-05-08T05:50:38Z

Current PR description is wrong, there is no SignificantDrop marker, yes?

PrestonFrom · 2022-05-08T10:31:50Z

@klensy Apologies! I missed updating the PR description after the initial code reviews. I just updated it. Thank you for pointing it out!

rust-highfive assigned Mark-Simulacrum Feb 21, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 21, 2022

PrestonFrom mentioned this pull request Feb 21, 2022

lint against unexpectedly late drop for temporaries in match scrutinee expressions #93883

Open

Manishearth reviewed Feb 21, 2022

View reviewed changes

Comment thread library/core/src/marker.rs Outdated

rust-highfive assigned oli-obk and unassigned Mark-Simulacrum Feb 21, 2022

This comment has been minimized.

Sign in to view

flip1995 reviewed Feb 28, 2022

View reviewed changes

This comment has been minimized.

Sign in to view

flip1995 reviewed Mar 3, 2022

View reviewed changes

Comment thread src/tools/clippy/clippy_lints/src/significant_drop_in_scrutinee.rs Outdated

This comment has been minimized.

Sign in to view

flip1995 reviewed Mar 7, 2022

View reviewed changes

oli-obk added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 9, 2022

oli-obk assigned flip1995 Mar 9, 2022

This comment has been minimized.

Sign in to view

Mark-Simulacrum closed this May 7, 2022

Mark-Simulacrum reopened this May 7, 2022

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 7, 2022

This was referenced May 7, 2022

Build LLVM with support for compression #95545

Closed

Fix rustdoc argument error #88831

Closed

Re-enable some gdb tests #95138

Closed

bors added the merged-by-bors This PR was explicitly merged by bors. label May 8, 2022

bors merged commit 4d1076c into rust-lang:master May 8, 2022

rustbot added this to the 1.62.0 milestone May 8, 2022

CAD97 mentioned this pull request May 21, 2022

clippy::let_underscore_lock lint should use #[clippy::has_significant_drop] attribute rather than a hardcoded list rust-lang/rust-clippy#8859

Open

Uh oh!

Uh oh!

Conversation

PrestonFrom commented Feb 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Feb 21, 2022

Uh oh!

rust-highfive commented Feb 21, 2022

Uh oh!

Uh oh!

oli-obk commented Feb 21, 2022

Uh oh!

This comment has been minimized.

bors commented Feb 26, 2022

Uh oh!

xFrednet commented Feb 27, 2022

Uh oh!

PrestonFrom commented Feb 27, 2022

Uh oh!

This comment has been minimized.

This comment has been minimized.

flip1995 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

PrestonFrom commented Mar 3, 2022

Uh oh!

This comment has been minimized.

flip1995 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PrestonFrom commented Mar 6, 2022

Uh oh!

This comment has been minimized.

This comment has been minimized.

flip1995 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PrestonFrom commented Mar 10, 2022

Uh oh!

This comment has been minimized.

This comment has been minimized.

flip1995 commented May 7, 2022

Uh oh!

oli-obk commented May 7, 2022

Uh oh!

oli-obk commented May 7, 2022

Uh oh!

flip1995 commented May 7, 2022

Uh oh!

compiler-errors commented May 7, 2022

Uh oh!

PrestonFrom commented May 7, 2022

Uh oh!

Mark-Simulacrum commented May 7, 2022

Uh oh!

bors commented May 7, 2022

Uh oh!

compiler-errors commented May 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors commented May 8, 2022

Uh oh!

bors commented May 8, 2022

Uh oh!

rust-timer commented May 8, 2022

PrestonFrom commented Feb 21, 2022 •

edited

Loading

compiler-errors commented May 7, 2022 •

edited

Loading