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

Lifetime elision on methods can easily create obscure errors. #17822

Closed
eddyb opened this issue Oct 6, 2014 · 14 comments
Closed

Lifetime elision on methods can easily create obscure errors. #17822

eddyb opened this issue Oct 6, 2014 · 14 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lifetimes Area: Lifetimes / regions

Comments

@eddyb
Copy link
Member

eddyb commented Oct 6, 2014

While working on adding lifetimes to rustc::middle::ty, which have since spread throughout the compiler, I've found lifetime elision to create issues more times than it was helping.
Consider this:

struct Foo<'a> {...}
impl<'a> Foo<'a> {
    fn bar(&self) -> Bar {...}
    // ^ needs to be changed to:
    fn bar(&self) -> Bar<'a> {...}
}

For more complicated signatures, elision would not apply and I would often get a (regionck) error within the method, which was great, but where it did apply, it considered the signature to be this:

    fn bar<'b>(&'b self) -> Bar<'b>

Many times, the method itself would typeck, but callers would get the wrong constraints during regionck.
That leaves you with an error in a caller to bar, and because we're lacking blame assignment (is it even feasible?), you usually only know the function in which such an lifetime-elided method is being called.

This is suboptimal and even just a command line option (or attribute) to disable lifetime elision would've helped.
@nikomatsakis suggested disallowing lifetime elision inside an impl generic over lifetimes, or at least making it more careful.

@eddyb eddyb added A-lifetimes Area: Lifetimes / regions I-papercut labels Oct 6, 2014
@kmcallister kmcallister added the A-diagnostics Area: Messages for errors, warnings, and lints label Oct 6, 2014
@aturon
Copy link
Member

aturon commented Oct 6, 2014

cc @wycats

I agree with @nikomatsakis's suggestion. Certainly, that's a reasonable starting point, since we can always add elision rules later on.

@wycats
Copy link
Contributor

wycats commented Oct 6, 2014

What is @nikomatsakis's suggestion?

@wycats
Copy link
Contributor

wycats commented Oct 6, 2014

Ah I see. I am ok with that as well. I think we should consider revisiting that pattern sometime soon.

@reem
Copy link
Contributor

reem commented Oct 6, 2014

I have also seen this issue crop up a few times both on IRC and when helping people with Rust in person. People new to Rust get especially confused because they really have no idea what might be wrong as they get scared by lifetimes in general.

@wycats
Copy link
Contributor

wycats commented Oct 7, 2014

I feel extremely, extremely, (extremely) strongly that confusion on these lines is based on the current poor quality of error messages.

This is why I made error messages an important part of the original RFC, and why I was very saddened to see such a long time go by between the core functionality being merged and the error message improvements being worked on.

In general, our lifetime messaging is very bad, and doesn't match an explanation of what's happening that you would explain to a regular Rust programmer.

@reem
Copy link
Contributor

reem commented Oct 7, 2014

Right. I find much of my time when teaching new people (I've been doing a lot of it lately) is taken up explaining how to interpret compiler errors about lifetimes, which is a strong indicator of the quality of the error messages.

The tricky thing with generating better error messages is that it can be generally hard for the compiler to tell what the programmer intended to do. Perhaps generating elided lifetimes in error messages would be interesting, i.e. in an error message the above example would appear as something like:

struct Foo<'a> {...}
impl<'a> Foo<'a> {
    fn bar<'b>(&'b self) -> Bar<'b> {...}
}

An unused lifetimes lint would also help in cases like these.

@zwarich
Copy link

zwarich commented Oct 7, 2014

Having done a lot of lifetime plumbing recently in Servo, I agree with @eddyb's assessment. Lifetime elision hurts more than it helps inside of traits and impls parameterized by lifetimes, and I usually end up adding explicit parameters in the cases that lifetime elision does handle in order to make it more obvious to the reader.

@aturon
Copy link
Member

aturon commented Oct 7, 2014

re: error messages, see:

Note that implementing the error messages from our RFC is considered a 1.0 blocker, but it's not categorized as backwards-incompatible, and @pcwalton has been focusing on those bugs first.

(I agree that it'd be nice to fix this sooner, but that's a yak I don't have time to shave myself.)

@wycats
Copy link
Contributor

wycats commented Oct 7, 2014

For what it's worth, these error messages may seem fine to experienced Rust developers, but they lead to tweets like this one:

mismatched types: expected &mut <generic #13>

See this right here is why I’m OK with Go not having type parameters.
by @coda

The Rust type system isn't very complex, but poor error messages give a very bad first impression, and lead people to mistakenly believe that Rust has a high-falutin, pie-in-the-sky type system.

@eddyb
Copy link
Member Author

eddyb commented Oct 7, 2014

I should mention that the terrible error messages for lifetimes come from regionck, which has to infer them (and unlike types, you can't annotate most of the lifetimes in a function.

What regionck does, AFAIK, is it collects constraints from expressions like function calls, propagates the constraints, and produces errors if there are conflicts.
But regionck cannot tell "bad" constraints from "good" constraints, and "bad" ones can easily propagate further than "good" ones.

That just made me think that blame assignment heuristics are likely a waste of time.
However, listing all the constraint sources, like function calls, and annotating lifetimes in those expressions, that would be highly helpful.

@wycats I believe we "just" need to list the expressions generating those type variables, in a similar way to my suggestion for lifetimes above.

@tomjakubowski
Copy link
Contributor

cc me

@pcwalton
Copy link
Contributor

pcwalton commented Oct 9, 2014

What is the nomination here? Disallowing lifetime elision inside an impl generic over lifetimes?

I would like to suggest refraining from complaining about how long things take to implement. The reason why P-backcompat-lang issues take priority is obvious. The reason why P-backcompat-lang issues are not done is that people keep filing, nominating, and accepting new ones, expanding the scope of the language for 1.0. For example, we have 10 (10!!) nominations right now, just for this week.

@pnkfelix
Copy link
Member

pnkfelix commented Oct 9, 2014

A proposal to changes to the lifetime elisions rules needs an RFC. @zwarich is going to write one.

@zwarich
Copy link

zwarich commented Oct 10, 2014

I opened the RFC PR: rust-lang/rfcs#383.

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

No branches or pull requests

10 participants