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

Refactor how closures are represented to expose the types of the upvars #27087

Merged
merged 10 commits into from
Jul 24, 2015

Conversation

nikomatsakis
Copy link
Contributor

Refactors the "desugaring" of closures to expose the types of the upvars. This is necessary to be faithful with how actual structs work. The reasoning of the particular desugaring that I chose is explained in a fairly detailed comment.

As a side-effect, recursive closure types are prohibited unless a trait object intermediary is used. This fixes #25954 and also eliminates concerns about unrepresentable closure types that have infinite size, I believe. I don't believe this can cause regressions because of #25954.

(As for motivation, besides #25954 etc, this work is also intended as refactoring in support of incremental compilation, since closures are one of the thornier cases encountered when attempting to split node-ids into item-ids and within-item-ids. The goal is to eliminate the "internal def-id" distinction in astdecoding. However, I have to do more work on trans to really make progress there.)

r? @nrc

@nikomatsakis
Copy link
Contributor Author

@arielb1 ordinary fn pointers are virtual dispatch

@nikomatsakis
Copy link
Contributor Author

@tamird

should that file be removed too?

yeah, probably, thanks. :)

@arielb1
Copy link
Contributor

arielb1 commented Jul 19, 2015

@nikomatsakis

Using a dead type for a type parameter in that context does require virtual dispatch in the closure case too: if you can access the closure while it isn't hidden behind an object, you must be within the parent functions' scope, and its type parameters must therefore be alive. If/when fn items store their substs as part of their type, they will behave similarly (fn-items are unnameable in the same way as closures, so this interacts in the same way with anonymous types).

@nrc
Copy link
Member

nrc commented Jul 20, 2015

r=me with the comments addressed

@bors
Copy link
Contributor

bors commented Jul 22, 2015

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

@nikomatsakis nikomatsakis force-pushed the closure-exploration branch from 11f27b3 to 7a04326 Compare July 24, 2015 01:09
TyClosure variant; thread this through wherever closure substitutions
are expected, which leads to a net simplification. Simplify trans
treatment of closures in particular.
This was the intention before but silly coding caused it to run twice if
there are nested closures.
is being used now before the final regionck stage and in some cases SOME
amount of unresolved inference is OK. In fact, we could probably just
allow inference variables as well with only minimal pain.
upvars after analysis is done. Remove the `closure_upvars` helper and
just consult this list of type variables directly.
@nikomatsakis nikomatsakis force-pushed the closure-exploration branch 2 times, most recently from 138b551 to 71d4418 Compare July 24, 2015 12:25
@nikomatsakis
Copy link
Contributor Author

@bors r=nrc

@bors
Copy link
Contributor

bors commented Jul 24, 2015

📌 Commit 71d4418 has been approved by nrc

@nikomatsakis
Copy link
Contributor Author

@bors r=nrc

@bors
Copy link
Contributor

bors commented Jul 24, 2015

📌 Commit 71d4418 has been approved by nrc

@bors
Copy link
Contributor

bors commented Jul 24, 2015

⌛ Testing commit 71d4418 with merge 5c3cc0c...

@bors
Copy link
Contributor

bors commented Jul 24, 2015

💔 Test failed - auto-mac-32-opt

@alexcrichton
Copy link
Member

@bors: retry

On Fri, Jul 24, 2015 at 6:20 AM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-mac-32-opt
http://buildbot.rust-lang.org/builders/auto-mac-32-opt/builds/5863


Reply to this email directly or view it on GitHub
#27087 (comment).

@bors
Copy link
Contributor

bors commented Jul 24, 2015

⌛ Testing commit 71d4418 with merge 5e6b534...

bors added a commit that referenced this pull request Jul 24, 2015
Refactors the "desugaring" of closures to expose the types of the upvars. This is necessary to be faithful with how actual structs work. The reasoning of the particular desugaring that I chose is explained in a fairly detailed comment.

As a side-effect, recursive closure types are prohibited unless a trait object intermediary is used. This fixes #25954 and also eliminates concerns about unrepresentable closure types that have infinite size, I believe. I don't believe this can cause regressions because of #25954.

(As for motivation, besides #25954 etc, this work is also intended as refactoring in support of incremental compilation, since closures are one of the thornier cases encountered when attempting to split node-ids into item-ids and within-item-ids. The goal is to eliminate the "internal def-id" distinction in astdecoding. However, I have to do more work on trans to really make progress there.)

r? @nrc
@bors bors merged commit 71d4418 into rust-lang:master Jul 24, 2015
@nikomatsakis nikomatsakis deleted the closure-exploration branch March 30, 2016 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustc stackoverflow
5 participants