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

Shadowing lifetimes should be disallowed or linted #18824

Closed
Manishearth opened this issue Nov 10, 2014 · 5 comments
Closed

Shadowing lifetimes should be disallowed or linted #18824

Manishearth opened this issue Nov 10, 2014 · 5 comments

Comments

@Manishearth
Copy link
Member

The following code compiles:

struct Foo<'a>(&'a uint);

impl<'a> Foo<'a> {
  fn bar<'a>(self, other:Foo<'a>) {}
}

However, in the event of a lifetime mismatch, this can lead to confusing error messages.

use std::kinds::marker::ContravariantLifetime;
// Using a Contravariant lifetime because #[deriving] doesn't seem to work well
// with &-ptrs. Example holds with a covariant one as well.

#[deriving(PartialEq)]
struct Foo<'a>(ContravariantLifetime<'a>, uint);

impl<'a> Foo<'a> {
 fn bar<'a>(self, other: Foo<'a>) -> bool {
   self == other
 }
}

This gives a tautological recommendation along with a confusing error:

test.rs:8:2: 11:3 help: consider using an explicit lifetime parameter as shown: fn bar<'a>(self, other: Foo<'a>) -> bool
test.rs:8  fn bar<'a>(self, other: Foo<'a>) -> bool {
test.rs:9    //baz(self, other)
test.rs:10    self == other
test.rs:11  }
test.rs:10:12: 10:17 error: mismatched types: expected `Foo<'a>`, found `Foo<'a>` (lifetime mismatch)
test.rs:10    self == other

It's not really the error's fault for being so confusing, after all, we have shadowed the 'a from the impl with a 'a from the definition of baz(). This should probably be disallowed or at least linted.

The recommendation is flawed in two ways

  • Firstly, it recommended 'a, a lifetime which is already being used there (twice, actually). That's adding to the confusion. It clearly means to recommend a new lifetime ('b?)
  • It's wrong, even if you change the signature of bar() to use 'b, it's still wrong, but then it gives the correct recommendation to use fn bar(self, other: Foo<'a>) -> bool on recompiling.

This probably needs better error reporting (don't recommend lifetimes which are already being used in that context)

@glennw came across this in Servo, getting tautological errors like this and this. The former is due to lifetime shadowing (and can be fixed by disallowing shadowing). I'm not yet sure what prompted the latter one, one of the lifetimes is elided but the other one is named 'a in the surrounding impl and should have turned up in the error message instead of being underscored.

@japaric
Copy link
Member

japaric commented Nov 10, 2014

@nikomatsakis is working on a RFC that forbids shadowing of generic parameters (types/lifetimes).

@Manishearth
Copy link
Member Author

Great! :D

@japaric
Copy link
Member

japaric commented Nov 10, 2014

@Manishearth There is now a discussion topic on the forums.

@nikomatsakis
Copy link
Contributor

And an rfc: rust-lang/rfcs#459

@dgrunwald
Copy link
Contributor

Looks like this got implemented in #19777
This issue should probably be closed as a duplicate.

@Gankra Gankra closed this as completed Jan 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants