Given that we directly access the graph predecessors/successors in so many
other places, and sometimes must do so to satisfy the borrow checker, there is
little value in having this trivial helper method.
- Look up the node's predecessors only once
- Get rid of some overly verbose logging
- Explain why some nodes need physical counters
- Extract a helper method to create and set a physical node counter
These tests might have originally been useful as an implementation aid, but now
they don't provide enough value to justify the burden of updating them as the
underlying code changes.
The code they test is still exercised by the main end-to-end coverage tests.
Some of these cases currently don't occur in practice, but are included for
completeness, and to avoid having to add them later as branch coverage and
MC/DC coverage start building more complex expressions.
These assertions detect situations where a BCB node would have both a physical
counter and one or more in-edge counters/expressions.
For most BCBs that situation would indicate an implementation bug. However,
it's perfectly fine in the case of a BCB having an edge that loops back to
itself.
Given the complexity and risk involved in fixing the assertions, and the fact
that nothing relies on them actually being true, this patch just removes them
instead.
This makes it possible for two nodes/edges in the coverage graph to share the
same counter, without causing the instrumentor to inject unwanted duplicate
counter-increment statements.
`BcbBranch` represented an out-edge of a coverage graph node, but would
silently refer to a node instead in cases where that node only had one in-edge.
Instead we now refer to a graph edge as a `(from_bcb, to_bcb)` pair, or
sometimes as just one of those nodes when the other node is implied by the
surrounding context. The case of sole in-edges is handled by special code added
directly to `get_or_make_edge_counter_operand`.
This was previously a helper method in `MakeBcbCounters`, but putting it in the
graph lets us call it from `BcbBranch`, and gives us a more fine-grained
borrow.
In some cases we need to prepare a coverage expression that is the sum of an
arbitrary number of other terms. This patch simplifies the code paths that
build those sums.
This causes some churn in the mappings, because the previous code was building
its sums in a somewhat idiosyncratic order.
Now that this code path unconditionally calls `make_branch_counters`, we might
as well make that method responsible for creating the node's counter as well,
since it needs the resulting term anyway.
Historically, these errors existed so that the coverage debug code could dump
additional information before reporting a compiler bug. That debug code was
removed by #115962, so we can now simplify these methods by making them panic
when they detect a bug.
Even though expression details are now stored in the info structure, we still
need to inject `ExpressionUsed` statements into MIR, because if one is missing
during codegen then we know that it was optimized out and we can remap all of
its associated code regions to zero.
Coverage codegen can now allocate arrays based on the number of
counters/expressions originally used by the instrumentor.
The existing query that inspects coverage statements is still used for
determining the number of counters passed to `llvm.instrprof.increment`. If
some high-numbered counters were removed by MIR optimizations, the instrumented
binary can potentially use less memory and disk space at runtime.
Having to keep passing in a graph reference was a holdover from when the graph
was partly mutated during traversal. As of #114354 that is no longer necessary,
so we can simplify the traversal code by storing a graph reference as a field
in `TraverseCoverageGraphWithLoops`.
When these methods were originally written, I wasn't aware that
`newtype_index!` already supports addition with ordinary numbers, without
needing to unwrap and re-wrap.
The concrete type `CoverageSpan` is no longer used outside of the `spans`
module.
This is a separate patch to avoid noise in the preceding patch that actually
encapsulates coverage spans.
By encapsulating the coverage spans in a struct, we can change the internal
representation without disturbing existing call sites. This will be useful for
grouping coverage spans by BCB.
This patch includes some changes that were originally in #115912, which avoid
the need for a particular test to deal with coverage spans at all.
(Comments/logs referring to `CoverageSpan` are updated in a subsequent patch.)
