Skip to content

Commit

Permalink
Suggest replacing named lifetime with 'static
Browse files Browse the repository at this point in the history 
```
warning: unnecessary lifetime parameter `'a`
  --> file.rs:11:14
   |
11 | fn foo<'c, 'a: 'static, 'b, 'd>(_x: &'a str) -> &'a str {
   |            ^^^^^^^^^^^
help: you can use the `'static` lifetime directly, in place of `'a`
   |
11 | fn foo<'c, 'b, 'd>(_x: &'static str) -> &'static str {
   |           --            ^^^^^^^          ^^^^^^^

warning: unnecessary lifetime parameter `'a`
  --> file.rs:14:9
   |
14 | fn bar<'a: 'static>(_x: &'a str) -> &'a str {
   |        ^^^^^^^^^^^
help: you can use the `'static` lifetime directly, in place of `'a`
   |
14 | fn bar(_x: &'static str) -> &'static str {
   |      --     ^^^^^^^          ^^^^^^^
```
  • Loading branch information
@estebank
estebank committed Jun 12, 2018
1 parent ae0659c commit 46704fd
Show file tree
Hide file tree
Showing 6 changed files with 301 additions and 90 deletions.
17 changes: 17 additions & 0 deletions src/librustc/hir/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,13 @@ impl GenericParam {
_ => false,
}
}

pub fn span(&self) -> Span {
match *self {
GenericParam::Lifetime(ref lifetime) => lifetime.lifetime.span,
GenericParam::Type(ref typaram) => typaram.span,
}
}
}

pub trait GenericParamsExt {
Expand Down Expand Up @@ -1662,6 +1669,16 @@ pub struct Ty {
pub hir_id: HirId,
}

impl Ty {
pub fn lifetimes(&self) -> Vec<Lifetime> {
// FIXME(estebank): expand to all `Ty_`s
match self.node {
TyRptr(lifetime, _) => vec![lifetime],
_ => vec![],
}
}
}

impl fmt::Debug for Ty {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "type({})",
Expand Down
232 changes: 154 additions & 78 deletions src/librustc/middle/resolve_lifetime.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
});
Expand Down Expand Up @@ -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)
})
Expand Down Expand Up @@ -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 {
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,
Expand All @@ -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)`
});
}
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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();
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -2149,82 +2176,131 @@ 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);
);
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<_>>();
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);
}
}
}
}
Expand Down
12 changes: 8 additions & 4 deletions src/test/ui/issue-26638.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,23 @@ error[E0106]: missing lifetime specifier
--> $DIR/issue-26638.rs:14:40
|
LL | fn parse_type_2(iter: fn(&u8)->&u8) -> &str { iter() }
| ^ expected lifetime parameter
| ^
| |
| expected lifetime parameter
| help: consider giving it an explicit bound or `'static` lifetime: `&'static`
|
= help: this function's return type contains a borrowed value with an elided lifetime, but the lifetime cannot be derived from the arguments
= help: consider giving it an explicit bounded or 'static lifetime

error[E0106]: missing lifetime specifier
--> $DIR/issue-26638.rs:17:22
|
LL | fn parse_type_3() -> &str { unimplemented!() }
| ^ expected lifetime parameter
| ^
| |
| expected lifetime parameter
| help: consider giving it a `'static` lifetime: `&'static`
|
= help: this function's return type contains a borrowed value, but there is no value for it to be borrowed from
= help: consider giving it a 'static lifetime

error: aborting due to 3 previous errors

Expand Down
Loading

0 comments on commit 46704fd

Please sign in to comment.