coverage: Make #[coverage(..)] apply recursively to nested functions
#126721
Conversation
rustbot
added
S-waiting-on-review
|
Based on this implementation, it should be fairly simple to permit coverage attributes on things like EDIT: It was easy enough that I just did it, so this implementation now also permits coverage attributes on mod and impl. |
Zalathar
force-pushed
the
nested-cov-attr
branch
7 times, most recently
from
207b8d1
to
8115343
Compare
|
☔ The latest upstream changes (presumably #126891) made this pull request unmergeable. Please resolve the merge conflicts. |
Zalathar
force-pushed
the
nested-cov-attr
branch
2 times, most recently
from
522b0bb
to
f0f220b
Compare
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
|
#126682 has merged, so I've rebased this and marked it as ready. |
| // Check the parent def (and so on recursively) until we find an | ||
| // enclosing attribute or reach the crate root. | ||
| Some(parent) => tcx.coverage_attr_on(parent), |
There was a problem hiding this comment.
Is it fine for this query to just call itself recursively (via the query system) to walk up the def tree, or is there something special I have to do here?
There was a problem hiding this comment.
Presumably it'd make sense to do some sort of path compression/memoisation so that the query is only linear in the number of items, rather than quadratic. But not quite sure what exists to do that in the compiler.
(Each item performs a number of queries equivalent to their number of parents, rather than just one to check their immediate parent which is cached.)
There was a problem hiding this comment.
Given that I'm calling tcx.coverage_attr_on (and not recursing on fn coverage_attr_on directly), my understanding is that the query system will memoize the result.
So individual functions will only recurse until they hit something (typically the enclosing mod/impl) for which there is a previously-memoized result.
There was a problem hiding this comment.
Yea this is totally fine. We do this for a lot of queries (e.g. generics_of)
These attributes apply to all enclosed functions/methods/closures, unless explicitly overridden by another coverage attribute.
Zalathar
force-pushed
the
nested-cov-attr
branch
from
f0f220b
to
7f37f8a
Compare
|
r? @oli-obk so the intent is for nested modules also to use the attribute from the outer module? This is a new concept in Rust iirc, so maybe add it as an important point on the tracking issue for T-lang to talk about during stabilization |
|
The current behaviour (scanning all the way up to the crate root) wasn't my original intention, but as I implemented the feature it ended up being (a) easier, and (b) plausibly the right design, so I decided to stick with it until/unless the team decides otherwise. |
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
coverage: Make `#[coverage(..)]` apply recursively to nested functions This PR makes the (currently-unstable) `#[coverage(off)]` and `#[coverage(on)]` attributes apply recursively to all nested functions/closures, instead of just the function they are directly attached to. Those attributes can now also be applied to modules and to impl/impl-trait blocks, where they have no direct effect, but will be inherited by all enclosed functions/closures/methods that don't override the inherited value. --- Fixes rust-lang#126625.
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#125016 (Update compiler_builtins to 0.1.113) - rust-lang#126571 (Less `maybe_whole_expr`, take 2) - rust-lang#126721 (coverage: Make `#[coverage(..)]` apply recursively to nested functions) - rust-lang#126928 (Some `Nonterminal` removal precursors) - rust-lang#126929 (Remove `__rust_force_expr`.) - rust-lang#126941 (Migrate `run-make/llvm-ident` to `rmake.rs`) - rust-lang#126970 (Simplify `str::clone_into`) - rust-lang#126980 (set self.is_known_utf8 to false in extend_from_slice) - rust-lang#126983 (Remove `f16` and `f128` ICE paths from smir) r? `@ghost` `@rustbot` modify labels: rollup
coverage: Make `#[coverage(..)]` apply recursively to nested functions This PR makes the (currently-unstable) `#[coverage(off)]` and `#[coverage(on)]` attributes apply recursively to all nested functions/closures, instead of just the function they are directly attached to. Those attributes can now also be applied to modules and to impl/impl-trait blocks, where they have no direct effect, but will be inherited by all enclosed functions/closures/methods that don't override the inherited value. --- Fixes rust-lang#126625.
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#125016 (Update compiler_builtins to 0.1.113) - rust-lang#126571 (Less `maybe_whole_expr`, take 2) - rust-lang#126692 (patch `rust-lld` and `ld.lld` on NixOS) - rust-lang#126721 (coverage: Make `#[coverage(..)]` apply recursively to nested functions) - rust-lang#126928 (Some `Nonterminal` removal precursors) - rust-lang#126929 (Remove `__rust_force_expr`.) - rust-lang#126970 (Simplify `str::clone_into`) - rust-lang#126980 (set self.is_known_utf8 to false in extend_from_slice) - rust-lang#126983 (Remove `f16` and `f128` ICE paths from smir) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 6 pull requests Successful merges: - rust-lang#126571 (Less `maybe_whole_expr`, take 2) - rust-lang#126721 (coverage: Make `#[coverage(..)]` apply recursively to nested functions) - rust-lang#126928 (Some `Nonterminal` removal precursors) - rust-lang#126929 (Remove `__rust_force_expr`.) - rust-lang#126980 (set self.is_known_utf8 to false in extend_from_slice) - rust-lang#126983 (Remove `f16` and `f128` ICE paths from smir) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#126721 - Zalathar:nested-cov-attr, r=oli-obk coverage: Make `#[coverage(..)]` apply recursively to nested functions This PR makes the (currently-unstable) `#[coverage(off)]` and `#[coverage(on)]` attributes apply recursively to all nested functions/closures, instead of just the function they are directly attached to. Those attributes can now also be applied to modules and to impl/impl-trait blocks, where they have no direct effect, but will be inherited by all enclosed functions/closures/methods that don't override the inherited value. --- Fixes rust-lang#126625.
This PR makes the (currently-unstable)
#[coverage(off)]and#[coverage(on)]attributes apply recursively to all nested functions/closures, instead of just the function they are directly attached to.Those attributes can now also be applied to modules and to impl/impl-trait blocks, where they have no direct effect, but will be inherited by all enclosed functions/closures/methods that don't override the inherited value.
Fixes #126625.