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

Suggest if let on let refutable binding #65248

Merged
merged 1 commit into from Oct 13, 2019
Merged

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Oct 9, 2019

Fix #58385.

@rust-highfive
Copy link
Collaborator

r? @cramertj

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 9, 2019
}
err.note("for more information, visit \
Copy link
Member

Choose a reason for hiding this comment

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

Should this note perhaps be in the rustc --explain rather than inline here? it seems like it's a lot of extra text

@cramertj
Copy link
Member

cramertj commented Oct 9, 2019

r=me (fine either way WRT my comment above)

It's sort of unfortunate how much extra space these hints take up, but it seems like they provide useful additional information.

@estebank
Copy link
Contributor Author

estebank commented Oct 9, 2019

@cramertj I can turn the following

error[E0005]: refutable pattern in local binding: `std::i32::MIN..=-1i32` and `1i32..=std::i32::MAX` not covered
  --> $DIR/const-match-check.rs:31:24
   |
LL |     let x: [i32; { let 0 = 0; 0 }] = [];
   |                        ^ patterns `std::i32::MIN..=-1i32` and `1i32..=std::i32::MAX` not covered
   |
   = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
   = note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
help: you might want to use `if let` to ignore the variant that isn't matched
   |
LL |     let x: [i32; { if let 0 = 0 { /* */ } 0 }] = [];
   |                    ^^^^^^^^^^^^^^^^^^^^^^

into

error[E0005]: refutable pattern in local binding: `std::i32::MIN..=-1i32` and `1i32..=std::i32::MAX` not covered
  --> $DIR/const-match-check.rs:31:24
   |
LL |     let x: [i32; { let 0 = 0; 0 }] = [];
   |                    ----^-----
   |                    |   |
   |                    |   patterns `std::i32::MIN..=-1i32` and `1i32..=std::i32::MAX` not covered
   |                    help: you might want to use `if let` instead: `if let 0 = 0 { /* */ }`
   |
   = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
   = note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html

but I fear that readability actually suffers.

@estebank
Copy link
Contributor Author

estebank commented Oct 9, 2019

it seems like it's a lot of extra text

I kind of agree, but I fail to see what I can remove without compromising on onboarding experience. I think it makes sense merge this as is and after we have real life experience with it we can cut it back if it proves to be too annoying in real codebases.

@bors r=cramertj

@bors
Copy link
Contributor

bors commented Oct 9, 2019

📌 Commit 4bb1592 has been approved by cramertj

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 9, 2019
err.span_suggestion(
span,
"you might want to use `if let` to ignore the variant that isn't matched",
format!("if {} {{ /* */ }}", &snippet[..snippet.len() - 1]),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
format!("if {} {{ /* */ }}", &snippet[..snippet.len() - 1]),
format!("if {} {{ /* ... */ }}", &snippet[..snippet.len() - 1]),

@Centril
Copy link
Contributor

Centril commented Oct 10, 2019

but I fear that readability actually suffers.

That could be fixed by having some vertical space between the two inline labels/notes.

tmandry added a commit to tmandry/rust that referenced this pull request Oct 11, 2019
tmandry added a commit to tmandry/rust that referenced this pull request Oct 11, 2019
Rollup of 16 pull requests

Successful merges:

 - rust-lang#64337 (libstd: Fix typos in doc)
 - rust-lang#64986 (Function pointers as const generic arguments)
 - rust-lang#65048 (Added doc about behavior of extend on HashMap)
 - rust-lang#65191 (Add some regression tests)
 - rust-lang#65200 (Add ?Sized bound to a supertrait listing in E0038 error documentation)
 - rust-lang#65205 (Add long error explanation for E0568)
 - rust-lang#65240 (self-profiling: Add events for metadata loading (plus a small dep-tracking optimization))
 - rust-lang#65248 (Suggest `if let` on `let` refutable binding)
 - rust-lang#65252 (expand: Simplify expansion of derives)
 - rust-lang#65263 (Deduplicate is_{freeze,copy,sized}_raw)
 - rust-lang#65265 (Cleanup librustc mir err codes)
 - rust-lang#65266 (Mark Path::join as must_use)
 - rust-lang#65276 (Don't cc rust-lang/compiler for toolstate changes)
 - rust-lang#65277 (Query generator kind for error reporting)
 - rust-lang#65283 (stability: Do not use `buffer_lint` after lowering to HIR)
 - rust-lang#65289 (Fix suggested bound addition diagnostic)

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Oct 11, 2019
Rollup of 16 pull requests

Successful merges:

 - rust-lang#64337 (libstd: Fix typos in doc)
 - rust-lang#64986 (Function pointers as const generic arguments)
 - rust-lang#65048 (Added doc about behavior of extend on HashMap)
 - rust-lang#65191 (Add some regression tests)
 - rust-lang#65200 (Add ?Sized bound to a supertrait listing in E0038 error documentation)
 - rust-lang#65205 (Add long error explanation for E0568)
 - rust-lang#65240 (self-profiling: Add events for metadata loading (plus a small dep-tracking optimization))
 - rust-lang#65248 (Suggest `if let` on `let` refutable binding)
 - rust-lang#65252 (expand: Simplify expansion of derives)
 - rust-lang#65263 (Deduplicate is_{freeze,copy,sized}_raw)
 - rust-lang#65265 (Cleanup librustc mir err codes)
 - rust-lang#65266 (Mark Path::join as must_use)
 - rust-lang#65276 (Don't cc rust-lang/compiler for toolstate changes)
 - rust-lang#65277 (Query generator kind for error reporting)
 - rust-lang#65283 (stability: Do not use `buffer_lint` after lowering to HIR)
 - rust-lang#65289 (Fix suggested bound addition diagnostic)

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Oct 13, 2019
bors added a commit that referenced this pull request Oct 13, 2019
Rollup of 13 pull requests

Successful merges:

 - #65039 (Document missing deny by default lints)
 - #65069 (Implement Clone::clone_from for VecDeque)
 - #65165 (Improve docs on some char boolean methods)
 - #65248 (Suggest `if let` on `let` refutable binding)
 - #65250 (resolve: fix error title regarding private constructors)
 - #65295 (Move diagnostics code out of the critical path)
 - #65320 (Report `CONST_ERR` lint in external macros)
 - #65327 (replace the hand-written binary search with the library one)
 - #65339 (do not reference LLVM for our concurrency memory model)
 - #65357 (syntax: simplify maybe_annotate_with_ascription)
 - #65358 (simplify maybe_stage_features)
 - #65359 (simplify integer_lit)
 - #65360 (mbe: reduce panictry! uses.)

Failed merges:

r? @ghost
@bors bors merged commit 4bb1592 into rust-lang:master Oct 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reduce jargon in "refutable pattern in local binding" diagnostic
5 participants