-
Notifications
You must be signed in to change notification settings - Fork 13k
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
coverage: Add enums to accommodate other kinds of coverage mappings #119842
Conversation
This is less elegant in some ways, since we no longer visit a BCB's spans as a batch, but will make it much easier to add support for other kinds of coverage mapping regions (e.g. branch regions or gap regions).
(rustbot has picked a reviewer for you, use r? to override) |
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
let one = |a| std::iter::once(a); | ||
match *self { | ||
Self::Code(term) => one(term), | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this will work with variants that have multiple terms unless all of them are slices and you use https://doc.rust-lang.org/std/slice/fn.from_ref.html here (in which case you could proabably return a slice instead of an iterator)
But also not important, you can refactor to make it work when you add more variants
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of the variants supported by LLVM have exactly zero, one, or two terms.
So my plan is to eventually change this definition to something like:
let one = |a| [Some(a), None].into_iter().flatten()
That way the individual arms that use one
won't have to change.
(But I won't bother doing this until I actually add a variant that requires 0 or 2 terms.)
@bors r+ rollup |
coverage: Add enums to accommodate other kinds of coverage mappings Extracted from rust-lang#118305. LLVM supports several different kinds of coverage mapping regions, but currently we only ever emit ordinary “code” regions. This PR performs the plumbing required to add other kinds of regions as enum variants, but does not add any specific variants other than `Code`. The main motivation for this change is branch coverage, but it will also allow separate experimentation with gap regions and skipped regions, which might help in producing more accurate and useful coverage reports. --- `@rustbot` label +A-code-coverage
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#119448 (annotate-snippets: update to 0.10) - rust-lang#119813 (Silence some follow-up errors [2/x]) - rust-lang#119836 (chore: remove unnecessary blank line) - rust-lang#119841 (Remove `DiagnosticBuilder::buffer`) - rust-lang#119842 (coverage: Add enums to accommodate other kinds of coverage mappings) - rust-lang#119845 (rint: further doc tweaks) - rust-lang#119852 (give const-err4 a more descriptive name) - rust-lang#119853 (rustfmt.toml: don't ignore just any tests path, only root one) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#119842 - Zalathar:kind, r=oli-obk coverage: Add enums to accommodate other kinds of coverage mappings Extracted from rust-lang#118305. LLVM supports several different kinds of coverage mapping regions, but currently we only ever emit ordinary “code” regions. This PR performs the plumbing required to add other kinds of regions as enum variants, but does not add any specific variants other than `Code`. The main motivation for this change is branch coverage, but it will also allow separate experimentation with gap regions and skipped regions, which might help in producing more accurate and useful coverage reports. --- ``@rustbot`` label +A-code-coverage
After some quick experiments, I'm sad to report that LLVM's gap regions appear to be unusable. LLVM documentation suggests that they work like code regions, except that code regions always take priority over gap regions when determining line counts. In practice, that's not accurate. For example, replacing all of our code regions with gap regions ought to have no effect on the resulting coverage reports, but in fact it causes some (but not all) of our line counts to disappear, for reasons that I haven't fully determined. (For example, gap regions that span across multiple lines appear to be handled differently.) I suspect that the |
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#119448 (annotate-snippets: update to 0.10) - rust-lang#119813 (Silence some follow-up errors [2/x]) - rust-lang#119836 (chore: remove unnecessary blank line) - rust-lang#119841 (Remove `DiagnosticBuilder::buffer`) - rust-lang#119842 (coverage: Add enums to accommodate other kinds of coverage mappings) - rust-lang#119845 (rint: further doc tweaks) - rust-lang#119852 (give const-err4 a more descriptive name) - rust-lang#119853 (rustfmt.toml: don't ignore just any tests path, only root one) r? `@ghost` `@rustbot` modify labels: rollup
Extracted from #118305.
LLVM supports several different kinds of coverage mapping regions, but currently we only ever emit ordinary “code” regions. This PR performs the plumbing required to add other kinds of regions as enum variants, but does not add any specific variants other than
Code
.The main motivation for this change is branch coverage, but it will also allow separate experimentation with gap regions and skipped regions, which might help in producing more accurate and useful coverage reports.
@rustbot label +A-code-coverage