Report an error when lifetime parameters are shadowed. #19777
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I'm ok with a hard error here for now, the error message should be pretty helpful in fixing it. r=me with a compile-fail test using |
nikomatsakis
force-pushed
the
warn-on-shadowing
branch
2 times, most recently
from
a8a2438 to
784c486
Compare
|
I decided to go with a warning; it seems kinder, and also it will make it easier to land the patch. It does mean we'll need a separate PR to make this a hard error. |
|
FWIW, I think a hard error is fine |
|
@flaper87 honestly my biggest motivation is that I do not want this patch to bounce because of some obscure shadowed lifetime in a test somewhere, or in some windows-specific code |
|
so being kind to myself is also kind to others (a warning makes for an easier transition, whether it's for our code or others') :) |
nikomatsakis
force-pushed
the
warn-on-shadowing
branch
3 times, most recently
from
6f61604 to
6b7a38d
Compare
This is not technically a [breaking-change], but it will be soon, so
you should update your code. Typically, shadowing is accidental, and
the shadowing lifetime can simply be removed. This frequently occurs
in constructor patterns:
```rust
// Old:
impl<'a> SomeStruct<'a> { fn new<'a>(..) -> SomeStruct<'a> { ... } }
// Should be:
impl<'a> SomeStruct<'a> { fn new(..) -> SomeStruct<'a> { ... } }
```
Otherwise, you should rename the inner lifetime to something
else. Note though that lifetime elision frequently applies:
```rust
// Old
impl<'a> SomeStruct<'a> {
fn get<'a>(x: &'a self) -> &'a T { &self.field }
}
// Should be:
impl<'a> SomeStruct<'a> {
fn get(x: &self) -> &T { &self.field }
}
``
nikomatsakis
force-pushed
the
warn-on-shadowing
branch
from
6b7a38d to
1718cd6
Compare
bors
added a commit
that referenced
this pull request
Dec 16, 2014
per rfc 459 cc #19390 One question is: should we start by warning, and only switch to hard error later? I think we discussed something like this in the meeting. r? @alexcrichton
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
per rfc 459
cc #19390
One question is: should we start by warning, and only switch to hard error later? I think we discussed something like this in the meeting.
r? @alexcrichton