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

Track closure signatures & kinds in freshened types #43938

Merged
merged 1 commit into from
Aug 30, 2017

Conversation

arielb1
Copy link
Contributor

@arielb1 arielb1 commented Aug 17, 2017

.chain([closure_sig_marker, closure_kind_marker].iter().cloned()
.map(From::from))
.collect();
let params = tcx.intern_substs(&params);
Copy link
Member

Choose a reason for hiding this comment

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

Should be possible to call mk_substs on the iterator here.

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.

OK, I get the idea here -- this is clever. The other approach that I had been contemplating was slightly different. I had hoped to add facts to the environment indicating when we know the kind of a closure (rather than invoking closure_kind, basically). In this way, we would invalidate the cached entries because there would be a new environment. But I think this is a simpler thing to do for now for sure (and maybe forever -- since I was worried about screwing up the caching results of other things if we grow the param env).

@@ -150,6 +157,53 @@ impl<'a, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for TypeFreshener<'a, 'gcx, 'tcx> {
t
}

ty::TyClosure(def_id, substs) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

feels like some comments laying out the strategy here would be very welcome

@arielb1 arielb1 added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 22, 2017
arielb1 added a commit to arielb1/rust that referenced this pull request Aug 27, 2017
After a projection was processed, its derived subobligations no longer
need any processing when encountered, and can be removed. This improves
the status of rust-lang#43787.

This is actually complementary to rust-lang#43938 - that PR fixes selection
caching (and @remram44's example, which "accidentally" worked because of
the buggy projection caching) while this PR fixes projection caching
@arielb1 arielb1 changed the title [WIP] track closure signatures & kinds in freshened types Track closure signatures & kinds in freshened types Aug 27, 2017
@arielb1
Copy link
Contributor Author

arielb1 commented Aug 27, 2017

Now with cleaner code & comments & hopefully ready for landing!

@arielb1
Copy link
Contributor Author

arielb1 commented Aug 27, 2017

NOTE: this PR will require some changes after #43076 lands, so I'll have this (unchanged) version for beta and a modified version for master.

@arielb1 arielb1 changed the title Track closure signatures & kinds in freshened types [Waiting For Generators] Track closure signatures & kinds in freshened types Aug 27, 2017
@bors
Copy link
Contributor

bors commented Aug 28, 2017

☔ The latest upstream changes (presumably #43076) made this pull request unmergeable. Please resolve the merge conflicts.

bors added a commit that referenced this pull request Aug 28, 2017
clear out projection subobligations after they are processed

After a projection was processed, its derived subobligations no longer
need any processing when encountered, and can be removed. This improves
the status of #43787.

This is actually complementary to #43938 - that PR fixes selection
caching (and @remram44's example, which "accidentally" worked because of
the buggy projection caching) while this PR fixes projection caching.

r? @nikomatsakis
@arielb1 arielb1 changed the title [Waiting For Generators] Track closure signatures & kinds in freshened types Track closure signatures & kinds in freshened types Aug 29, 2017
This allows caching closure signatures and kinds in the normal selection
and evaluation caches, and fixes the exponential worst-case in
@remram44's example, which is a part of rust-lang#43787.

This improvement is complenentary to rust-lang#43999 - they fix different cases.
@arielb1
Copy link
Contributor Author

arielb1 commented Aug 29, 2017

Rebased

//! comparisons (for caching), it is possible to do a `ty::_match` operation between
//! 2 freshened types - this works even with the closure encoding.
//!
//! __An important detail concerning regions.__ The freshener also replaces *all* free regions with
Copy link
Contributor

Choose a reason for hiding this comment

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

nice comments

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Aug 29, 2017

📌 Commit 75d6820 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Aug 29, 2017

⌛ Testing commit 75d6820 with merge 2edfd62c37290bd1a24da88a4aee315ed4c1cfdf...

@bors
Copy link
Contributor

bors commented Aug 29, 2017

💔 Test failed - status-travis

@arielb1
Copy link
Contributor Author

arielb1 commented Aug 29, 2017

@bors retry

@arielb1 arielb1 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 29, 2017
@arielb1
Copy link
Contributor Author

arielb1 commented Aug 29, 2017

Please don't roll this up as this is a performance improvement PR.

@Mark-Simulacrum
Copy link
Member

@bors p=100

bors added a commit that referenced this pull request Aug 30, 2017
Track closure signatures & kinds in freshened types

r? @nikomatsakis
@bors
Copy link
Contributor

bors commented Aug 30, 2017

⌛ Testing commit 75d6820 with merge 09ea6b7...

@bors
Copy link
Contributor

bors commented Aug 30, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 09ea6b7 to master...

@bors bors merged commit 75d6820 into rust-lang:master Aug 30, 2017
bors added a commit that referenced this pull request Aug 31, 2017
[beta] Track closure signatures & kinds in freshened types

This allows caching closure signatures and kinds in the normal selection
and evaluation caches, and fixes the exponential worst-case in
@remram44's example, which is a part of #43787.

This improvement is complenentary to #43999 - they fix different cases.

This is the pre-generators variation of #43938, cloned for beta.
nikomatsakis pushed a commit to nikomatsakis/rust that referenced this pull request Sep 28, 2017
After a projection was processed, its derived subobligations no longer
need any processing when encountered, and can be removed. This improves
the status of rust-lang#43787.

This is actually complementary to rust-lang#43938 - that PR fixes selection
caching (and @remram44's example, which "accidentally" worked because of
the buggy projection caching) while this PR fixes projection caching
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants