Skip to content

Commit

Permalink
only do the elided-lifetimes-in-paths lint on type paths
Browse files Browse the repository at this point in the history
Instead of linting for invisible lifetimes during (a callee of)
`visit_path` (in the `LifetimeContext` visitor), we do it in the
`hir::TyPath` branch of `visit_ty`. This fixes a false positive where
we were suggesting `<'_>` on the struct name of a struct construction
expression. (Applying the suggestion would result in a parse error.)

Also, it seems better for the primary lint span (as distinguished from
the zero-width suggestion span where we propose the insertion of
anonymous lifetimes) to cover the entire path including any
angle-bracketed type parameters, rather than just the last path-name
segment. That is: we should highlight all of `Ref<T>` when we suggest
replacing it with `Ref<'_, T>`, rather than just highlighting the
`Ref`.

(Interestingly, this false positive didn't happen for tuple structs
like the one we already had in our UI test, probably because of
something something value vs. type namespace &c. that the present
author doesn't understand.)

This is in the matter of issue no. 52041.
  • Loading branch information
zackmdavis committed Jul 15, 2018
1 parent 7d03b39 commit 22c4f56
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 21 deletions.
42 changes: 25 additions & 17 deletions src/librustc/middle/resolve_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,10 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
self.with(scope, |_, this| this.visit_ty(&mt.ty));
}
hir::TyPath(hir::QPath::Resolved(None, ref path)) => {
let segment = &path.segments[path.segments.len()-1];
if let Some(ref args) = segment.args {
self.lint_implicit_lifetimes_in_typath(path, args);
}
if let Def::Existential(exist_ty_did) = path.def {
assert!(exist_ty_did.is_local());
// Resolve the lifetimes that are applied to the existential type.
Expand Down Expand Up @@ -869,7 +873,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
for (i, ref segment) in path.segments.iter().enumerate() {
let depth = path.segments.len() - i - 1;
if let Some(ref args) = segment.args {
self.visit_segment_args(path.def, depth, segment.ident, args);
self.visit_segment_args(path.def, depth, args);
}
}
}
Expand Down Expand Up @@ -1667,7 +1671,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
&mut self,
def: Def,
depth: usize,
segment_ident: ast::Ident,
generic_args: &'tcx hir::GenericArgs,
) {
if generic_args.parenthesized {
Expand All @@ -1690,9 +1693,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
_ => None,
}).collect::<Vec<_>>();
if elide_lifetimes {
self.lint_implicit_lifetimes_in_segment(
segment_ident, generic_args, &lifetimes
);
self.resolve_elided_lifetimes(lifetimes);
} else {
lifetimes.iter().for_each(|lt| self.visit_lifetime(lt));
Expand Down Expand Up @@ -2070,30 +2070,38 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
}
}

fn lint_implicit_lifetimes_in_segment(&mut self,
segment_ident: ast::Ident,
generic_args: &'tcx hir::GenericArgs,
lifetime_refs: &[&'tcx hir::Lifetime]) {
let num_implicit_lifetimes = lifetime_refs.iter()
.filter(|lt| lt.name.is_implicit()).count();
if num_implicit_lifetimes == 0 {
fn lint_implicit_lifetimes_in_typath(&mut self,
path: &hir::Path,
generic_args: &'tcx hir::GenericArgs) {
let implicit_lifetimes = generic_args.args.iter()
.filter_map(|arg| {
if let hir::GenericArg::Lifetime(lt) = arg {
if lt.name.is_implicit() {
return Some(lt)
}
}
None
}).collect::<Vec<_>>();

if implicit_lifetimes.len() == 0 {
return;
}

let mut err = self.tcx.struct_span_lint_node(
lint::builtin::ELIDED_LIFETIMES_IN_PATHS,
lifetime_refs[0].id, // FIXME: HirIdify #50928
segment_ident.span,
implicit_lifetimes[0].id, // FIXME: HirIdify #50928
path.span,
&format!("implicit lifetime parameters in types are deprecated"),
);

if num_implicit_lifetimes == 1 {
if implicit_lifetimes.len() == 1 {
let segment_span = &path.segments[path.segments.len()-1].ident.span;
let (replace_span,
suggestion) = if generic_args.args.len() == num_implicit_lifetimes &&
suggestion) = if generic_args.args.len() == implicit_lifetimes.len() &&
generic_args.bindings.is_empty() {
// If there are no (non-implicit) generic args or bindings, our
// suggestion includes the angle brackets
(segment_ident.span.shrink_to_hi(), "<'_>")
(segment_span.shrink_to_hi(), "<'_>")
} else {
// Otherwise—sorry, this is kind of gross—we need to infer the
// replacement point span from the generics that do exist
Expand Down
9 changes: 9 additions & 0 deletions src/test/ui/in-band-lifetimes/elided-lifetimes.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,21 @@ fn bar(x: &Foo<'_>) {}

struct Wrapped<'a>(&'a str);

struct WrappedWithBow<'a> {
gift: &'a str
}

fn wrap_gift(gift: &str) -> Wrapped<'_> {
//~^ ERROR implicit lifetime parameters in types are deprecated
//~| HELP indicate the anonymous lifetime
Wrapped(gift)
}

fn wrap_gift_with_bow(gift: &str) -> WrappedWithBow<'_> {
//~^ ERROR implicit lifetime parameters in types are deprecated
//~| HELP indicate the anonymous lifetime
WrappedWithBow { gift }
}

fn main() {
let honesty = RefCell::new((4, 'e'));
Expand Down
9 changes: 9 additions & 0 deletions src/test/ui/in-band-lifetimes/elided-lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,21 @@ fn bar(x: &Foo<'_>) {}

struct Wrapped<'a>(&'a str);

struct WrappedWithBow<'a> {
gift: &'a str
}

fn wrap_gift(gift: &str) -> Wrapped {
//~^ ERROR implicit lifetime parameters in types are deprecated
//~| HELP indicate the anonymous lifetime
Wrapped(gift)
}

fn wrap_gift_with_bow(gift: &str) -> WrappedWithBow {
//~^ ERROR implicit lifetime parameters in types are deprecated
//~| HELP indicate the anonymous lifetime
WrappedWithBow { gift }
}

fn main() {
let honesty = RefCell::new((4, 'e'));
Expand Down
16 changes: 12 additions & 4 deletions src/test/ui/in-band-lifetimes/elided-lifetimes.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,24 @@ LL | #![deny(elided_lifetimes_in_paths)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error: implicit lifetime parameters in types are deprecated
--> $DIR/elided-lifetimes.rs:33:29
--> $DIR/elided-lifetimes.rs:37:29
|
LL | fn wrap_gift(gift: &str) -> Wrapped {
| ^^^^^^^- help: indicate the anonymous lifetime: `<'_>`

error: implicit lifetime parameters in types are deprecated
--> $DIR/elided-lifetimes.rs:42:18
--> $DIR/elided-lifetimes.rs:43:38
|
LL | fn wrap_gift_with_bow(gift: &str) -> WrappedWithBow {
| ^^^^^^^^^^^^^^- help: indicate the anonymous lifetime: `<'_>`

error: implicit lifetime parameters in types are deprecated
--> $DIR/elided-lifetimes.rs:51:18
|
LL | let loyalty: Ref<(u32, char)> = honesty.borrow();
| ^^^ - help: indicate the anonymous lifetime: `'_,`
| ^^^^-^^^^^^^^^^^
| |
| help: indicate the anonymous lifetime: `'_,`

error: aborting due to 3 previous errors
error: aborting due to 4 previous errors

0 comments on commit 22c4f56

Please sign in to comment.