Removes unneeded check of #[no_coverage] in mapgen
#84875
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rust-highfive
added
the
S-waiting-on-review
richkadel
force-pushed
the
no-coverage-dont-check-unused
branch
from
814c43c to
997fac2
Compare
This comment has been minimized.
This comment has been minimized.
richkadel
force-pushed
the
no-coverage-dont-check-unused
branch
from
997fac2 to
28aeb7f
Compare
richkadel
force-pushed
the
no-coverage-dont-check-unused
branch
from
28aeb7f to
bad6f9b
Compare
this comment no longer applies |
tmandry
reviewed
May 6, 2021
src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.closure.txt
Outdated
Show resolved
Hide resolved
richkadel
force-pushed
the
no-coverage-dont-check-unused
branch
from
bad6f9b to
34d2f6c
Compare
And adds tests to validate it still works. There is an anticipated feature request to support a compiler flag that only adds coverage for specific files (or perhaps mods). As I thought about where that change would need to be supported, I realized that checking the attribute in mapgen (for unused functions) was unnecessary. The unused functions are only synthesized if they have MIR coverage, and functions with the `no_coverage` attribute will not have been instrumented with MIR coverage statements in the first place. New tests confirm this. Also, while adding tests, I updated resolved comments and FIXMEs in other tests.
richkadel
force-pushed
the
no-coverage-dont-check-unused
branch
from
34d2f6c to
cd3a8c1
Compare
|
@bors r+ |
|
📌 Commit cd3a8c1 has been approved by |
bors
added
S-waiting-on-bors
|
@bors rollup |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
May 7, 2021
Rollup of 9 pull requests Successful merges: - rust-lang#84779 (Add support for --test-args to cargotest) - rust-lang#84781 (Don't check bootstrap artifacts by default) - rust-lang#84787 (bump deps) - rust-lang#84815 (Update coverage docs and command line help) - rust-lang#84875 (Removes unneeded check of `#[no_coverage]` in mapgen) - rust-lang#84897 (Coverage instruments closure bodies in macros (not the macro body)) - rust-lang#84911 (Retry clang+llvm download) - rust-lang#84972 (CTFE inbounds-error-messages tweak) - rust-lang#84990 (Sort rustdoc-gui tests) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There is an anticipated feature request to support a compiler flag that
only adds coverage for specific files (or perhaps mods). As I thought
about where that change would need to be supported, I realized that
checking the attribute in mapgen (for unused functions) was unnecessary.
The unused functions are only synthesized if they have MIR coverage, and
functions with the
no_coverageattribute will not have beeninstrumented with MIR coverage statements in the first place.
New tests confirm this.
Also, while adding tests, I updated resolved comments and FIXMEs in
other tests, and expanded comments and tests on one remaining issue that
is still not resolved.
r? @tmandry
cc: @wesleywiser