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

Replace `state_for_location` with `DataflowResultsCursor` #64532

Merged
merged 3 commits into from Sep 18, 2019

Conversation

@ecstatic-morse
Copy link
Contributor

commented Sep 17, 2019

These are two different ways of getting the same data from the result of a dataflow analysis. However, state_for_location goes quadratic if you try to call it for every statement in the body.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Sep 17, 2019

r? @cramertj

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

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Sep 17, 2019

The job mingw-check of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2019-09-17T00:14:17.3888468Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-09-17T00:14:17.4076267Z ##[command]git config gc.auto 0
2019-09-17T00:14:17.4164414Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-09-17T00:14:17.4234969Z ##[command]git config --get-all http.proxy
2019-09-17T00:14:17.4383276Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/64532/merge:refs/remotes/pull/64532/merge
---
2019-09-17T00:24:21.1640857Z     Checking rustc_plugin v0.0.0 (/checkout/src/librustc_plugin/deprecated)
2019-09-17T00:24:21.2618948Z error: method is never used: `get`
2019-09-17T00:24:21.2619329Z    --> src/librustc_mir/dataflow/mod.rs:457:5
2019-09-17T00:24:21.2619721Z     |
2019-09-17T00:24:21.2620119Z 457 |     pub fn get(&self) -> &BitSet<BD::Idx> {
2019-09-17T00:24:21.2630640Z 
2019-09-17T00:24:21.6701530Z error: aborting due to previous error
2019-09-17T00:24:21.6702746Z 
2019-09-17T00:24:21.7960453Z error: Could not compile `rustc_mir`.
---
2019-09-17T00:24:21.8060651Z == clock drift check ==
2019-09-17T00:24:21.8079464Z   local time: Tue Sep 17 00:24:21 UTC 2019
2019-09-17T00:24:22.0946960Z   network time: Tue, 17 Sep 2019 00:24:22 GMT
2019-09-17T00:24:22.0947137Z == end clock drift check ==
2019-09-17T00:24:23.2605709Z ##[error]Bash exited with code '1'.
2019-09-17T00:24:23.2645365Z ##[section]Starting: Checkout
2019-09-17T00:24:23.2647834Z ==============================================================================
2019-09-17T00:24:23.2647921Z Task         : Get sources
2019-09-17T00:24:23.2647975Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@ecstatic-morse ecstatic-morse changed the title Add a getter for the current state to `DataflowResultsCursor` Replace `state_for_location` with `DataflowResultsCursor` Sep 17, 2019
@pnkfelix pnkfelix added the T-compiler label Sep 17, 2019
@pnkfelix

This comment has been minimized.

Copy link
Member

commented Sep 17, 2019

This change looks good to me.

I had a brief desire to attempt to find out if any open PR's are using state_for_location, but I don't think Github makes it easy to do a search like that...?

@ecstatic-morse

This comment has been minimized.

Copy link
Contributor Author

commented Sep 17, 2019

#64207 is exporting most of rustc_mir::dataflow for use in clippy. I believe the author is currently using state_for_location but I can help them switch to DataflowResultsCursor. No other open PRs show up when searching for "dataflow" that use this method.

@ecstatic-morse

This comment has been minimized.

Copy link
Contributor Author

commented Sep 18, 2019

The author of #64207 has already changed their clippy PR to use the cursor.

@tmandry

This comment has been minimized.

Copy link
Contributor

commented Sep 18, 2019

This also looks good to me. r+'ing so we can get it together with #64207.
@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

commented Sep 18, 2019

📌 Commit 73c7a68 has been approved by tmandry

tmandry added a commit to tmandry/rust that referenced this pull request Sep 18, 2019
… r=tmandry

Replace `state_for_location` with `DataflowResultsCursor`

These are two different ways of getting the same data from the result of a dataflow analysis. However, `state_for_location` goes quadratic if you try to call it for every statement in the body.
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 bors merged commit 73c7a68 into rust-lang:master Sep 18, 2019
4 checks passed
4 checks passed
pr Build #20190917.5 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-6.0) Linux x86_64-gnu-llvm-6.0 succeeded
Details
pr (LinuxTools) LinuxTools succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.