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

Add regression test for #12612 #12647

Closed

Conversation

GuillaumeGomez
Copy link
Member

Fixes #12612.
Fixes #12560.

Seems like this bug was fixed in #12535.

changelog: Add regression test for #12612 and #12560.

@rustbot
Copy link
Collaborator

rustbot commented Apr 8, 2024

r? @Jarcho

rustbot has assigned @Jarcho.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 8, 2024
@GuillaumeGomez GuillaumeGomez force-pushed the regression-12612 branch 2 times, most recently from 29ca5fd to 677615d Compare April 8, 2024 14:03
@Jarcho
Copy link
Contributor

Jarcho commented Apr 12, 2024

Both of those issues are only half fixed. map_clone still doesn't properly check the types involved.

@xFrednet
Copy link
Member

Hey @GuillaumeGomez , this is a ping from triage, since there hasn't been any activity in some time. Are you still planning to continue this implementation?

If you have any questions, you're always welcome to ask them in this PR or on Zulip.

@rustbot author

@rustbot rustbot 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 Jun 20, 2024
@GuillaumeGomez
Copy link
Member Author

I just confirmed again that the fix is done by adding a new test. I don't see what @Jarcho is referring to. Do you have an example where this bug can be reproduced by any chance?

@Jarcho
Copy link
Contributor

Jarcho commented Jul 3, 2024

The cause of both of those issues are the lint not verifying the types involved in the clone call. .map(|x: &X| Y::clone(...)) shouldn't be linted unless X and Y are the same type. #12535 happens to fix the issue when calling Arc::clone, but for an entirely different reason.

@Jarcho
Copy link
Contributor

Jarcho commented Jul 3, 2024

Looking into this again it does check the types correctly and there are already tests. This was actually fixed in #12282, not #12535.

@GuillaumeGomez
Copy link
Member Author

Closing then!

@GuillaumeGomez GuillaumeGomez deleted the regression-12612 branch July 17, 2024 18:35
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.

Wrong suggestion for clippy::map_clone Errorneous suggestion with map + Arc::clone of custom expression
4 participants