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

Only point out non-diverging arms for match suggestions #121146

Merged
merged 2 commits into from
Feb 16, 2024

Conversation

compiler-errors
Copy link
Member

Fixes #121144

There is no reason to point at diverging arms, which will always coerce to whatever is the match block's evaluated type.

This also removes the suggestion from #106601, since as I pointed out in #72634 (comment) the added suggestion is not firing in the right cases, but instead only when one of the match arms already actually evaluates to ().

r? estebank

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 15, 2024
@rust-log-analyzer

This comment has been minimized.

@@ -20,7 +20,7 @@ fn opt() -> Option<()> {
fn main() {
match opt() {
Some(()) => &S,
None => &R, //~ ERROR E0308
Copy link
Member Author

Choose a reason for hiding this comment

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

whoops, didn't mean to touch this test

@rust-log-analyzer

This comment has been minimized.

@estebank
Copy link
Contributor

estebank commented Feb 15, 2024

r=me after fixing ci

I find it a bit redundant to point to all the non-! prior arms, but limiting to 4 might be a good enough compromise between clarity and verbosity.

@compiler-errors
Copy link
Member Author

I find it a bit redundant to point to all the non-! prior arms, but limiting to 4 might be a good enough compromise between clarity and verbosity.

We already limit to 4

@compiler-errors
Copy link
Member Author

idk why CI didn't restart

@compiler-errors
Copy link
Member Author

@bors r=estebank

@bors
Copy link
Contributor

bors commented Feb 15, 2024

📌 Commit 6018e21 has been approved by estebank

It is now in the queue for this repository.

@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 Feb 15, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 15, 2024
…rms, r=estebank

Only point out non-diverging arms for match suggestions

Fixes rust-lang#121144

There is no reason to point at diverging arms, which will always coerce to whatever is the match block's evaluated type.

This also removes the suggestion from rust-lang#106601, since as I pointed out in rust-lang#72634 (comment) the added suggestion is not firing in the right cases, but instead only when one of the match arms already *actually* evaluates to `()`.

r? estebank
oli-obk added a commit to oli-obk/rust that referenced this pull request Feb 15, 2024
…rms, r=estebank

Only point out non-diverging arms for match suggestions

Fixes rust-lang#121144

There is no reason to point at diverging arms, which will always coerce to whatever is the match block's evaluated type.

This also removes the suggestion from rust-lang#106601, since as I pointed out in rust-lang#72634 (comment) the added suggestion is not firing in the right cases, but instead only when one of the match arms already *actually* evaluates to `()`.

r? estebank
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 15, 2024
…llaumeGomez

Rollup of 13 pull requests

Successful merges:

 - rust-lang#118264 (Optimize `VecDeque::drain` for (half-)open ranges)
 - rust-lang#120741 (Make `io::BorrowedCursor::advance` safe)
 - rust-lang#120777 (Bump Unicode to version 15.1.0, regenerate tables)
 - rust-lang#120971 (Fix comment in core/src/str/validations.rs)
 - rust-lang#121034 (Improve wording of `static_mut_ref`)
 - rust-lang#121095 (Add extra indent spaces for rust-playground link)
 - rust-lang#121109 (Add an ErrorGuaranteed to ast::TyKind::Err (attempt 2))
 - rust-lang#121119 (Make `async Fn` trait kind errors better)
 - rust-lang#121141 (Fix closure kind docs)
 - rust-lang#121145 (Update aarch64 target feature docs to match LLVM)
 - rust-lang#121146 (Only point out non-diverging arms for match suggestions)
 - rust-lang#121147 (Avoid debug logging entire MIR body)
 - rust-lang#121155 (doc: add note about panicking examples for strict_overflow_ops)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 16, 2024
…llaumeGomez

Rollup of 10 pull requests

Successful merges:

 - rust-lang#120777 (Bump Unicode to version 15.1.0, regenerate tables)
 - rust-lang#120971 (Fix comment in core/src/str/validations.rs)
 - rust-lang#121095 (Add extra indent spaces for rust-playground link)
 - rust-lang#121109 (Add an ErrorGuaranteed to ast::TyKind::Err (attempt 2))
 - rust-lang#121119 (Make `async Fn` trait kind errors better)
 - rust-lang#121141 (Fix closure kind docs)
 - rust-lang#121145 (Update aarch64 target feature docs to match LLVM)
 - rust-lang#121146 (Only point out non-diverging arms for match suggestions)
 - rust-lang#121147 (Avoid debug logging entire MIR body)
 - rust-lang#121155 (doc: add note about panicking examples for strict_overflow_ops)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 77eaa80 into rust-lang:master Feb 16, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 16, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 16, 2024
Rollup merge of rust-lang#121146 - compiler-errors:ignore-diverging-arms, r=estebank

Only point out non-diverging arms for match suggestions

Fixes rust-lang#121144

There is no reason to point at diverging arms, which will always coerce to whatever is the match block's evaluated type.

This also removes the suggestion from rust-lang#106601, since as I pointed out in rust-lang#72634 (comment) the added suggestion is not firing in the right cases, but instead only when one of the match arms already *actually* evaluates to `()`.

r? estebank
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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The "this and all prior arms are found to be of type" diagnostic is less helpful for many unreachable branches
5 participants