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

rustc_typeck: construct {Closure,Generator}Substs more directly. #74314

Merged
merged 1 commit into from
Aug 16, 2020

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Jul 14, 2020

We've previously not had a way to create {Closure,Generator}Substs other than instantiating all generics as inference variables and unifying the inference types (extracted using the regular {Closure,Generator}Substs accessors), with the actual types.

With this PR, those hacks, and assumptions about the order of closure/generator-specific components, are replaced with a simple API where the base Substs are combined with the additional information into a {Closure,Generator}Substs.
This might also be faster than relying inference, although probably not by much.

r? @nikomatsakis cc #53488 @blitzerr

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 14, 2020
@eddyb
Copy link
Member Author

eddyb commented Jul 14, 2020

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Jul 14, 2020

⌛ Trying commit 5533705 with merge 994616d7663772c32c9ad592d445f68a21fcc155...

@bors
Copy link
Contributor

bors commented Jul 14, 2020

☀️ Try build successful - checks-actions, checks-azure
Build commit: 994616d7663772c32c9ad592d445f68a21fcc155 (994616d7663772c32c9ad592d445f68a21fcc155)

@rust-timer
Copy link
Collaborator

Queued 994616d7663772c32c9ad592d445f68a21fcc155 with parent 9d09331, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (994616d7663772c32c9ad592d445f68a21fcc155): comparison url.

@eddyb
Copy link
Member Author

eddyb commented Jul 14, 2020

Yeah, it's neutral, no big surprise there.

@eddyb
Copy link
Member Author

eddyb commented Jul 16, 2020

Just noticed #69749 adds "parent substs" to the Split{Closure,Generator}Generics, I should do that for this and #74341.

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.

This seems like a more readable version. r=me if you fix your own FIXMEs to rename things, but did you want to hold off to avoid conflicting with @davidtwco's polymorphization PR?

@eddyb
Copy link
Member Author

eddyb commented Jul 19, 2020

@nikomatsakis Yeah, let's land polymorphization first, it's slightly cleaner than this PR anyway.

@Muirrum Muirrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 6, 2020
@Dylan-DPC-zz Dylan-DPC-zz added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 6, 2020
@Dylan-DPC-zz
Copy link

marking it blocked based on author's comment

@eddyb
Copy link
Member Author

eddyb commented Aug 16, 2020

@Dylan-DPC #69749 had already landed by the time the label was added.

@eddyb eddyb added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Aug 16, 2020
@Dylan-DPC-zz
Copy link

ah sweet race condition :D

@eddyb
Copy link
Member Author

eddyb commented Aug 16, 2020

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Aug 16, 2020

📌 Commit 5d44d54 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 16, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Aug 16, 2020
…matsakis

rustc_typeck: construct {Closure,Generator}Substs more directly.

We've previously not had a way to create `{Closure,Generator}Substs` other than instantiating all generics as inference variables and unifying the inference types (extracted using the regular `{Closure,Generator}Substs` accessors), with the actual types.

With this PR, those hacks, and assumptions about the order of closure/generator-specific components, are replaced with a simple API where the base `Substs` are combined with the additional information into a `{Closure,Generator}Substs`.
This might also be faster than relying inference, although probably not by much.

r? @nikomatsakis cc rust-lang#53488 @blitzerr
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 16, 2020
Rollup of 10 pull requests

Successful merges:

 - rust-lang#74204 (Don't visit foreign function bodies when lowering ast to hir)
 - rust-lang#74314 (rustc_typeck: construct {Closure,Generator}Substs more directly.)
 - rust-lang#74346 (Use LocalDefId instead of HirId for reachable_set elements.)
 - rust-lang#74399 (Move DelaySpanBugEmitted to ty::context)
 - rust-lang#75177 (Add regression test for issue-66768)
 - rust-lang#75223 (Add #[track_caller] to `Session::delay_span_bug`)
 - rust-lang#75423 (Move to intra-doc links for /library/core/src/hint.rs)
 - rust-lang#75485 (pin docs: add some forward references)
 - rust-lang#75569 (Bump minor version of emsdk to 1.38.47)
 - rust-lang#75596 (Switch to intra-doc links in /sys/windows/ext/{ffi,fs,process}.rs)

Failed merges:

r? @ghost
@bors bors merged commit c518347 into rust-lang:master Aug 16, 2020
@eddyb eddyb deleted the closure-substs-direct branch August 17, 2020 02:20
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
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.

None yet

8 participants