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

Add coverage to continue statements #84295

Merged
merged 2 commits into from
Apr 20, 2021
Merged

Conversation

richkadel
Copy link
Contributor

continue statements were missing coverage. This was particularly
noticeable in a match pattern that contained only a continue
statement, leaving the branch appear uncounted. This PR addresses the
problem and adds tests to prove it.

r? @tmandry
cc: @wesleywiser

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 18, 2021
`continue` statements were missing coverage. This was particularly
noticeable in a match pattern that contained only a `continue`
statement, leaving the branch appear uncounted. This PR addresses the
problem and adds tests to prove it.
And make the LocalDecl internal, to avoid needing to declare storage.
(For multiple `continue` stateuemtns, it must also be mutable.)
@@ -643,6 +650,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.cfg.start_new_block().unit()
}

// Add a dummy `Assign` statement to the CFG, with the span for the source code's `continue`
Copy link
Member

Choose a reason for hiding this comment

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

nit: the function itself doesn't know about continue

Comment on lines +10 to +15
9| | true => {
10| 10| continue;
11| | }
12| 0| _ => {
13| 0| x = 1;
14| 0| }
Copy link
Member

Choose a reason for hiding this comment

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

small detail, but can you think of why the beginning/end of the block have counters for the _ / x = 1 case, but not the continue?

@tmandry
Copy link
Member

tmandry commented Apr 20, 2021

This makes sense to me. Comments above shouldn't block merging, so

@bors r+

@bors
Copy link
Contributor

bors commented Apr 20, 2021

📌 Commit d1d7fb1 has been approved by tmandry

@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 Apr 20, 2021
@bors
Copy link
Contributor

bors commented Apr 20, 2021

⌛ Testing commit d1d7fb1 with merge b2c20b5...

@bors
Copy link
Contributor

bors commented Apr 20, 2021

☀️ Test successful - checks-actions
Approved by: tmandry
Pushing b2c20b5 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 20, 2021
@bors bors merged commit b2c20b5 into rust-lang:master Apr 20, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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.

6 participants