Skip to content

Unify Regions with RegionVids in UnificationTable #85313

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
May 18, 2021
Merged

Unify Regions with RegionVids in UnificationTable #85313

merged 6 commits into from
May 18, 2021

Conversation

jackh726
Copy link
Member

A few test output changes; might be able to revert those but figured I would open this for perf and comments.

r? @nikomatsakis

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 15, 2021
@jackh726
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 15, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Collaborator

bors commented May 15, 2021

⌛ Trying commit 61157b3 with merge ba1f60f056b31729e69b29bdcfb224b3f894854d...

@bors
Copy link
Collaborator

bors commented May 15, 2021

☀️ Try build successful - checks-actions
Build commit: ba1f60f056b31729e69b29bdcfb224b3f894854d (ba1f60f056b31729e69b29bdcfb224b3f894854d)

@rust-timer
Copy link
Collaborator

Queued ba1f60f056b31729e69b29bdcfb224b3f894854d with parent 1025db8, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (ba1f60f056b31729e69b29bdcfb224b3f894854d): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 15, 2021
nikomatsakis
nikomatsakis reviewed May 15, 2021
View reviewed changes
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some thoughts on Zulip.

compiler/rustc_middle/src/infer/unify_key.rs

fn unify_values(value1: &Self, value2: &Self) -> Result<Self, NoError> {
Ok(match (value1.0, value2.0) {
(Some(_), Some(_)) => *value1,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We better put a comment here about why this makes any sense at all...

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@jackh726
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 15, 2021
@bors
Copy link
Collaborator

bors commented May 15, 2021

⌛ Trying commit 909d0e8 with merge c12e8e573917460eb53ad39c819ba6dc57bf95d3...

@bors
Copy link
Collaborator

bors commented May 15, 2021

☀️ Try build successful - checks-actions
Build commit: c12e8e573917460eb53ad39c819ba6dc57bf95d3 (c12e8e573917460eb53ad39c819ba6dc57bf95d3)

@rust-timer
Copy link
Collaborator

Queued c12e8e573917460eb53ad39c819ba6dc57bf95d3 with parent eac3c7c, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (c12e8e573917460eb53ad39c819ba6dc57bf95d3): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 16, 2021
nikomatsakis
nikomatsakis requested changes May 16, 2021
View reviewed changes
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented May 17, 2021

📌 Commit c6e4284 has been approved by nikomatsakis

@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 May 17, 2021
@bors
Copy link
Collaborator

bors commented May 18, 2021

⌛ Testing commit c6e4284 with merge 3ecf2c3b6ed421194dd29c5f144c1c1c2b708e6d...

@bors
Copy link
Collaborator

bors commented May 18, 2021

💥 Test timed out

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 18, 2021
@lqd
Copy link
Member

lqd commented May 18, 2021

@bors retry

@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 May 18, 2021
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@bors
Copy link
Collaborator

bors commented May 18, 2021

⌛ Testing commit c6e4284 with merge 70b9d1e...

@bors
Copy link
Collaborator

bors commented May 18, 2021

☀️ Test successful - checks-actions
Approved by: nikomatsakis
Pushing 70b9d1e to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 18, 2021
@bors bors merged commit 70b9d1e into rust-lang:master May 18, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 18, 2021
@jackh726 jackh726 deleted the region_eq branch May 18, 2021 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikomatsakis nikomatsakis nikomatsakis approved these changes

Assignees

@nikomatsakis nikomatsakis

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.54.0
Development

Successfully merging this pull request may close these issues.
8 participants
@jackh726 @rust-timer @rust-log-analyzer @bors @nikomatsakis @lqd @rust-highfive @rustbot