From 47aabb382259709819b663283aa1cb247be08611 Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Sat, 14 Jul 2018 17:41:32 -0700 Subject: [PATCH] polish invisible-lifetimes lint: terminal-legibility and macro tolerance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • The "indicate the anonymous lifetime: `'_,`" span label seemed at risk of being hard to read, even if it's not wrong. When we're splicing the anonymous lifetime into an already-existing set of angle-brackets like this, it's probably better to show the entire expression. • By using the path span instead of that of its final segment, we can (very) slightly simplify our code (by the amount of one less local variable), and handle at least some macro scenarios correctly (see `autowrapper!` in the UI test, which is a run-rustfix test)! • Another macro scenario (in type-annotation position) is observed to work. This remains part of the grand endeavor of issue no. 52041! --- src/librustc/middle/resolve_lifetime.rs | 18 ++++++++--- .../in-band-lifetimes/elided-lifetimes.fixed | 31 +++++++++++++++++++ .../ui/in-band-lifetimes/elided-lifetimes.rs | 31 +++++++++++++++++++ .../in-band-lifetimes/elided-lifetimes.stderr | 26 +++++++++++++--- 4 files changed, 96 insertions(+), 10 deletions(-) diff --git a/src/librustc/middle/resolve_lifetime.rs b/src/librustc/middle/resolve_lifetime.rs index a4592d9482f6a..16d5e0112a1f5 100644 --- a/src/librustc/middle/resolve_lifetime.rs +++ b/src/librustc/middle/resolve_lifetime.rs @@ -2109,16 +2109,15 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { ); 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() == 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_span.shrink_to_hi(), "<'_>") + (path.span.shrink_to_hi(), "<'_>".to_owned()) } else { // Otherwise—sorry, this is kind of gross—we need to infer the - // replacement point span from the generics that do exist + // place to splice in `'_, ` from the generics that do exist let mut first_generic_span = None; for ref arg in &generic_args.args { match arg { @@ -2143,9 +2142,18 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> { break; } } - let replace_span = first_generic_span + let first_generic_span = first_generic_span .expect("checked earlier that non-implicit args or bindings exist"); - (replace_span.shrink_to_lo(), "'_, ") + // If our codemap can be trusted, construct the entire `Path<'_, T>` source + // suggestion + if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(path.span) { + let (before, after) = snippet.split_at( + (first_generic_span.lo().0 - path.span.lo().0) as usize + ); + (path.span, format!("{}{}{}", before, "'_, ", after)) + } else { + (first_generic_span.shrink_to_lo(), "'_, ".to_owned()) + } }; err.span_suggestion_with_applicability( replace_span, diff --git a/src/test/ui/in-band-lifetimes/elided-lifetimes.fixed b/src/test/ui/in-band-lifetimes/elided-lifetimes.fixed index 5d82cc8fd939a..c4d5d95036880 100644 --- a/src/test/ui/in-band-lifetimes/elided-lifetimes.fixed +++ b/src/test/ui/in-band-lifetimes/elided-lifetimes.fixed @@ -46,10 +46,41 @@ fn wrap_gift_with_bow(gift: &str) -> WrappedWithBow<'_> { WrappedWithBow { gift } } +macro_rules! autowrapper { + ($type_name:ident, $fn_name:ident, $lt:lifetime) => { + struct $type_name<$lt> { + gift: &$lt str + } + + fn $fn_name(gift: &str) -> $type_name<'_> { + //~^ ERROR implicit lifetime parameters in types are deprecated + //~| HELP indicate the anonymous lifetime + $type_name { gift } + } + } +} + +autowrapper!(Autowrapped, autowrap_gift, 'a); +//~^ NOTE in this expansion of autowrapper! +//~| NOTE in this expansion of autowrapper! + +macro_rules! anytuple_ref_ty { + ($($types:ty),*) => { + Ref<'_, ($($types),*)> + //~^ ERROR implicit lifetime parameters in types are deprecated + //~| HELP indicate the anonymous lifetime + } +} + fn main() { let honesty = RefCell::new((4, 'e')); let loyalty: Ref<'_, (u32, char)> = honesty.borrow(); //~^ ERROR implicit lifetime parameters in types are deprecated //~| HELP indicate the anonymous lifetime let generosity = Ref::map(loyalty, |t| &t.0); + + let laughter = RefCell::new((true, "magic")); + let yellow: anytuple_ref_ty!(bool, &str) = laughter.borrow(); + //~^ NOTE in this expansion of anytuple_ref_ty! + //~| NOTE in this expansion of anytuple_ref_ty! } diff --git a/src/test/ui/in-band-lifetimes/elided-lifetimes.rs b/src/test/ui/in-band-lifetimes/elided-lifetimes.rs index b0158b8b3f484..92676777b3824 100644 --- a/src/test/ui/in-band-lifetimes/elided-lifetimes.rs +++ b/src/test/ui/in-band-lifetimes/elided-lifetimes.rs @@ -46,10 +46,41 @@ fn wrap_gift_with_bow(gift: &str) -> WrappedWithBow { WrappedWithBow { gift } } +macro_rules! autowrapper { + ($type_name:ident, $fn_name:ident, $lt:lifetime) => { + struct $type_name<$lt> { + gift: &$lt str + } + + fn $fn_name(gift: &str) -> $type_name { + //~^ ERROR implicit lifetime parameters in types are deprecated + //~| HELP indicate the anonymous lifetime + $type_name { gift } + } + } +} + +autowrapper!(Autowrapped, autowrap_gift, 'a); +//~^ NOTE in this expansion of autowrapper! +//~| NOTE in this expansion of autowrapper! + +macro_rules! anytuple_ref_ty { + ($($types:ty),*) => { + Ref<($($types),*)> + //~^ ERROR implicit lifetime parameters in types are deprecated + //~| HELP indicate the anonymous lifetime + } +} + fn main() { let honesty = RefCell::new((4, 'e')); let loyalty: Ref<(u32, char)> = honesty.borrow(); //~^ ERROR implicit lifetime parameters in types are deprecated //~| HELP indicate the anonymous lifetime let generosity = Ref::map(loyalty, |t| &t.0); + + let laughter = RefCell::new((true, "magic")); + let yellow: anytuple_ref_ty!(bool, &str) = laughter.borrow(); + //~^ NOTE in this expansion of anytuple_ref_ty! + //~| NOTE in this expansion of anytuple_ref_ty! } diff --git a/src/test/ui/in-band-lifetimes/elided-lifetimes.stderr b/src/test/ui/in-band-lifetimes/elided-lifetimes.stderr index 4aba457c60edf..b3da5279a17fc 100644 --- a/src/test/ui/in-band-lifetimes/elided-lifetimes.stderr +++ b/src/test/ui/in-band-lifetimes/elided-lifetimes.stderr @@ -23,12 +23,28 @@ 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 + --> $DIR/elided-lifetimes.rs:77:18 | LL | let loyalty: Ref<(u32, char)> = honesty.borrow(); - | ^^^^-^^^^^^^^^^^ - | | - | help: indicate the anonymous lifetime: `'_,` + | ^^^^^^^^^^^^^^^^ help: indicate the anonymous lifetime: `Ref<'_, (u32, char)>` -error: aborting due to 4 previous errors +error: implicit lifetime parameters in types are deprecated + --> $DIR/elided-lifetimes.rs:69:9 + | +LL | Ref<($($types),*)> + | ^^^^^^^^^^^^^^^^^^ help: indicate the anonymous lifetime: `Ref<'_, ($($types),*)>` +... +LL | let yellow: anytuple_ref_ty!(bool, &str) = laughter.borrow(); + | ---------------------------- in this macro invocation + +error: implicit lifetime parameters in types are deprecated + --> $DIR/elided-lifetimes.rs:55:36 + | +LL | fn $fn_name(gift: &str) -> $type_name { + | ^^^^^^^^^^- help: indicate the anonymous lifetime: `<'_>` +... +LL | autowrapper!(Autowrapped, autowrap_gift, 'a); + | --------------------------------------------- in this macro invocation + +error: aborting due to 6 previous errors