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.

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

make erase_regions_ty query anonymous #45364

Merged
merged 1 commit into from
Oct 21, 2017
Merged

make erase_regions_ty query anonymous #45364

merged 1 commit into from
Oct 21, 2017

Conversation

nikomatsakis
Copy link
Contributor

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 18, 2017
@michaelwoerister
Copy link
Member

@bors try

@bors
Copy link
Contributor

bors commented Oct 18, 2017

⌛ Trying commit b5e2451 with merge afa558b...

bors added a commit that referenced this pull request Oct 18, 2017
@bors
Auto merge of #45364 - nikomatsakis:issue-44137-anon, r=<try>
afa558b 
make `erase_regions_ty` query anonymous

r? @michaelwoerister
@bors
Copy link
Contributor

bors commented Oct 18, 2017

☀️ Test successful - status-travis
State: approved= try=True

@Mark-Simulacrum
Copy link
Member

Presuming you wanted a perf build: http://perf.rust-lang.org/compare.html?start=7a4f39453cfbeab1c2220331f2eae8938165c964&end=afa558bf87d8910d28d3d52a4aa1232bd7fae2bc&stat=instructions%3Au.

@nikomatsakis
Copy link
Contributor Author

Doesn't look especially significant, though it does help a few tests.

@michaelwoerister
Copy link
Member

Thanks, @Mark-Simulacrum! That's indeed what I wanted.

@nikomatsakis I think those are pretty good improvements for a one line change :)

Could we even go further and not even allocate a node if it is anonymous and has no inputs?

@michaelwoerister
Copy link
Member

There might be room for further improvement here by not even allocating a DepNode in cases like this but that would need a slightly bigger change. Let's not wait for that.

@bors r+

@bors
Copy link
Contributor

bors commented Oct 20, 2017

📌 Commit b5e2451 has been approved by michaelwoerister

@michaelwoerister
Copy link
Member

I opened #45408 regarding further optimization.

@kennytm kennytm 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 Oct 20, 2017
@bors
Copy link
Contributor

bors commented Oct 21, 2017

⌛ Testing commit b5e2451 with merge 6a136f6...

bors added a commit that referenced this pull request Oct 21, 2017
@bors
Auto merge of #45364 - nikomatsakis:issue-44137-anon, r=michaelwoerister
6a136f6 
make `erase_regions_ty` query anonymous

r? @michaelwoerister
@bors
Copy link
Contributor

bors commented Oct 21, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: michaelwoerister
Pushing 6a136f6 to master...

@bors bors merged commit b5e2451 into rust-lang:master Oct 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@michaelwoerister michaelwoerister

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@nikomatsakis @michaelwoerister @bors @Mark-Simulacrum @kennytm