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: option_if_let_else FP when value partially moved #14209

Merged
merged 1 commit into from
Mar 19, 2025

Conversation

profetia
Copy link
Contributor

@profetia profetia commented Feb 13, 2025

fixes #13964

The lint option_map_unwrap_or used to have a similar issue in #10579, so I borrowed its solution to fix this one.

changelog: [option_if_let_else]: fix FP when value partially moved

@rustbot
Copy link
Collaborator

rustbot commented Feb 13, 2025

r? @llogiq

rustbot has assigned @llogiq.
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 Feb 13, 2025
@profetia
Copy link
Contributor Author

r? clippy

@rustbot rustbot assigned blyxyas and unassigned llogiq Feb 24, 2025
@profetia
Copy link
Contributor Author

profetia commented Mar 7, 2025

r? clippy

Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

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

This PR has changed my mind in introducing new visitors, althought it still has to be done very carefully to avoid needless work / bugs, this use is very elegant.

Some documentation would be greatly appreciated, I left you some suggestions.
Thanks for the contribution! ❤️

@blyxyas
Copy link
Member

blyxyas commented Mar 14, 2025

I've just checked lintcheck, and it seems that this is removing a lot of real positives :/

@profetia profetia force-pushed the issue13964 branch 2 times, most recently from 6d6363f to c7d776b Compare March 17, 2025 06:18
@profetia
Copy link
Contributor Author

I've just checked lintcheck, and it seems that this is removing a lot of real positives :/

Should be better now.

@profetia
Copy link
Contributor Author

r? @blyxyas

@rustbot rustbot assigned blyxyas and unassigned flip1995 Mar 17, 2025
Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! ❤️

@blyxyas blyxyas added this pull request to the merge queue Mar 19, 2025
Merged via the queue into rust-lang:master with commit 31497d6 Mar 19, 2025
11 checks passed
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.

Suggested map_or_else when value partialy moved
5 participants