Skip to content

Remove reachable coverage without counters - #99711

Merged
bors merged 1 commit into
rust-lang:masterfrom
tmiasko:coverage
Jul 26, 2022
Merged

Remove reachable coverage without counters#99711
bors merged 1 commit into
rust-lang:masterfrom
tmiasko:coverage

Conversation

@tmiasko

Copy link
Copy Markdown
Contributor

Remove reachable coverage without counters to maintain invariant that
either there is no coverage at all or there is a live coverage counter
left that provides the function source hash.

The motivating example would be a following closure:

let f = |x:bool| {debug_assert!(x);};

Which, with span changes from #93967, with disabled debug assertions,
after the final CFG simplifications but before removal of dead blocks,
gives rise to MIR:

fn main::{closure#0}(_1:&[closure@a.rs:2:13:2:22],_2:bool) -> (){
debug x => _2;letmut _0:();
bb0:{Coverage::Expression(4294967295) = 1 - 2;return;}
...}

Which also makes the initial instrumentation quite suspect, although
this pull request doesn't attempt to address that aspect directly.

Fixes#98833.

r? @wesleywiser@richkadel

Remove reachable coverage without counters to maintain invariant that
either there is no coverage at all or there is a live coverage counter
left that provides the function source hash.
The motivating example would be a following closure:
```rust
let f = |x: bool| {
debug_assert!(x);
};
```
Which, with span changes from rust-lang#93967, with disabled debug assertions,
after the final CFG simplifications but before removal of dead blocks,
gives rise to MIR:
```rust
fn main::{closure#0}(_1: &[closure@a.rs:2:13: 2:22], _2: bool) -> () {
debug x => _2;
let mut _0: ();
bb0: {
Coverage::Expression(4294967295) = 1 - 2;
return;
}
...
}
```
@rustbotrustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 25, 2022
@rustbot

Copy link
Copy Markdown
Collaborator

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 25, 2022
@wesleywiser

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented Jul 25, 2022

Copy link
Copy Markdown
Collaborator

📌 Commit 5f40a4f has been approved by wesleywiser

It is now in the queue for this repository.

@borsbors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 25, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jul 25, 2022
Remove reachable coverage without counters
Remove reachable coverage without counters to maintain invariant that
either there is no coverage at all or there is a live coverage counter
left that provides the function source hash.
The motivating example would be a following closure:
```rust
let f = |x: bool| {
debug_assert!(x);
};
```
Which, with span changes from rust-lang#93967, with disabled debug assertions,
after the final CFG simplifications but before removal of dead blocks,
gives rise to MIR:
```rust
fn main::{closure#0}(_1: &[closure@a.rs:2:13: 2:22], _2: bool) -> () {
debug x => _2;
let mut _0: ();
bb0: {
Coverage::Expression(4294967295) = 1 - 2;
return;
}
...
}
```
Which also makes the initial instrumentation quite suspect, although
this pull request doesn't attempt to address that aspect directly.
Fixesrust-lang#98833.
r? `@wesleywiser` `@richkadel`
@Dylan-DPCDylan-DPC mentioned this pull request Jul 25, 2022
@JohnTitorJohnTitor mentioned this pull request Jul 26, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 26, 2022
Rollup of 7 pull requests
Successful merges:
- rust-lang#98211 (Implement `fs::get_path` for FreeBSD.)
- rust-lang#99353 (Slightly improve mismatched GAT where clause error)
- rust-lang#99593 (Suggest removing the tuple struct field for the unwrapped value)
- rust-lang#99615 (Remove some explicit `self.infcx` for `FnCtxt`, which already derefs into `InferCtxt`)
- rust-lang#99711 (Remove reachable coverage without counters)
- rust-lang#99718 (Avoid `&str`/`Symbol` to `String` conversions)
- rust-lang#99720 (Sync rustc_codegen_cranelift)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 3c1eef2 into rust-lang:masterJul 26, 2022
@rustbotrustbot added this to the 1.64.0 milestone Jul 26, 2022
@tmiasko
tmiasko deleted the coverage branch July 30, 2022 07:22
bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Aug 24, 2022
Rollup of 7 pull requests
Successful merges:
- rust-lang#98211 (Implement `fs::get_path` for FreeBSD.)
- rust-lang#99353 (Slightly improve mismatched GAT where clause error)
- rust-lang#99593 (Suggest removing the tuple struct field for the unwrapped value)
- rust-lang#99615 (Remove some explicit `self.infcx` for `FnCtxt`, which already derefs into `InferCtxt`)
- rust-lang#99711 (Remove reachable coverage without counters)
- rust-lang#99718 (Avoid `&str`/`Symbol` to `String` conversions)
- rust-lang#99720 (Sync rustc_codegen_cranelift)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Recent nightly started ICEing with "No counters provided the source_hash for used function"

5 participants

@tmiasko@rustbot@wesleywiser@bors@rust-highfive