Skip to content

[unnecessary_literal_unwrap]: Fix ICE on None.unwrap_or_default() - #11106

Merged
bors merged 2 commits into
rust-lang:masterfrom
syvb:literal_unwrap_ice
Jul 20, 2023
Merged

[unnecessary_literal_unwrap]: Fix ICE on None.unwrap_or_default()#11106
bors merged 2 commits into
rust-lang:masterfrom
syvb:literal_unwrap_ice

Conversation

@syvb

@syvb syvb commented Jul 4, 2023 •
edited by dswij
Loading

Copy link
Copy Markdown
Contributor

Fixes #11099
Fixes #11064

I'm running into #11099 (cc @y21) on my Rust codebase. Clippy ICEs on this code when evaluating the unnecessary_literal_unwrap lint:

fn main() {
    let val1: u8 = None.unwrap_or_default();
}

This fixes that ICE and adds an message specifically for that case:

error: used `unwrap_or_default()` on `None` value
  --> $DIR/unnecessary_literal_unwrap.rs:26:5
   |
LL |     None::<String>.unwrap_or_default();
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the `None` and `unwrap_or_default()`: `String::default()`

This PR also fixes the same ICE with None.unwrap_or_else (by giving the generic error message for the lint in that case).

changelog: Fix ICE in unnecessary_literal_unwrap on None.unwrap_or_default()

@rustbot

rustbot commented Jul 4, 2023

Copy link
Copy Markdown
Collaborator

r? @dswij

(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 4, 2023
@y21

y21 commented Jul 4, 2023

Copy link
Copy Markdown
Member

Nice, I was about to look into this one as well! ^^
This ICE also happens for unwrap_or and unwrap_or_else (forgot to mention it in the issue): https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=12873182c7a7d7ca7a9fb869b2a2b565

Can you handle these also and add tests for them?

@syvb
syvb force-pushed the literal_unwrap_ice branch 5 times, most recently from 0d94bbf to a80778c Compare July 7, 2023 14:14
@syvb

syvb commented Jul 8, 2023

Copy link
Copy Markdown
Contributor Author

I updated this to add specific error messages for unwrap_or and unwrap_or_else.

@bors

bors commented Jul 12, 2023

Copy link
Copy Markdown
Contributor

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

@bors

bors commented Jul 12, 2023

Copy link
Copy Markdown
Contributor

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

@syvb
syvb force-pushed the literal_unwrap_ice branch from a80778c to 8d258c1 Compare July 13, 2023 17:06
@Centri3

Centri3 commented Jul 20, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

This also seems to fix #11064 as well (it doesn't ICE anymore, but the suggestion is wrong).

Can you link the two by adding "Fixes #11064, fixes #11099" to the PR description? That way they're closed when this is merged.

dswij
dswij approved these changes Jul 20, 2023
View reviewed changes

@dswij dswij left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for this! Sorry for the slow review.

@dswij

dswij commented Jul 20, 2023

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented Jul 20, 2023

Copy link
Copy Markdown
Contributor

📌 Commit 8d258c1 has been approved by dswij

It is now in the queue for this repository.

@bors

bors commented Jul 20, 2023

Copy link
Copy Markdown
Contributor

⌛ Testing commit 8d258c1 with merge fca1f9a...

@bors

bors commented Jul 20, 2023

Copy link
Copy Markdown
Contributor

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

@bors
bors merged commit fca1f9a into rust-lang:master Jul 20, 2023
@flip1995 flip1995 added beta-nominated Nominated for backporting to the compiler in the beta channel. beta-accepted Accepted for backporting to the compiler in the beta channel. and removed beta-nominated Nominated for backporting to the compiler in the beta channel. labels Aug 14, 2023
@flip1995 flip1995 mentioned this pull request Aug 17, 2023
Merged
@flip1995

flip1995 commented Aug 17, 2023

Copy link
Copy Markdown
Member

rust-lang/rust#114937

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 17, 2023
@bors
Auto merge of rust-lang#114937 - flip1995:clippy_beta_backport, r=Mar…
8c60295 
…k-Simulacrum

[beta] Clippy backports for ICE fixes

This backports PRs to beta, that fix ICEs, some lint grouping and FP fixes. Namely:

- rust-lang/rust-clippy#11191
- rust-lang/rust-clippy#11172
- rust-lang/rust-clippy#11130
- rust-lang/rust-clippy#11106
- rust-lang/rust-clippy#11104
- rust-lang/rust-clippy#11077
- rust-lang/rust-clippy#11070 (This PR is not synced to the Rust repo yet, but I will open a separate PR to get it into `master`, before beta is branched: rust-lang#114938)
- rust-lang/rust-clippy#11069

Kind of a big backport, but most of it is tests.

r? `@Mark-Simulacrum`

cc `@Manishearth`
@xFrednet xFrednet removed the beta-accepted Accepted for backporting to the compiler in the beta channel. label Aug 18, 2023
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

8 participants

@syvb @rustbot @y21 @bors @Centri3 @dswij @flip1995 @xFrednet