Skip to content

Detect illegal hidden lifetimes in impl Trait - #49041

Merged
bors merged 8 commits into
rust-lang:masterfrom
nikomatsakis:issue-46541-impl-trait-hidden-lifetimes
Mar 22, 2018
Merged

Detect illegal hidden lifetimes in impl Trait#49041
bors merged 8 commits into
rust-lang:masterfrom
nikomatsakis:issue-46541-impl-trait-hidden-lifetimes

Conversation

@nikomatsakis

@nikomatsakisnikomatsakis commented Mar 15, 2018

Copy link
Copy Markdown
Contributor

This branch fixes#46541 -- however, it presently doesn't build because it also breaks a number of existing usages of impl Trait. I'm opening it as a WIP for now, just because we want to move on impl Trait, but I'll try to fix the problem in a bit.

(The problem is due to the fact that we apparently infer stricter lifetimes in closures that we need to; for example, if you capture a variable of type &'a &'b u32, we will put precisely those lifetimes into the closure, even if the closure would be happy with &'a &'a u32. This causes the present chance to affect things that are not invariant.) fixed

r? @cramertj

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 15, 2018
@nikomatsakis
nikomatsakisforce-pushed the issue-46541-impl-trait-hidden-lifetimes branch 3 times, most recently from 12f01cd to 3901391CompareMarch 16, 2018 00:33
@emilyalbini

Copy link
Copy Markdown
Member

Ping from triage @cramertj! This PR needs your review.

@cramertj

Copy link
Copy Markdown
Member

This seems fine to me so far, but I think @nikomatsakis was going to push some updates to the behavior of lifetimes inside of closures. @nikomatsakis is that still your plan?

@nikomatsakis

Copy link
Copy Markdown
ContributorAuthor

@cramertj I did that already

@cramertj

cramertj commented Mar 20, 2018

Copy link
Copy Markdown
Member

Is the remaining travis failure (ui/generator/auto-trait-regions.rs) something you're aware of?

@nikomatsakis

Copy link
Copy Markdown
ContributorAuthor

@cramertj I saw it now, I'll go try and fix it

@nikomatsakis
nikomatsakisforce-pushed the issue-46541-impl-trait-hidden-lifetimes branch from 3901391 to 60b9558CompareMarch 20, 2018 20:12
@cramertj

Copy link
Copy Markdown
Member

(@nikomatsakis travis is still mad)

@nikomatsakis

Copy link
Copy Markdown
ContributorAuthor

@cramertj yeah I didn't start trying to fix it yet :)

We used to make the upvar types in the closure `==` but that was
stronger than we needed. Subtyping suffices, since we are copying the
upvar value into the closure field. This in turn allows us to infer
smaller lifetimes in captured values in some cases (like the example
here), avoiding errors.
@nikomatsakis
nikomatsakisforce-pushed the issue-46541-impl-trait-hidden-lifetimes branch from 60b9558 to 9d5ec9eCompareMarch 21, 2018 09:41
@nikomatsakis

nikomatsakis commented Mar 21, 2018

Copy link
Copy Markdown
ContributorAuthor

@cramertj ok fixed now (I think)

@nikomatsakisnikomatsakis changed the title [WIP] Detect illegal hidden lifetimes in impl TraitDetect illegal hidden lifetimes in impl TraitMar 21, 2018
@cramertj

Copy link
Copy Markdown
Member

Travis still red.

@nikomatsakis

Copy link
Copy Markdown
ContributorAuthor

Yeah, spoke a bit prematurely. I fixed a few more things.

@cramertj

Copy link
Copy Markdown
Member

(@nikomatsakis just in case you weren't aware, this is still red)

@cramertj

Copy link
Copy Markdown
Member

@bors r+ p=10

@bors

bors commented Mar 22, 2018

Copy link
Copy Markdown
Collaborator

📌 Commit 2e8a1ab has been approved by cramertj

@borsbors 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 Mar 22, 2018
@cramertjcramertj mentioned this pull request Mar 22, 2018
2 tasks
@bors

bors commented Mar 22, 2018

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 2e8a1ab with merge e575773...

bors added a commit that referenced this pull request Mar 22, 2018
…etimes, r=cramertj
Detect illegal hidden lifetimes in `impl Trait`
This branch fixes#46541 -- however, it presently doesn't build because it also *breaks* a number of existing usages of impl Trait. I'm opening it as a WIP for now, just because we want to move on impl Trait, but I'll try to fix the problem in a bit.
~~(The problem is due to the fact that we apparently infer stricter lifetimes in closures that we need to; for example, if you capture a variable of type `&'a &'b u32`, we will put *precisely* those lifetimes into the closure, even if the closure would be happy with `&'a &'a u32`. This causes the present chance to affect things that are not invariant.)~~ fixed
r? @cramertj
@bors

bors commented Mar 22, 2018

Copy link
Copy Markdown
Collaborator

☀️ Test successful - status-appveyor, status-travis
Approved by: cramertj
Pushing e575773 to master...

@bors
bors merged commit 2e8a1ab into rust-lang:masterMar 22, 2018
@borsbors mentioned this pull request Mar 22, 2018
4 tasks
kennytm added a commit to kennytm/rust that referenced this pull request Mar 24, 2018
…atsakis
Stabilize impl Trait
Blocked on:
- [x] rust-lang#49041 and
- [ ] completion of FCP in rust-lang#34511 (comment) (3 days from now).
I have not yet done any docs work for this-- I probably won't get to it until this weekend (might be a project for the flight to the all-hands).
bors added a commit that referenced this pull request Mar 26, 2018
Stabilize impl Trait
Blocked on:
- [x] #49041 and
- [ ] completion of FCP in #34511 (comment) (3 days from now).
I have not yet done any docs work for this-- I probably won't get to it until this weekend (might be a project for the flight to the all-hands).
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: 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.

🚀 existential impl Trait allows lifetimes that would not be allowed by abstract type

5 participants

@nikomatsakis@emilyalbini@cramertj@bors@rust-highfive