From 22c4f5685c5e45dd7d9c23be1090724d5d078625 Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Sat, 14 Jul 2018 12:59:20 -0700 Subject: [PATCH] only do the elided-lifetimes-in-paths lint on type paths 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` 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. --- src/librustc/middle/resolve_lifetime.rs | 42 +++++++++++-------- .../in-band-lifetimes/elided-lifetimes.fixed | 9 ++++ .../ui/in-band-lifetimes/elided-lifetimes.rs | 9 ++++ .../in-band-lifetimes/elided-lifetimes.stderr | 16 +++++-- 4 files changed, 55 insertions(+), 21 deletions(-) diff --git a/src/librustc/middle/resolve_lifetime.rs b/src/librustc/middle/resolve_lifetime.rs index 630c6494c4201..7c6718f3d065e 100644 --- a/src/librustc/middle/resolve_lifetime.rs +++ b/src/librustc/middle/resolve_lifetime.rs @@ -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. @@ -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); } } } @@ -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 { @@ -1690,9 +1693,6 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { _ => None, }).collect::>(); 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)); @@ -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::>(); + + 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 diff --git a/src/test/ui/in-band-lifetimes/elided-lifetimes.fixed b/src/test/ui/in-band-lifetimes/elided-lifetimes.fixed index b3047c13e62cf..24df541b08fe0 100644 --- a/src/test/ui/in-band-lifetimes/elided-lifetimes.fixed +++ b/src/test/ui/in-band-lifetimes/elided-lifetimes.fixed @@ -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')); diff --git a/src/test/ui/in-band-lifetimes/elided-lifetimes.rs b/src/test/ui/in-band-lifetimes/elided-lifetimes.rs index 493a10cbe460b..661dd7530b723 100644 --- a/src/test/ui/in-band-lifetimes/elided-lifetimes.rs +++ b/src/test/ui/in-band-lifetimes/elided-lifetimes.rs @@ -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')); diff --git a/src/test/ui/in-band-lifetimes/elided-lifetimes.stderr b/src/test/ui/in-band-lifetimes/elided-lifetimes.stderr index 6bc2885ce20b8..4aba457c60edf 100644 --- a/src/test/ui/in-band-lifetimes/elided-lifetimes.stderr +++ b/src/test/ui/in-band-lifetimes/elided-lifetimes.stderr @@ -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