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.

Sign up for GitHub

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

Jump to bottom

Fix manual_unwrap_or false positive #13061

Merged
merged 2 commits into from
Jul 13, 2024
Merged

Fix manual_unwrap_or false positive #13061

merged 2 commits into from
Jul 13, 2024

Conversation

tesuji
Copy link
Contributor

@tesuji tesuji commented Jul 7, 2024

changelog: [manual_unwrap_or]: fix false positive of if let Option<T>

Closes #13018

@rustbot
Copy link
Collaborator

rustbot commented Jul 7, 2024

r? @dswij

rustbot has assigned @dswij.
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 Jul 7, 2024
@tesuji tesuji force-pushed the fix-map-unwrap-or-13018 branch from 74c40db to 31db1d5 Compare July 7, 2024 03:49
dswij
dswij approved these changes Jul 7, 2024
View reviewed changes
Copy link
Member

@dswij dswij left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@tesuji Do you want to look into the FIXME in this PR?

@tesuji
Copy link
Contributor Author

tesuji commented Jul 7, 2024 •
edited
Loading

I kinda think that FIXME about using dereferecing module is more like a question rather than an improvement.
And it's non-actionable because expr_adjustments(then_expr) is empty. Meaning that there are no magic defers or re-borrowing to make then_expr's type equals to let_expr.unwrap()'s type.
I'll remove the FIXME.

@tesuji tesuji force-pushed the fix-map-unwrap-or-13018 branch 2 times, most recently from 042cef1 to b19513c Compare July 7, 2024 13:38
@tesuji
Fix 13018: self should be T
c46c1f6 
with `Option::unwrap_or(self, T) -> T`.
@tesuji tesuji force-pushed the fix-map-unwrap-or-13018 branch from b19513c to c46c1f6 Compare July 7, 2024 13:42
@tesuji
Copy link
Contributor Author

tesuji commented Jul 13, 2024

Also cc rust-lang/rust#127629. In the future, I may try to suggest map_or instead unwrap_or.

@dswij
Copy link
Member

dswij commented Jul 13, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Jul 13, 2024

📌 Commit c46c1f6 has been approved by dswij

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jul 13, 2024

⌛ Testing commit c46c1f6 with merge e7d17e6...

@bors
Copy link
Contributor

bors commented Jul 13, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: dswij
Pushing e7d17e6 to master...

@bors bors merged commit e7d17e6 into rust-lang:master Jul 13, 2024
8 checks passed
@tesuji tesuji deleted the fix-map-unwrap-or-13018 branch July 13, 2024 23:34
@tesuji tesuji mentioned this pull request Jul 19, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dswij dswij dswij approved these changes

Assignees

@dswij dswij

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Nightly clippy: manual_unwrap_or false positive
4 participants
@tesuji @rustbot @dswij @bors