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

Propagate unreachability in coverage #137

Merged
merged 2 commits into from
Jul 23, 2018
Merged

Conversation

elinorbgr
Copy link
Contributor

This is an attempt at propagating upward unreachability in processed files. The rationale being that code that can be statically determined to be unreachable cannot be tested, and should not be marked as missed in coverage.

The algorithm for propagating coverage I implemented is the following:

  • unreachable!() macro invocation is unreachable
  • a list of statements is unreachable iff at least one of the statements is unreachable
  • a loop is unreachable iff its body is unreachable
  • an if block is unreachable iff both its branches are unreachable
    • in particular an if with no else branch is reachable
  • a match block is unreachable iff all its branches are unreachable
    • in particular, a match with no branch is unreachable, as it must be matching on an uninhabited type like an empty enum
  • a function or method with an unreachable body is unreachable itself

This is a conservative implementation with minimal assumptions, I probably missed a few heuristics that could propagate more unreachability.

@xd009642
Copy link
Owner

Cool I'll have a look over this now. One thought/concern is in instances where this leads to big blocks of code marked as not coverable whether that would be intuitive to users or whether they'd assume this is an issue in tarpaulin

Copy link
Owner

@xd009642 xd009642 left a comment

Choose a reason for hiding this comment

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

All looks good, I'd just like one small test to make sure a list of statements is properly ignored 👍

_ =>{}
}
}
}


fn process_statements(stmts: &[Stmt], ctx: &Context, analysis: &mut LineAnalysis) {
fn process_statements(stmts: &[Stmt], ctx: &Context, analysis: &mut LineAnalysis) -> SubResult {
// in a list of statements, if any of them is unreachable, the whole list is
Copy link
Owner

Choose a reason for hiding this comment

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

Can there be a test for this. Just something simple like

fn test_unreachable() {
    let x: u32 = foo();
    if x > 5 { 
        bar();
    }
    unreachable!();
}

@elinorbgr
Copy link
Contributor Author

Added. It was a good finding: adding this test allowed me to find a bug I did not anticipate, statement macro calls like unreachable!(); are considered as items, not semicolon-ended-expressions...

@xd009642
Copy link
Owner

Ah I've been bitten by that one before, always forget it's a thing.

This all looks good to me, so merging in 👍

@xd009642 xd009642 merged commit 11cda26 into xd009642:develop Jul 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants