Add existential type definitions #51414
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rust-highfive
added
the
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
oli-obk
force-pushed
the
impl_trait_type_def
branch
2 times, most recently
from
763eec8
to
065b7cb
Compare
oli-obk
force-pushed
the
impl_trait_type_def
branch
from
065b7cb
to
9b1bd94
Compare
oli-obk
commented
Jun 7, 2018
| @@ -284,7 +284,7 @@ pub fn change_return_impl_trait() -> impl Clone { | |||
| } | |||
|
|
|||
| #[cfg(not(cfail1))] | |||
| #[rustc_clean(cfg = "cfail2", except = "Hir, HirBody")] | |||
| #[rustc_clean(cfg = "cfail2")] | |||
There was a problem hiding this comment.
So this is the only change that I worry about. Not sure what's going on here. cc @michaelwoerister
There was a problem hiding this comment.
did you ever get resolution as to why this change is necessary and whether it is safe to do?
There was a problem hiding this comment.
I think the Hir is clean, because we now store just a NodeId and a DefId to the existential type declaration. If you change the trait of the impl Trait, those ids won't change. If changing the type of a type alias has the same effect on the incremental cache, then this change is fine, if not, then it's probably not fine.
There was a problem hiding this comment.
Discussed this on irc https://botbot.me/mozilla/rustc/2018-06-18/?msg=101231153&page=1 his is fine as it is. The HIR does indeed not change anymore, but later queries will get retriggered because the HIR of the desugared existential type item has changed
|
cc @pnkfelix |
|
r? @pnkfelix |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
oli-obk
changed the title
oli-obk
force-pushed
the
impl_trait_type_def
branch
from
245b4d6
to
adec1f9
Compare
Merged
|
Ping from triage @pnkfelix! This PR needs your review. |
|
@oli-obk this seems fine to me. I now remember that @michaelwoerister may be out of contact for a little while. I'm personally okay with taking the risk of landing this even without having resolved the question of what is going on in that test you had to change. So I'm going to tell bors to go ahead and land it. |
|
@bors r+ |
|
📌 Commit adec1f9 has been approved by |
bors
added
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Jun 18, 2018
Add existential type definitions Note: this does not allow creating named existential types, it just desugars `impl Trait` to a less (but still very) hacky version of actual `existential type` items. r? @nikomatsakis
|
☀️ Test successful - status-appveyor, status-travis |
This was referenced Jun 18, 2018
|
This PR regressed the compile speed of the @oli-obk: Do you know why this might be? Can you investigate? |
|
CC @rust-lang/wg-compiler-performance |
oli-obk
commented
Jun 19, 2018
| @@ -123,6 +123,11 @@ impl<'cx, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for QueryNormalizer<'cx, 'gcx, 'tcx | |||
| let generic_ty = self.tcx().type_of(def_id); | |||
| let concrete_ty = generic_ty.subst(self.tcx(), substs); | |||
| self.anon_depth += 1; | |||
| if concrete_ty == ty { | |||
There was a problem hiding this comment.
This sanity check is on a hot path. I'll change it to a debug assert
bors
added a commit
that referenced
this pull request
Jun 23, 2018
async/await This PR implements `async`/`await` syntax for `async fn` in Rust 2015 and `async` closures and `async` blocks in Rust 2018 (tracking issue: #50547). Limitations: non-`move` async closures with arguments are currently not supported, nor are `async fn` with multiple different input lifetimes. These limitations are not fundamental and will be removed in the future, however I'd like to go ahead and get this PR merged so we can start experimenting with this in combination with futures 0.3. Based on #51414. cc @petrochenkov for parsing changes. r? @eddyb
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note: this does not allow creating named existential types, it just desugars
impl Traitto a less (but still very) hacky version of actualexistential typeitems.r? @nikomatsakis