Make source-based code coverage compatible with MIR inlining #83080
Conversation
tmiasko
force-pushed
the
inline-coverage
branch
from
297de96
to
cdb28e2
Compare
|
Oh, this test also should be removed (now that the warning is deleted): src/test/ui/mir/mir-inlining/inline-instrument-coverage-fail.rs |
|
Thanks. I removed now obsolete test for incompatibility, I also adjusted the computation of reachable functions to account for situation where function does not have its code generated directly, but is still reachable after being inlined into another. I will look into adding some test cases that combine code coverage and inlining. |
|
BTW I can do another review when done, but I don't have Compiler Team approval privileges, so you'll need someone to do that (likely @wesleywiser or @tmandry) to push to bors, when ready. |
When codegenning code coverage use the instance that coverage data was originally generated for, to ensure basic level of compatibility with MIR inlining.
Consider functions to be reachable for code coverage purposes, either when they reach the code generation directly, or indirectly as inlined part of another function.
tmiasko
force-pushed
the
inline-coverage
branch
from
35aa8c2
to
7d00591
Compare
This comment has been minimized.
This comment has been minimized.
tmiasko
force-pushed
the
inline-coverage
branch
from
7d00591
to
0d84e0b
Compare
|
Added support for compile-flags to coverage tests and used it to create a MIR inlining test case. |
| if let Some(previous_expression) = self.expressions[expression_index].replace(Expression { | ||
| lhs, | ||
| op, | ||
| rhs, | ||
| region: region.clone(), |
There was a problem hiding this comment.
Nit: Might be slightly nicer to extract a local for Expression { lhs, op, rhs ... } and then use it in both places instead of constructing twice.
|
@bors r+ |
|
📌 Commit 0d84e0b has been approved by |
bors
added
the
S-waiting-on-bors
Make source-based code coverage compatible with MIR inlining When codegenning code coverage use the instance that coverage data was originally generated for, to ensure basic level of compatibility with MIR inlining. Fixes rust-lang#83061
Rollup of 11 pull requests Successful merges: - rust-lang#82191 (Vec::dedup_by optimization) - rust-lang#82270 (Emit error when trying to use assembler syntax directives in `asm!`) - rust-lang#82434 (Add more links between hash and btree collections) - rust-lang#83080 (Make source-based code coverage compatible with MIR inlining) - rust-lang#83168 (Extend `proc_macro_back_compat` lint to `procedural-masquerade`) - rust-lang#83192 (ci/docker: Add SDK/NDK level 21 to android docker for 32bit platforms) - rust-lang#83204 (Simplify C compilation for Fortanix-SGX target) - rust-lang#83216 (Allow registering tool lints with `register_tool`) - rust-lang#83223 (Display error details when a `mmap` call fails) - rust-lang#83228 (Don't show HTML diff if tidy isn't installed for rustdoc tests) - rust-lang#83231 (Switch riscvgc-unknown-none-elf use lp64d ABI) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
When codegenning code coverage use the instance that coverage data was
originally generated for, to ensure basic level of compatibility with
MIR inlining.
Fixes #83061