Known limitations of branch coverage instrumentation about rust HOT 8 OPEN

Based on my investigations so far:

• If-let (and friends) and let-else are relatively easy to support. I have implementations of them, though I haven't filed PRs yet because I'm wary of making any changes that will accidentally make the other branching constructs harder to support.
• Match arms are tricky to support at the moment. Conceptually, it makes sense to treat each arm (except the last) as a branch, with the true arm leading to the guard check (if any) followed by the corresponding body, and the false arm leading to the test for the next arm. However, that control-flow structure doesn't necessarily exist in the lowering from THIR to MIR, so we don't always have a suitable place to insert block markers.
  • I have a sketch of a plan for how to deal with this, by inserting block markers in all the true arms, and then using coverage expressions to create synthetic counts for the false arms. However, this will require some non-trivial reshuffling of how the InstrumentCoverage MIR pass currently deals with branch coverage information.
• The try operator desugars to a match expression, so if we support those, then we should be close to supporting try as well.
  • The caveat here is that we currently ignore all branches that come from expansion of any kind, so once we support match arms, we'll need to deal with that limitation as well.
• Await seems complicated enough that I'm inclined to put it in the too-hard basket for now.

from rust.

There's also some discussion at #79649 (comment) about whether the right-hand-side of a lazy boolean expression (&&/||) should be treated as a “branch” condition, even when it's not part of an if condition.

My view is that “branch coverage” should not be in the business of instrumenting things that clearly aren't branches at the language level, at least not by default. There are valid use-cases for that behaviour, but if it exists then it should require some additional level of opt-in beyond just enabling branch coverage.

(If people want to discuss this point further, I would suggest creating a separate issue for that discussion.)

from rust.

Is there a draft folk for if-let pattern?

I have tried a bit and found a possibly feasible way to insert block markers, see a naive draft.

Because if let would be taken as match guard this method may solve them together (not sure, I just check it could find the true/false blocks for code like if let (En::A(Some(a)) | En::B(a), En::C(b)) = _.

By this then BranchSpan might need to change its true_marker and false_marker to Vec<BlockMarkerId>.

from rust.

I managed to get my match-arm idea working, so I'll try to tie up the loose ends and post it as a PR.

from rust.

#124154 shows my current approach to instrumenting match arms for branch coverage.

from rust.

I now have draft PRs for if-let, let-else, and match arms (without or-patterns).

from rust.

Noticed the match arms pay attention to occasion like comment at first line of make_expression in counters.rs, I think it may be better put a note here rather reviews of the current two prs.

Many redundant expressions like this may be introduced by match arms.
The root cause is rustc may generate several test blocks for certain pattern. For example, code (A | B, C | D) will generate CFG like code

if match A {
    if match C {
        // Assume here is BcbCounter 1
    }  else if match D {
        // ...
    }
} else if match B {
    if match C {
        // Assume here is BcbCounter 2
    }  else if match D {
        // ...
    }
}

The true_term of C apparently should be BcbCounter 1 + BcbCounter 2. Pattern (A | B, C | D, E | F) even make terms of E more sophisticated. Actually they are just composition of several BcbCounter::Counter thus there probably many of intermediate expressions can be optimized out.

However, given that Expressions does not have runtime overhead the main side effect of these redundant expressions might only make people who read mir directly distressed. Further if we wanted to minimize the number of expressions, probably do it in llvm is better.

from rust.

The true_term of C apparently should be BcbCounter 1 + BcbCounter 2. Pattern (A | B, C | D, E | F) even make terms of E more sophisticated. Actually they are just composition of several BcbCounter::Counter thus there probably many of intermediate expressions can be optimized out.

This might not be "apparent". We also can view C as several branches sharing one span. By this view most work could be simplified but users would be confused with many branches in llvm-cov reports.

from rust.