Add a new CoverageKind::Branch to MIR

[Swatinem]

This adds a couple intermediate types, and threads branch coverage from MIR through to codegen, and emits branch coverage regions via LLVM.

Based on top of #114399 and #115058, and so far untested, as we do not (yet) generate any branches in MIR.

[rustbot]

r? @davidtwco

(rustbot has picked a reviewer for you, use r? to override)

[Zalathar]

@rustbot label +A-code-coverage

[Swatinem]

Rebased on top of #114399 which makes the changes here a lot simpler.

[Swatinem]

A first proof of concept seems to be functional and it outputs the following counters / mappings (see 7f985c4):

- Branch at (prev + 2, 8) to (start + 0, 12)
    true  = c1
    false = (c0 - c1)

As well as llvm-cov output:

  ------------------
  |  Branch (LL:13): [True: 1, False: 5]
  ------------------

There are still a couple of problems with this POC that I will take a look at this week:

• code_regions are not yet correct for macros and derives and match statements.
• llvm-cov is also outputting different lines for generic instantiation, but I believe that is rather the way how llvm-cov reports these things.
• The column numbers seem to be way off sometimes, reporting thing like Branch (LL:192675): [True: 0, False: 0].
• This still needs to be hooked up to the existing expression simplification, as it is currently outputting (Zero - Zero) expressions for unreachable branches.
• It seems like the code is outputting duplicates for some Code(Zero) regions.

[Zalathar]

I was surprised to see changes to coverage-dump, because it should already work with branch regions as-is. (I tested it against a C program compiled with clang.)

In the existing implementation, it prints the raw true/false counters inline, and then also expands expressions (if any) on separate lines as needed.

[Zalathar]

To save anyone else the trouble of checking whether this is accidentally stable (it's not):

New values of InstrumentCoverage are automatically treated as nightly-only unless they are explicitly added to the check shown below on line 2674, so Branch is correctly treated as a nightly-only value. 👍

rust/compiler/rustc_session/src/config.rs

Lines 2665 to 2685 in c6f5495

	// Handle both `-Z instrument-coverage` and `-C instrument-coverage`; the latter takes
	// precedence.
	match (cg.instrument_coverage, unstable_opts.instrument_coverage) {
	(Some(ic_c), Some(ic_z)) if ic_c != ic_z => {
	handler.early_error(
	"incompatible values passed for `-C instrument-coverage` \
	and `-Z instrument-coverage`",
	);
	}
	(Some(InstrumentCoverage::Off | InstrumentCoverage::All), _) => {}
	(Some(_), _) if !unstable_opts.unstable_options => {
	handler.early_error("`-C instrument-coverage=except-*` requires `-Z unstable-options`");
	}
	(None, None) => {}
	(None, ic) => {
	handler
	.early_warn("`-Z instrument-coverage` is deprecated; use `-C instrument-coverage`");
	cg.instrument_coverage = ic;
	}
	_ => {}
	}

[Swatinem]

Also, I haven’t yet hooked up the option, so it doesn’t do anything right now, yet.

[Swatinem]

In the existing implementation, it prints the raw true/false counters inline, and then also expands expressions (if any) on separate lines as needed.

I didn’t know this was intentional, as the inline Debug output looks a bit untidy.
I reverted these changes, but renamed the identifiers to r#true to have slightly tidier output.

[Zalathar]

In that case, I’ll switch over to r#true and r#false in the original branch as well.

How do you feel about only showing the resolved lines when the true/false counter is an expression, versus always showing them? I chose to hide them (to keep the output compact), but if you think always showing them is better overall, I can make that change in my branch too.

[Swatinem]

How do you feel about only showing the resolved lines when the true/false counter is an expression, versus always showing them? I chose to hide them (to keep the output compact), but if you think always showing them is better overall, I can make that change in my branch too.

IMO for Branch it does make sense to have the true/false expressions on separate lines, but it does not make such a difference bth.

[davidtwco]

Use @rustbot ready when you want this to be reviewed.

[Dylan-DPC]

@Swatinem any updates on this? thanks

[Swatinem]

I will close this PR for now.
Smaller parts of it have landed independently, and @Zalathar is still hard at work overhauling the guts of coverage instrumentation, which I am keeping track of and reviewing. Apart from that I don’t have too much time to dedicate to this right now, though I might pick that up again once I do.