-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Suggest replacing named lifetime with 'static
#51513
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -531,7 +531,12 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { | |
s: ROOT_SCOPE, | ||
}; | ||
self.with(scope, |old_scope, this| { | ||
this.check_lifetime_params(old_scope, &generics.params); | ||
this.check_lifetime_params( | ||
old_scope, | ||
&generics.params, | ||
Some(generics.span), | ||
&[], | ||
); | ||
intravisit::walk_item(this, item); | ||
}); | ||
} | ||
|
@@ -574,7 +579,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { | |
self.with(scope, |old_scope, this| { | ||
// a bare fn has no bounds, so everything | ||
// contained within is scoped within its binder. | ||
this.check_lifetime_params(old_scope, &c.generic_params); | ||
this.check_lifetime_params(old_scope, &c.generic_params, None, &[]); | ||
intravisit::walk_ty(this, ty); | ||
}); | ||
self.is_in_fn_syntax = was_in_fn_syntax; | ||
|
@@ -871,7 +876,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { | |
abstract_type_parent: false, | ||
}; | ||
let result = self.with(scope, |old_scope, this| { | ||
this.check_lifetime_params(old_scope, &bound_generic_params); | ||
this.check_lifetime_params(old_scope, &bound_generic_params, None, &[]); | ||
this.visit_ty(&bounded_ty); | ||
walk_list!(this, visit_ty_param_bound, bounds); | ||
}); | ||
|
@@ -938,7 +943,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { | |
abstract_type_parent: false, | ||
}; | ||
self.with(scope, |old_scope, this| { | ||
this.check_lifetime_params(old_scope, &trait_ref.bound_generic_params); | ||
this.check_lifetime_params(old_scope, &trait_ref.bound_generic_params, None, &[]); | ||
walk_list!(this, visit_generic_param, &trait_ref.bound_generic_params); | ||
this.visit_trait_ref(&trait_ref.trait_ref) | ||
}) | ||
|
@@ -1441,6 +1446,18 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { | |
.collect(); | ||
|
||
let next_early_index = index + generics.ty_params().count() as u32; | ||
let mut arg_lifetimes = vec![]; | ||
for input in &decl.inputs { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This vector is only used in the case of lint/error, right? I'm not sure how I feel about building it eagerly -- maybe we can convert this into a closure or a trait or something? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense, I'll see what I can do about it. |
||
for lt in input.lifetimes() { | ||
arg_lifetimes.push(lt); | ||
} | ||
} | ||
if let hir::FunctionRetTy::Return(ref output) = decl.output { | ||
for lt in output.lifetimes() { | ||
arg_lifetimes.push(lt); | ||
} | ||
} | ||
|
||
|
||
let scope = Scope::Binder { | ||
lifetimes, | ||
|
@@ -1450,7 +1467,12 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { | |
track_lifetime_uses: false, | ||
}; | ||
self.with(scope, move |old_scope, this| { | ||
this.check_lifetime_params(old_scope, &generics.params); | ||
this.check_lifetime_params( | ||
old_scope, | ||
&generics.params, | ||
Some(generics.span), | ||
&arg_lifetimes, | ||
); | ||
this.hack(walk); // FIXME(#37666) workaround in place of `walk(this)` | ||
}); | ||
} | ||
|
@@ -2031,7 +2053,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { | |
|
||
if let Some(params) = error { | ||
if lifetime_refs.len() == 1 { | ||
self.report_elision_failure(&mut err, params); | ||
self.report_elision_failure(&mut err, params, span); | ||
} | ||
} | ||
|
||
|
@@ -2042,6 +2064,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { | |
&mut self, | ||
db: &mut DiagnosticBuilder, | ||
params: &[ElisionFailureInfo], | ||
span: Span, | ||
) { | ||
let mut m = String::new(); | ||
let len = params.len(); | ||
|
@@ -2097,18 +2120,22 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { | |
"this function's return type contains a borrowed value, but \ | ||
there is no value for it to be borrowed from" | ||
); | ||
help!(db, "consider giving it a 'static lifetime"); | ||
db.span_suggestion( | ||
span, | ||
"consider giving it a `'static` lifetime", | ||
"&'static ".to_owned(), | ||
); | ||
} else if elided_len == 0 { | ||
help!( | ||
db, | ||
"this function's return type contains a borrowed value with \ | ||
an elided lifetime, but the lifetime cannot be derived from \ | ||
the arguments" | ||
); | ||
help!( | ||
db, | ||
"consider giving it an explicit bounded or 'static \ | ||
lifetime" | ||
db.span_suggestion( | ||
span, | ||
"consider giving it an explicit bound or `'static` lifetime", | ||
"&'static ".to_owned(), | ||
); | ||
} else if elided_len == 1 { | ||
help!( | ||
|
@@ -2149,82 +2176,147 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { | |
self.insert_lifetime(lifetime_ref, lifetime.shifted(late_depth)); | ||
} | ||
|
||
fn check_lifetime_params(&mut self, old_scope: ScopeRef, params: &'tcx [hir::GenericParam]) { | ||
for (i, lifetime_i) in params.lifetimes().enumerate() { | ||
match lifetime_i.lifetime.name { | ||
hir::LifetimeName::Static | hir::LifetimeName::Underscore => { | ||
let lifetime = lifetime_i.lifetime; | ||
let name = lifetime.name.name(); | ||
let mut err = struct_span_err!( | ||
self.tcx.sess, | ||
lifetime.span, | ||
E0262, | ||
"invalid lifetime parameter name: `{}`", | ||
name | ||
); | ||
err.span_label( | ||
lifetime.span, | ||
format!("{} is a reserved lifetime name", name), | ||
); | ||
err.emit(); | ||
} | ||
hir::LifetimeName::Fresh(_) | ||
| hir::LifetimeName::Implicit | ||
| hir::LifetimeName::Name(_) => {} | ||
} | ||
|
||
// It is a hard error to shadow a lifetime within the same scope. | ||
for lifetime_j in params.lifetimes().skip(i + 1) { | ||
if lifetime_i.lifetime.name == lifetime_j.lifetime.name { | ||
struct_span_err!( | ||
self.tcx.sess, | ||
lifetime_j.lifetime.span, | ||
E0263, | ||
"lifetime name `{}` declared twice in the same scope", | ||
lifetime_j.lifetime.name.name() | ||
).span_label(lifetime_j.lifetime.span, "declared twice") | ||
.span_label(lifetime_i.lifetime.span, "previous declaration here") | ||
.emit(); | ||
} | ||
} | ||
|
||
// It is a soft error to shadow a lifetime within a parent scope. | ||
self.check_lifetime_def_for_shadowing(old_scope, &lifetime_i.lifetime); | ||
|
||
for bound in &lifetime_i.bounds { | ||
match bound.name { | ||
hir::LifetimeName::Underscore => { | ||
fn check_lifetime_params( | ||
&mut self, | ||
old_scope: ScopeRef, | ||
params: &'tcx [hir::GenericParam], | ||
generics_span: Option<Span>, | ||
arg_lifetimes: &[hir::Lifetime], | ||
) { | ||
for (i, param_i) in params.iter().enumerate() { | ||
if let hir::GenericParam::Lifetime(lifetime_i) = param_i { | ||
match lifetime_i.lifetime.name { | ||
hir::LifetimeName::Static | hir::LifetimeName::Underscore => { | ||
let lifetime = lifetime_i.lifetime; | ||
let name = lifetime.name.name(); | ||
let mut err = struct_span_err!( | ||
self.tcx.sess, | ||
bound.span, | ||
E0637, | ||
"invalid lifetime bound name: `'_`" | ||
lifetime.span, | ||
E0262, | ||
"invalid lifetime parameter name: `{}`", | ||
name | ||
); | ||
err.span_label( | ||
lifetime.span, | ||
format!("{} is a reserved lifetime name", name), | ||
); | ||
err.span_label(bound.span, "`'_` is a reserved lifetime name"); | ||
err.emit(); | ||
} | ||
hir::LifetimeName::Static => { | ||
self.insert_lifetime(bound, Region::Static); | ||
self.tcx | ||
.sess | ||
.struct_span_warn( | ||
lifetime_i.lifetime.span.to(bound.span), | ||
hir::LifetimeName::Fresh(_) | ||
| hir::LifetimeName::Implicit | ||
| hir::LifetimeName::Name(_) => {} | ||
} | ||
|
||
// It is a hard error to shadow a lifetime within the same scope. | ||
for param_j in params.iter().skip(i + 1) { | ||
if let hir::GenericParam::Lifetime(lifetime_j) = param_j { | ||
if lifetime_i.lifetime.name == lifetime_j.lifetime.name { | ||
struct_span_err!( | ||
self.tcx.sess, | ||
lifetime_j.lifetime.span, | ||
E0263, | ||
"lifetime name `{}` declared twice in the same scope", | ||
lifetime_j.lifetime.name.name() | ||
).span_label(lifetime_j.lifetime.span, "declared twice") | ||
.span_label(lifetime_i.lifetime.span, "previous declaration here") | ||
.emit(); | ||
} | ||
} | ||
} | ||
|
||
// It is a soft error to shadow a lifetime within a parent scope. | ||
self.check_lifetime_def_for_shadowing(old_scope, &lifetime_i.lifetime); | ||
|
||
for bound in &lifetime_i.bounds { | ||
match bound.name { | ||
hir::LifetimeName::Underscore => { | ||
let mut err = struct_span_err!( | ||
self.tcx.sess, | ||
bound.span, | ||
E0637, | ||
"invalid lifetime bound name: `'_`" | ||
); | ||
err.span_label(bound.span, "`'_` is a reserved lifetime name"); | ||
err.emit(); | ||
} | ||
hir::LifetimeName::Static => { | ||
self.insert_lifetime(bound, Region::Static); | ||
let sp = lifetime_i.lifetime.span.to(bound.span); | ||
let mut warn = self.tcx.sess.struct_span_warn( | ||
sp, | ||
&format!( | ||
"unnecessary lifetime parameter `{}`", | ||
lifetime_i.lifetime.name.name() | ||
), | ||
) | ||
.help(&format!( | ||
"you can use the `'static` lifetime directly, in place \ | ||
of `{}`", | ||
lifetime_i.lifetime.name.name() | ||
)) | ||
.emit(); | ||
} | ||
hir::LifetimeName::Fresh(_) | ||
| hir::LifetimeName::Implicit | ||
| hir::LifetimeName::Name(_) => { | ||
self.resolve_lifetime_ref(bound); | ||
); | ||
|
||
// all the use spans for this lifetime in arguments to be replaced | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should extract this into a helper fn, but the basic idea seems pretty good 💯 |
||
// with `'static` | ||
let mut spans_to_replace = arg_lifetimes.iter().filter_map(|lifetime| { | ||
if lifetime.name.name() == lifetime_i.lifetime.name.name() { | ||
Some((lifetime.span, "'static".to_owned())) | ||
} else { | ||
None | ||
} | ||
}).collect::<Vec<_>>(); | ||
// all the use spans for this lifetime in lifetime bounds | ||
for param in params.iter() { | ||
if let hir::GenericParam::Lifetime(lifetime_def) = param { | ||
for lifetime in &lifetime_def.bounds { | ||
if lifetime.name.name() == lifetime_i.lifetime.name.name() { | ||
spans_to_replace.push((lifetime.span, | ||
"'static".to_owned())); | ||
} | ||
} | ||
} | ||
} | ||
|
||
if let (1, Some(sp)) = (params.len(), generics_span) { | ||
spans_to_replace.push((sp, "".into())); | ||
warn.multipart_suggestion( | ||
&format!( | ||
"you can use the `'static` lifetime directly, \ | ||
in place of `{}`", | ||
lifetime_i.lifetime.name.name(), | ||
), | ||
spans_to_replace, | ||
); | ||
} else if params.len() > 1 { | ||
let sp = if let Some(next_param) = params.iter().nth(i + 1) { | ||
// we'll remove everything until the next parameter | ||
lifetime_i.lifetime.span.until(next_param.span()) | ||
} else if let Some(prev_param) = params.iter().nth(i - 1) { | ||
// this must be the last argument, include the previous comma | ||
self.tcx.sess.codemap() | ||
.next_point(prev_param.span()) | ||
.to(sp) | ||
} else { // THIS SHOULDN'T HAPPEN :| | ||
sp | ||
}; | ||
|
||
spans_to_replace.push((sp, "".into())); | ||
|
||
warn.multipart_suggestion( | ||
&format!( | ||
"you can use the `'static` lifetime directly, \ | ||
in place of `{}`", | ||
lifetime_i.lifetime.name.name(), | ||
), | ||
spans_to_replace, | ||
); | ||
} else { | ||
warn.help(&format!( | ||
"you can use the `'static` lifetime directly, in place of `{}`", | ||
lifetime_i.lifetime.name.name(), | ||
)); | ||
} | ||
warn.emit(); | ||
} | ||
hir::LifetimeName::Fresh(_) | ||
| hir::LifetimeName::Implicit | ||
| hir::LifetimeName::Name(_) => { | ||
self.resolve_lifetime_ref(bound); | ||
} | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT | ||
// file at the top-level directory of this distribution and at | ||
// http://rust-lang.org/COPYRIGHT. | ||
// | ||
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or | ||
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license | ||
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your | ||
// option. This file may not be copied, modified, or distributed | ||
// except according to those terms. | ||
|
||
struct Foo<'a>(&'a str); | ||
|
||
fn _foo<'a: 'static, 'b>(_x: &'a str, _y: &'b str) -> &'a str { | ||
//~^ WARNING unnecessary lifetime parameter `'a` | ||
"" | ||
} | ||
|
||
fn _foo1<'b, 'a: 'static>(_x: &'a str, _y: Foo<'a>) -> &'a str { | ||
//~^ WARNING unnecessary lifetime parameter `'a` | ||
"" | ||
} | ||
|
||
fn _foo2<'c, 'a: 'static, 'b>(_x: &'a str, _y: &'b str) -> &'a str { | ||
//~^ WARNING unnecessary lifetime parameter `'a` | ||
"" | ||
} | ||
|
||
fn _foo3<'c, 'a: 'static, 'b: 'a, 'd>(_x: &'a str, _y: &'b str) -> &'a str { | ||
//~^ WARNING unnecessary lifetime parameter `'a` | ||
"" | ||
} | ||
|
||
fn _foo4<'a: 'static>(_x: &'a str, _y: &str) -> &'a str { | ||
//~^ WARNING unnecessary lifetime parameter `'a` | ||
"" | ||
} | ||
|
||
fn _foo5<'a: 'static, 'b>(_x: &'a str, _y: &'b str) -> &'b str { | ||
//~^ WARNING unnecessary lifetime parameter `'a` | ||
"" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
error[E0601]: `main` function not found in crate `static_lifetime` | ||
| | ||
= note: consider adding a `main` function to `$DIR/static-lifetime.rs` | ||
|
||
warning: unnecessary lifetime parameter `'a` | ||
--> $DIR/static-lifetime.rs:13:9 | ||
| | ||
LL | fn _foo<'a: 'static, 'b>(_x: &'a str, _y: &'b str) -> &'a str { | ||
| ^^^^^^^^^^^ | ||
help: you can use the `'static` lifetime directly, in place of `'a` | ||
| | ||
LL | fn _foo<'b>(_x: &'static str, _y: &'b str) -> &'static str { | ||
| -- ^^^^^^^ ^^^^^^^ | ||
|
||
warning: unnecessary lifetime parameter `'a` | ||
--> $DIR/static-lifetime.rs:18:14 | ||
| | ||
LL | fn _foo1<'b, 'a: 'static>(_x: &'a str, _y: Foo<'a>) -> &'a str { | ||
| ^^^^^^^^^^^ | ||
help: you can use the `'static` lifetime directly, in place of `'a` | ||
| | ||
LL | fn _foo1<'b>(_x: &'static str, _y: Foo<'static>) -> &'static str { | ||
| -- ^^^^^^^ ^^^^^^^ ^^^^^^^ | ||
|
||
warning: unnecessary lifetime parameter `'a` | ||
--> $DIR/static-lifetime.rs:23:14 | ||
| | ||
LL | fn _foo2<'c, 'a: 'static, 'b>(_x: &'a str, _y: &'b str) -> &'a str { | ||
| ^^^^^^^^^^^ | ||
help: you can use the `'static` lifetime directly, in place of `'a` | ||
| | ||
LL | fn _foo2<'c, 'b>(_x: &'static str, _y: &'b str) -> &'static str { | ||
| -- ^^^^^^^ ^^^^^^^ | ||
|
||
warning: unnecessary lifetime parameter `'a` | ||
--> $DIR/static-lifetime.rs:28:14 | ||
| | ||
LL | fn _foo3<'c, 'a: 'static, 'b: 'a, 'd>(_x: &'a str, _y: &'b str) -> &'a str { | ||
| ^^^^^^^^^^^ | ||
help: you can use the `'static` lifetime directly, in place of `'a` | ||
| | ||
LL | fn _foo3<'c, 'b: 'static, 'd>(_x: &'static str, _y: &'b str) -> &'static str { | ||
| -- ^^^^^^^ ^^^^^^^ ^^^^^^^ | ||
|
||
warning: unnecessary lifetime parameter `'a` | ||
--> $DIR/static-lifetime.rs:33:10 | ||
| | ||
LL | fn _foo4<'a: 'static>(_x: &'a str, _y: &str) -> &'a str { | ||
| ^^^^^^^^^^^ | ||
help: you can use the `'static` lifetime directly, in place of `'a` | ||
| | ||
LL | fn _foo4(_x: &'static str, _y: &str) -> &'static str { | ||
| -- ^^^^^^^ ^^^^^^^ | ||
|
||
warning: unnecessary lifetime parameter `'a` | ||
--> $DIR/static-lifetime.rs:38:10 | ||
| | ||
LL | fn _foo5<'a: 'static, 'b>(_x: &'a str, _y: &'b str) -> &'b str { | ||
| ^^^^^^^^^^^ | ||
help: you can use the `'static` lifetime directly, in place of `'a` | ||
| | ||
LL | fn _foo5<'b>(_x: &'static str, _y: &'b str) -> &'b str { | ||
| -- ^^^^^^^ | ||
|
||
error: aborting due to previous error | ||
|
||
For more information about this error, try `rustc --explain E0601`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we use a visitor for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, what is this used for? It seems like it doesn't handle shadowing (which, I guess, we disallow at present, so maybe that's ok).
(I was thinking of types like
for<'a> fn(&'a u32)
, which mention'a
, but not the same'a
as might be in scope outside... but as I said at present we do disallow such shadowing)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's only used to gather all the lifetime spans in the argument and return tys that match the name being checked.