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

Fix [unnecessary_lazy_eval] when type has significant drop #9750

Merged
merged 1 commit into from
Nov 22, 2022

Conversation

kraktus
Copy link
Contributor

@kraktus kraktus commented Oct 29, 2022

fix for #9427 (comment)

However current implementation gives too many false positive, rending the lint almost useless.

I don't know what's the best way to check if a type has a "significant" drop (in the common meaning, not the internal rustc one, for example Option<(u8, u8)> should not be considered significant)

changelog: Fix [unnecessary_lazy_eval] when type has significant drop

@rust-highfive
Copy link

r? @Manishearth

(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 Oct 29, 2022
@Manishearth
Copy link
Member

cc @rust-lang/clippy thoughts?

@xFrednet
Copy link
Member

xFrednet commented Nov 1, 2022

I would probably go with the rustc "significant" drop. In most cases, false negatives are better than false positives. But I have no strong opinion on this.

@llogiq
Copy link
Contributor

llogiq commented Nov 6, 2022

Yeah, the other benefit of following the rustc definition is that we'll be updated automatically if it ever changed.

@kraktus
Copy link
Contributor Author

kraktus commented Nov 13, 2022

Just to be clear, do you think the current fix would be enough applied as-is? (Documentation apart)

@kraktus kraktus marked this pull request as draft November 21, 2022 11:33
@kraktus kraktus force-pushed the lazy_eval branch 2 times, most recently from 27540a7 to ed183ee Compare November 21, 2022 11:38
@kraktus kraktus changed the title WIP: fix [unnecessary_lazy_eval] when type has significant drop Fix [unnecessary_lazy_eval] when type has significant drop Nov 21, 2022
@kraktus kraktus marked this pull request as ready for review November 21, 2022 11:40
@kraktus
Copy link
Contributor Author

kraktus commented Nov 21, 2022

Should be good to go 👍

@Manishearth
Copy link
Member

r? @xFrednet

@xFrednet
Copy link
Member

LGTM, thank you for the update and in general all the work you currently do around Clippy. It's really a big help!

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 22, 2022

📌 Commit ed183ee has been approved by xFrednet

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Nov 22, 2022

⌛ Testing commit ed183ee with merge 5595d7f...

@bors
Copy link
Collaborator

bors commented Nov 22, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: xFrednet
Pushing 5595d7f to master...

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
Development

Successfully merging this pull request may close these issues.

None yet

6 participants