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: lifetime elision should not apply to covariant uses of lifetimes #15907

Closed
aturon opened this issue Jul 23, 2014 · 13 comments
Closed

rustc: lifetime elision should not apply to covariant uses of lifetimes #15907

aturon opened this issue Jul 23, 2014 · 13 comments
Labels
A-lifetimes Area: Lifetimes / regions
Milestone

Comments

@aturon
Copy link
Member

aturon commented Jul 23, 2014

As was noted in the RFC discussion, covariant lifetime positions (which I was mistakenly referring to as "contravariant") should require explicit lifetime annotations.

cc #15552

@aturon
Copy link
Member Author

aturon commented Jul 23, 2014

Nominating, as this is a backwards-incompatible change.

@pcwalton
Copy link
Contributor

I'm going to bring this up at the meeting, because I don't think it's worth doing. Long story short: This would require a whole new pass in typeck that would involve duplicating all the variance rules just to enforce this one rule that seems like it rarely ever comes up.

@pcwalton
Copy link
Contributor

Based on some quick analysis I believe that, in our existing codebase (rustc+libs), covariant lifetimes never come up in any situations where lifetime elision could possibly apply. So I have extra reason to believe this is not worth doing.

@aturon
Copy link
Member Author

aturon commented Jul 24, 2014

Thanks for collecting that information -- could you post a few example fn names? I'm just curious.

@zwarich
Copy link

zwarich commented Jul 24, 2014

@pcwalton Covariant lifetimes being used in a meaningful way doesn't really come up that much in general in the existing codebase. When I removed covariance altogether, I only found one instance up to libsyntax (and even that was potentially just a bug in the inference algorithm, since it wasn't obvious to me what the issue was). That said, if we are going to support them then lifetime elision shouldn't do some obviously incorrect thing for them, especially after specifically stating in the RFC discussion that it would be handled correctly.

@pcwalton
Copy link
Contributor

@aturon Not sure what you mean. What function names?

@zwarich I don't want to add an entire extra pass to the typechecker, one that duplicates all the variance information, to guard against some potentially confusing behavior that never comes up in practice. That just strikes me as gratuitous language complexity.

@aturon
Copy link
Member Author

aturon commented Jul 24, 2014

@pcwalton I'm curious about the places where covariant lifetimes do come up.

I was assuming they did show up in function arguments, and you were saying that the elision rules wouldn't apply anyway; I'd just be curious to know what the functions are.

But maybe you're saying they never show up in argument position?

@pcwalton
Copy link
Contributor

They show up on every closure that has a lifetime in it. However, according to my analysis, none of those appear in argument position when there was a by-reference return value present. Examples of functions that take closures that take arguments by reference are filter and skip_while.

@aturon
Copy link
Member Author

aturon commented Jul 24, 2014

@pcwalton Those closures are higher-ranked.

For embedded closures like that, it's critical that the lifetime elision rules apply in a nested fashion, so that

fn filter<A>(predicate: |&A|: 'r -> bool) -> Filter<'r, A, Self>

becomes

fn filter<'r, A>(predicate: <'a>|&'a A|: 'r -> bool) -> Filter<'r, A, Self>

The rule in question was about a hypothetical situation like:

fn foo(callback: Callback) -> V

where Callback<'a> contains a closure |&'a T| -> U.

@pcwalton
Copy link
Contributor

Ah, right. I'll rerun the analysis.

@pnkfelix
Copy link
Member

Assigning P-backcompat-lang, 1.0 milestone, with expectation that we will discuss further in the team mtg next week.

@pnkfelix pnkfelix added this to the 1.0 milestone Jul 24, 2014
@nikomatsakis
Copy link
Contributor

I tend to agree with @pcwalton that it is infeasible to get this rule correct and not worth the trouble.

@pcwalton
Copy link
Contributor

Per the meeting we decided to close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lifetimes Area: Lifetimes / regions
Projects
None yet
Development

No branches or pull requests

5 participants