Adds two source span utility functions used in source-based coverage #76003
Conversation
This file contains 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
|
Looks good to me, though I'd like to run it by someone on t-compiler first. r? @wesleywiser |
tmiasko
reviewed
Aug 27, 2020
richkadel
force-pushed
the
llvm-coverage-map-gen-6b.4
branch
2 times, most recently
from
e68136a
to
37562b0
Compare
wesleywiser
approved these changes
Aug 28, 2020
|
@bors r+ rollup |
|
📌 Commit 37562b024d4a6f2662bd99e347a36ec8b041afff has been approved by |
bors
added
S-waiting-on-bors
|
☔ The latest upstream changes (presumably #74862) made this pull request unmergeable. Please resolve the merge conflicts. |
bors
added
S-waiting-on-author
This comment has been minimized.
This comment has been minimized.
|
@joshtriplett Can you expand on that? I'm not seeing why this would need to. |
richkadel
force-pushed
the
llvm-coverage-map-gen-6b.4
branch
from
37562b0
to
83adfeb
Compare
|
@wesleywiser - I rebased everything in the PR chain after the massive directory path changes that just landed. (No real changes required.) I also don't see why a crater run would be useful, since this only adds two non-mutating methods (that no existing crates would have called) and doesn't add any data elements or anything. Can we get these queued back up in bors (this PR and also #76002 )? Also if you haven't already, and have time, please take a look at #76074 and #76004 ? |
|
@wesleywiser @richkadel Sorry, was reviewing several PRs on mobile, and accidentally posted that comment to the wrong one. That comment was meant for #76010 . |
|
@bors r+ |
|
📌 Commit 83adfebc369fd7f7ac2bbc563a04ebb72d366dcd has been approved by |
bors
added
S-waiting-on-bors
|
@bors rollup=always |
`span.is_empty()` - returns true if `lo()` and `hi()` are equal. This is not only a convenience, but makes it clear that a `Span` can be empty (that is, retrieving the source for an empty `Span` will return an empty string), and codifies the (otherwise undocumented--in the rustc_span package, at least) fact that `Span` is a half-open interval (where `hi()` is the open end). `source_map.lookup_file_span()` - returns an enclosing `Span` representing the start and end positions of the file enclosing the given `BytePos`. This gives developers a clear way to quickly determine if any any other `BytePos` or `Span` is also from the same file (for example, by simply calling `file_span.contains(span)`). This results in much simpler code and is much more runtime efficient compared with the obvious alternative: calling `source_map.lookup_line()` for any two `Span`'s byte positions, handle both arms of the `Result` (both contain the file), and then compare files. It is also more efficient than the non-public method `lookup_source_file_idx()` for each `BytePos`, because, while comparing the internal source file indexes would be efficient, looking up the source file index for every `BytePos` or `Span` to be compared requires a binary search (worst case performance being O(log n) for every lookup). `source_map.lookup_file_span()` performs the binary search only once, to get the `file_span` result that can be used to compare to any number of other `BytePos` or `Span` values and those comparisons are always O(1).
richkadel
force-pushed
the
llvm-coverage-map-gen-6b.4
branch
from
83adfeb
to
7225f66
Compare
|
@bors r=wesleywiser |
|
📌 Commit 7225f66 has been approved by |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Sep 1, 2020
Rollup of 12 pull requests Successful merges: - rust-lang#75945 (Use `env::func()`, not 'the function env::func' in docs for std::env) - rust-lang#76002 (Fix `-Z instrument-coverage` on MSVC) - rust-lang#76003 (Adds two source span utility functions used in source-based coverage) - rust-lang#76059 (Clean up E0764) - rust-lang#76103 (Clean up E0769) - rust-lang#76139 (Make `cow_is_borrowed` methods const) - rust-lang#76154 (Fix rustdoc strings indentation) - rust-lang#76161 (Remove notrust in rustc_middle) - rust-lang#76163 (README: Adjust Linux and macOS support platform and architecture) - rust-lang#76166 (Make `StringReader` private) - rust-lang#76172 (Revert rust-lang#75463) - rust-lang#76178 (Update expect-test to 1.0) Failed merges: r? @ghost
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Sep 4, 2020
… r=tmandry Tools, tests, and experimenting with MIR-derived coverage counters Leverages the new mir_dump output file in HTML+CSS (from rust-lang#76074) to visualize coverage code regions and the MIR features that they came from (including overlapping spans). See example below. The `run-make-fulldeps/instrument-coverage` test has been refactored to maximize test coverage and reduce code duplication. The new tests support testing with and without `-Clink-dead-code`, so Rust coverage can be tested on MSVC (which, currently, only works with `link-dead-code` _disabled_). New tests validate coverage region generation and coverage reports with multiple counters per function. Starting with a simple `if-else` branch tests, coverage tests for each additional syntax type can be added by simply dropping in a new Rust sample program. Includes a basic, MIR-block-based implementation of coverage injection, available via `-Zexperimental-coverage`. This implementation has known flaws and omissions, but is simple enough to validate the new tools and tests. The existing `-Zinstrument-coverage` option currently enables function-level coverage only, which at least appears to generate accurate coverage reports at that level. Experimental coverage is not accurate at this time. When branch coverage works as intended, the `-Zexperimental-coverage` option should be removed. This PR replaces the bulk of PR rust-lang#75828, with the remaining parts of that PR distributed among other separate and indentpent PRs. This PR depends on two of those other PRs: rust-lang#76002, rust-lang#76003 and rust-lang#76074 Rust compiler MCP rust-lang/compiler-team#278 Relevant issue: rust-lang#34701 - Implement support for LLVMs code coverage instrumentation  r? @tmandry FYI: @wesleywiser
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.
span.is_empty()- returns true iflo()andhi()are equal. This isnot only a convenience, but makes it clear that a
Spancan be empty(that is, retrieving the source for an empty
Spanwill return an emptystring), and codifies the (otherwise undocumented--in the rustc_span
package, at least) fact that
Spanis a half-open interval (wherehi()is the open end).source_map.lookup_file_span()- returns an enclosingSpanrepresenting the start and end positions of the file enclosing the given
BytePos. This gives developers a clear way to quickly determine if anyany other
BytePosorSpanis also from the same file (for example,by simply calling
file_span.contains(span)).This results in much simpler code and is much more runtime efficient
compared with the obvious alternative: calling
source_map.lookup_line()for any two
Span's byte positions, handle both arms of theResult(both contain the file), and then compare files. It is also more
efficient than the non-public method
lookup_source_file_idx()for eachBytePos, because, while comparing the internal source file indexeswould be efficient, looking up the source file index for every
BytePosor
Spanto be compared requires a binary search (worst caseperformance being O(log n) for every lookup).
source_map.lookup_file_span()performs the binary search only once, toget the
file_spanresult that can be used to compare to any number ofother
BytePosorSpanvalues and those comparisons are always O(1).This PR was split out from PR #75828 .
r? @tmandry
FYI: @wesleywiser