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

Prohibit unconstrained lifetimes that appear in associated types #24461

Merged
merged 4 commits into from
Apr 17, 2015

Conversation

nikomatsakis
Copy link
Contributor

This makes it illegal to have unconstrained lifetimes that appear in an associated type definition. Arguably, we should prohibit all unconstrained lifetimes -- but it would break various macros. It'd be good to evaluate how large a break change it would be. But this seems like the minimal change we need to do to establish soundness, so we should land it regardless. Another variant would be to prohibit all lifetimes that appear in any impl item, not just associated types. I don't think that's necessary for soundness -- associated types are different because they can be projected -- but it would feel a bit more consistent and "obviously" safe. I'll experiment with that in the meantime.

r? @aturon

Fixes #22077.

@nikomatsakis
Copy link
Contributor Author

@bors r=aturon p=1 8eb2b14

Giving p=1 because this is a soundness fix.

@bors
Copy link
Contributor

bors commented Apr 16, 2015

📌 Commit b8282dd has been approved by aturon

@bors
Copy link
Contributor

bors commented Apr 16, 2015

⌛ Testing commit b8282dd with merge b9a83fc...

@bors
Copy link
Contributor

bors commented Apr 16, 2015

💔 Test failed - auto-mac-64-opt

@nikomatsakis nikomatsakis force-pushed the issue-22077-unused-lifetimes branch from b8282dd to d504d12 Compare April 16, 2015 23:25
@nikomatsakis
Copy link
Contributor Author

@bors r=aturon p=1 c53c637

@bors
Copy link
Contributor

bors commented Apr 16, 2015

📌 Commit d504d12 has been approved by aturon

@bors
Copy link
Contributor

bors commented Apr 16, 2015

⌛ Testing commit d504d12 with merge 6a40cec...

@bors
Copy link
Contributor

bors commented Apr 17, 2015

💔 Test failed - auto-mac-32-opt

multiple kinds of parameters (regions and types, specifically)
which get mentioned in an associated type are constrained.  Arguably we
should just require that all regions are constrained, but that is more
of a breaking change.
@nikomatsakis nikomatsakis force-pushed the issue-22077-unused-lifetimes branch from d504d12 to 5368070 Compare April 17, 2015 14:06
@nikomatsakis
Copy link
Contributor Author

@bors r=aturon 5368070

@bors
Copy link
Contributor

bors commented Apr 17, 2015

⌛ Testing commit 5368070 with merge 4879b3b...

@bors
Copy link
Contributor

bors commented Apr 17, 2015

💔 Test failed - auto-mac-64-nopt-t

@nikomatsakis
Copy link
Contributor Author

@bors retry // segfault is ... probably not our fault, I want to see how repeatable it is

@bors
Copy link
Contributor

bors commented Apr 17, 2015

⌛ Testing commit 5368070 with merge b62cb9e...

@bors
Copy link
Contributor

bors commented Apr 17, 2015

💔 Test failed - auto-mac-64-nopt-t

@nikomatsakis
Copy link
Contributor Author

@bors retry // acrichto claims this is fixed

@bors
Copy link
Contributor

bors commented Apr 17, 2015

⌛ Testing commit 5368070 with merge f305579...

bors added a commit that referenced this pull request Apr 17, 2015
…turon

This makes it illegal to have unconstrained lifetimes that appear in an associated type definition. Arguably, we should prohibit all unconstrained lifetimes -- but it would break various macros. It'd be good to evaluate how large a break change it would be. But this seems like the minimal change we need to do to establish soundness, so we should land it regardless. Another variant would be to prohibit all lifetimes that appear in any impl item, not just associated types. I don't think that's necessary for soundness -- associated types are different because they can be projected -- but it would feel a bit more consistent and "obviously" safe. I'll experiment with that in the meantime.

r? @aturon 

Fixes #22077.
@bors bors merged commit 5368070 into rust-lang:master Apr 17, 2015
@pnkfelix
Copy link
Member

soundness fix.

going from nominated to (nominated, accepted)

@pnkfelix pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Apr 23, 2015
@alexcrichton alexcrichton removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Apr 23, 2015
@nikomatsakis nikomatsakis deleted the issue-22077-unused-lifetimes branch March 30, 2016 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trait implementation accepts wrong lifetime in method signature with associated type
5 participants