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

Fixes to manual_let_else's divergence check #11787

Merged
merged 2 commits into from Nov 12, 2023

Conversation

Jarcho
Copy link
Contributor

@Jarcho Jarcho commented Nov 9, 2023

A few changes to the divergence check in manual_let_else and moves it the implementation to clippy_utils since it's generally useful:

  • Handle internal break and continue expressions.
    e.g. The first loop is divergent, but the second is not.
    {
        loop {
            break 'outer;
        };
    }
    {
        loop {
            break;
        };
    }
  • Match rust's definition of divergence which is defined via the type system.
    e.g. The following is not considered divergent by rustc as the inner block has a result type of ():
    {
        'a: {
            panic!();
            break 'a;
        };
    }
  • Handle when adding a single semicolon would make the expression divergent.
    e.g. The following would be a divergent if a semicolon were added after the if expression:
    { if panic!() { 0 } else { 1 } }

changelog: None

@rustbot
Copy link
Collaborator

rustbot commented Nov 9, 2023

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 Nov 9, 2023
@Jarcho Jarcho changed the title Divergence check Fixes to manual_let_else's divergence check Nov 9, 2023
Copy link
Member

@dswij dswij left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for this!

@dswij
Copy link
Member

dswij commented Nov 12, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 12, 2023

📌 Commit a44bb07 has been approved by dswij

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Nov 12, 2023

⌛ Testing commit a44bb07 with merge 6a15f3b...

@bors
Copy link
Collaborator

bors commented Nov 12, 2023

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

1 similar comment
@bors
Copy link
Collaborator

bors commented Nov 12, 2023

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

@bors bors merged commit 6a15f3b into rust-lang:master Nov 12, 2023
5 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.

None yet

4 participants