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

[incorrect_clone_impl_on_copy_type]: Do not lint if only has MaybeUninit fields #11089

Closed
wants to merge 2 commits into from

Conversation

Centri3
Copy link
Member

@Centri3 Centri3 commented Jul 3, 2023

Closes #11072
Closes #11071

changelog: Enhancement: [incorrect_clone_impl_on_copy_type]: Do not lint if only has MaybeUninit fields
changelog: Enhancement: [incorrect_clone_impl_on_copy_type]: Downgrade to complexity
changelog: Enhancement: [incorrect_clone_impl_on_copy_type]: Do not lint never-like enums

@rustbot
Copy link
Collaborator

rustbot commented Jul 3, 2023

r? @llogiq

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 3, 2023
Copy link
Contributor

@llogiq llogiq left a comment

Choose a reason for hiding this comment

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

I'm not sure about the lint group change, and there's one simplification I found. Otherwise this looks good to me.

Why complexity?

clippy_lints/src/incorrect_impls.rs Outdated Show resolved Hide resolved
@Centri3
Copy link
Member Author

Centri3 commented Jul 3, 2023

Because it seemed there was mostly agreement of it in #11072, I put it in complexity because there isn't really any other category that has a more similar meaning to correctness whilst still being warn-by-default.

In most cases, this is a complexity issue; like Self(self.0). Sometimes, it's correctness. It's just the safest bet, really.

@bors
Copy link
Collaborator

bors commented Jul 7, 2023

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

@bors
Copy link
Collaborator

bors commented Jul 13, 2023

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

@llogiq
Copy link
Contributor

llogiq commented Oct 23, 2023

I'm currently trying to catch up on my review queue. This looks good, pending a rebase.

@bors delegate+

@bors
Copy link
Collaborator

bors commented Oct 23, 2023

✌️ @Centri3, you can now approve this pull request!

If @llogiq told you to "r=me" after making some further change, please make that change, then do @bors r=@llogiq

@xFrednet
Copy link
Member

Hey this is triage, I'm closing this due to inactivity. Currently, @Centri3 sadly doesn't have the time to continue this implementation. If anyone is interested in continuing this PR, you're more than welcome to create a new PR and push it over the finish line. :D

Thank you to @Centri3 and the reviewers for the time, that you already put into this!

@rustbot label +S-inactive-closed -S-waiting-on-author -S-waiting-on-review

@xFrednet xFrednet closed this Mar 31, 2024
@rustbot rustbot added S-inactive-closed Status: Closed due to inactivity and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Mar 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive-closed Status: Closed due to inactivity
Projects
None yet
5 participants