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

@ecstatic-morse ecstatic-morse 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
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 17, 2019
@rust-highfive
Copy link
Collaborator

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 Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 17, 2019
@pnkfelix
Copy link
Member

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
Copy link
Contributor Author

ecstatic-morse 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
Copy link
Contributor Author

ecstatic-morse commented Sep 18, 2019

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

@tmandry
Copy link
Member

tmandry commented Sep 18, 2019

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

@bors
Copy link
Contributor

bors commented Sep 18, 2019

📌 Commit 73c7a68 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
… 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
@ecstatic-morse ecstatic-morse deleted the dataflow-cursor-get branch October 6, 2020 01:42
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. T-compiler Relevant 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.

6 participants