Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make rustc_mir::dataflow module pub (for clippy) #64207

Merged
merged 1 commit into from
Sep 18, 2019

Conversation

sinkuu
Copy link
Contributor

@sinkuu sinkuu commented Sep 6, 2019

I'm working on fixing false-positives of a MIR-based clippy lint (rust-lang/rust-clippy#4509), and in need of the dataflow infrastructure.

@rust-highfive
Copy link
Collaborator

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 6, 2019
@cramertj
Copy link
Member

cramertj commented Sep 6, 2019

r? @tmandry or @eddyb -- wanted to check that this is reasonably stable enough to expose, and that this is the right thing to use in clippy.

@rust-highfive rust-highfive assigned tmandry and unassigned cramertj Sep 6, 2019
@tmandry
Copy link
Member

tmandry commented Sep 6, 2019

I think this is fine, but I'd like to double check with either @eddyb, @pnkfelix, or @matthewjasper.

What are you using to consume the results? Right now I think DataflowResultsCursor is the preferred way (see #62315).

@sinkuu
Copy link
Contributor Author

sinkuu commented Sep 7, 2019

I'm using state_for_location and DataflowResultsCursor. I'll try to switch to the latter.

@JohnCSimon
Copy link
Member

Ping from triage:

@sinkuu @tmandry This PR has not been updated in a week. What is the status of the PR?

Thanks.

@sinkuu
Copy link
Contributor Author

sinkuu commented Sep 16, 2019

@JohnCSimon Waiting for an approval from

either @eddyb, @pnkfelix, or @matthewjasper.

@tmandry
Copy link
Member

tmandry commented Sep 18, 2019

No objections have been voiced so I'm approving this.
@bors r+

@bors
Copy link
Contributor

bors commented Sep 18, 2019

📌 Commit 867ce76 has been approved by tmandry

@bors bors 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 Sep 18, 2019
tmandry added a commit to tmandry/rust that referenced this pull request Sep 18, 2019
Make rustc_mir::dataflow module pub (for clippy)

I'm working on fixing false-positives of a MIR-based clippy lint (rust-lang/rust-clippy#4509), and in need of the dataflow infrastructure.
bors added a commit that referenced this pull request Sep 18, 2019
Rollup of 5 pull requests

Successful merges:

 - #64207 (Make rustc_mir::dataflow module pub (for clippy))
 - #64348 (PR: documentation spin loop hint)
 - #64532 (Replace `state_for_location` with `DataflowResultsCursor`)
 - #64578 (Fix issue22656 with LLDB 8)
 - #64580 (Update books)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented Sep 18, 2019

⌛ Testing commit 867ce76 with merge eceec57...

@bors bors merged commit 867ce76 into rust-lang:master Sep 18, 2019
@sinkuu sinkuu deleted the pub_dataflow branch September 19, 2019 00:34
bors added a commit to rust-lang/rust-clippy that referenced this pull request Oct 3, 2019
Fix false-positive of redundant_clone and move to clippy::perf

This PR introduces dataflow analysis to `redundant_clone` lint to filter out borrowed variables, which had been incorrectly detected.

Depends on rust-lang/rust#64207.

changelog: Moved `redundant_clone` lint to `perf` group

# What this lint catches

## `clone`/`to_owned`

```rust
let s = String::new();
let t = s.clone();
```

```rust
// MIR
_1 = String::new();
_2 = &_1;
_3 = clone(_2); // (*)
```

We can turn this `clone` call into a move if

1. `_2` is the sole borrow of `_1` at the statement `(*)`
2. `_1` is not used hereafter

## `Deref` + type-specific `to_owned` method

```rust
let s = std::path::PathBuf::new();
let t = s.to_path_buf();
```

```rust
// MIR
_1 = PathBuf::new();
_2 = &1;
_3 = call deref(_2);
_4 = _3;                         // Copies borrow
StorageDead(_2);
_5 = Path::to_path_buf(_4); // (*)
```

We can turn this `to_path_buf` call into a move if

1. `_3` `_4` are the sole borrow of `_1` at `(*)`
2. `_1` is not used hereafter

# What this PR introduces

1. `MaybeStorageLive` that determines whether a local lives at a particular location
2. `PossibleBorrowerVisitor` that constructs [`TransitiveRelation`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_data_structures/transitive_relation/struct.TransitiveRelation.html) of possible borrows, e.g. visiting `_2 = &1; _3 = &_2:` will result in `_3 -> _2 -> _1` relation. Then `_3` and `_2` will be counted as possible borrowers of `_1` in the sole-borrow analysis above.
phansch added a commit to phansch/rust-clippy that referenced this pull request Oct 4, 2019
Fix false-positive of redundant_clone and move to clippy::perf

This PR introduces dataflow analysis to `redundant_clone` lint to filter out borrowed variables, which had been incorrectly detected.

Depends on rust-lang/rust#64207.

changelog: Moved `redundant_clone` lint to `perf` group

# What this lint catches

## `clone`/`to_owned`

```rust
let s = String::new();
let t = s.clone();
```

```rust
// MIR
_1 = String::new();
_2 = &_1;
_3 = clone(_2); // (*)
```

We can turn this `clone` call into a move if

1. `_2` is the sole borrow of `_1` at the statement `(*)`
2. `_1` is not used hereafter

## `Deref` + type-specific `to_owned` method

```rust
let s = std::path::PathBuf::new();
let t = s.to_path_buf();
```

```rust
// MIR
_1 = PathBuf::new();
_2 = &1;
_3 = call deref(_2);
_4 = _3;                         // Copies borrow
StorageDead(_2);
_5 = Path::to_path_buf(_4); // (*)
```

We can turn this `to_path_buf` call into a move if

1. `_3` `_4` are the sole borrow of `_1` at `(*)`
2. `_1` is not used hereafter

# What this PR introduces

1. `MaybeStorageLive` that determines whether a local lives at a particular location
2. `PossibleBorrowerVisitor` that constructs [`TransitiveRelation`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_data_structures/transitive_relation/struct.TransitiveRelation.html) of possible borrows, e.g. visiting `_2 = &1; _3 = &_2:` will result in `_3 -> _2 -> _1` relation. Then `_3` and `_2` will be counted as possible borrowers of `_1` in the sole-borrow analysis above.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants