MIR InstrumentCoverage - Can the source_info.span for FakeRead statements be more consistent?

[richkadel]

Rust's LLVM InstrProf-based source code coverage implementation instruments Rust code via the MIR pass InstrumentCoverage. Most criteria for identifying coverage regions and counter locations are very general, based on Control Flow Graph (CFG) analysis of the MIR, and a fairly straightforward mapping of MIR Statements and Terminators to their source code regions (Spans).

StatementKind::FakeRead are an exception, requiring special handling for at lease one case: When its cause is ForGuardBinding, the Statements source_info.span seems to be wrong.

Example:

    match somenum {
        x if x < 1 => { ... }
    }...

The BasicBlock within the match arm code included one of these statements, but the span
for it covered the literal number 1 in this source. The actual FakeRead statement, and its components, have nothing to do with that source span:

    FakeRead(ForGuardBinding, _4);

where _4 is:

    _4 = &_1; (at the span for the first `x`)

and _1 is the Place for somenum.

The current workaround is to ignore these Statements, and rely on other Statements for coverage (which appears to provide reasonable results):

fn filtered_statement_span(statement: &'a Statement<'tcx>, body_span: Span) -> Option<Span> {
    match statement.kind {
...
        StatementKind::FakeRead(cause, _) if cause == FakeReadCause::ForGuardBinding => None,

But workarounds like this one may be incomplete or error-prone, and could be hard to maintain in the future. Plus, if the given span is actually wrong, this bug would have been introduced in an earlier phase (perhaps lowering AST to HIR, or a later pass), the invalid span may trigger other compiler issues.

The arm code BasicBlock already has its own assignment for x itself, _3 = 1, and I've
decided it's reasonable for that span (even though outside the arm code) to be part of
the counted coverage of the arm code execution, but I can't justify including the literal
1 in the arm code. I'm pretty sure that, if the FakeRead(ForGuardBinding) has a
purpose in codegen, it's probably in the right BasicBlock, but if so, the Statements
source_info.span can't be right.

If the span can be corrected, the match pattern for this special case can be removed.