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

Rollup of 5 pull requests #61601

Merged
merged 20 commits into from
Jun 6, 2019
Merged

Rollup of 5 pull requests #61601

merged 20 commits into from
Jun 6, 2019

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Jun 6, 2019

Successful merges:

Failed merges:

r? @ghost

czipperz and others added 20 commits May 30, 2019 16:46
Co-Authored-By: Steven Fackler <[email protected]>
Change visit api

r? @oli-obk

In the [first commit](rust-lang@37386d3) of this PR, I'm changing `visit_place` to be the function that traverses the `Place` and have only that responsibility. Then there are two other functions `visit_place_base` and `visit_projection` which are the ones in charge of visiting the base and the projection. Visitor implementors can implement any of those.

In the [second commit](rust-lang@e786f63) we can already see some things that confuses me, which I think this division will make more clear. The old code, first checked if the place was a base, did something with it and then called `super_place` [here](rust-lang@e786f63#diff-d583e4efe1a72516e274158e53223633L678). `super_place` checks again if it's a base [here](https://github.com/rust-lang/rust/blob/master/src/librustc/mir/visit.rs#L679-L684) and in case is a local, visits the local and stuff like that. That's not very obvious on the code, and if I'm not wrong it's not needed. In this PR or we have [this](rust-lang@e786f63#diff-d583e4efe1a72516e274158e53223633R673) as I did or we can just do `- => self.super_place_base(...)` and that will be obvious that I'm letting the default implementation process the base.
…=oli-obk

Make visitors iterate

r? @oli-obk

The second commit is not completely equivalent, unsure if the code is wrong or not. Tests pass though, otherwise we would need to iterate in the opposite direction as it happened in other parts of the code.
@Centril
Copy link
Contributor Author

Centril commented Jun 6, 2019

@bors r+ p=5 rollup=never

@bors
Copy link
Contributor

bors commented Jun 6, 2019

📌 Commit 9e97970 has been approved by Centril

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 6, 2019
@bors
Copy link
Contributor

bors commented Jun 6, 2019

⌛ Testing commit 9e97970 with merge 5eeb567...

bors added a commit that referenced this pull request Jun 6, 2019
Rollup of 5 pull requests

Successful merges:

 - #61376 (Add Bound::cloned())
 - #61554 (Change visit api)
 - #61559 (Make visitors iterate)
 - #61585 (Update .mailmap with my name)
 - #61591 (Update .mailmap)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented Jun 6, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: Centril
Pushing 5eeb567 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 6, 2019
@bors bors merged commit 9e97970 into rust-lang:master Jun 6, 2019
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #61601!

Tested on commit 5eeb567.
Direct link to PR: #61601

💔 rls on windows: test-pass → test-fail (cc @Xanewok, @rust-lang/infra).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Jun 6, 2019
Tested on commit rust-lang/rust@5eeb567.
Direct link to PR: <rust-lang/rust#61601>

💔 rls on windows: test-pass → test-fail (cc @Xanewok, @rust-lang/infra).
@Centril Centril deleted the rollup-uegbsns branch June 7, 2019 08:31
@Centril Centril added the rollup A PR which is a rollup label Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup 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.

7 participants