Use kw::Empty for elided lifetimes in path.

cjgillot · cjgillot · commit 1c737d69979b · 2022-11-24T17:48:59.000Z
diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs
@@ -2101,7 +2101,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                 hir::LifetimeName::Param(param)
             }
             LifetimeRes::Fresh { param, .. } => {
-                debug_assert_eq!(ident.name, kw::UnderscoreLifetime);
                 let param = self.local_def_id(param);
                 hir::LifetimeName::Param(param)
             }
diff --git a/compiler/rustc_ast_lowering/src/path.rs b/compiler/rustc_ast_lowering/src/path.rs
@@ -309,7 +309,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                 let id = NodeId::from_u32(i);
                 let l = self.lower_lifetime(&Lifetime {
                     id,
-                    ident: Ident::new(kw::UnderscoreLifetime, elided_lifetime_span),
+                    ident: Ident::new(kw::Empty, elided_lifetime_span),
                 });
                 GenericArg::Lifetime(l)
             }),
diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs
@@ -29,14 +29,15 @@ use std::fmt;
 #[derive(Debug, Copy, Clone, Encodable, HashStable_Generic)]
 pub struct Lifetime {
     pub hir_id: HirId,
-    pub ident: Ident,
 
     /// Either "`'a`", referring to a named lifetime definition,
-    /// or "``" (i.e., `kw::Empty`), for elision placeholders.
+    /// `'_` referring to an anonymous lifetime (either explicitly `'_` or `&type`),
+    /// or "``" (i.e., `kw::Empty`) when appearing in path.
     ///
-    /// HIR lowering inserts these placeholders in type paths that
-    /// refer to type definitions needing lifetime parameters,
-    /// `&T` and `&mut T`, and trait objects without `... + 'a`.
+    /// See `Lifetime::suggestion_position` for practical use.
+    pub ident: Ident,
+
+    /// Semantics of this lifetime.
     pub res: LifetimeName,
 }
 
@@ -135,6 +136,19 @@ impl fmt::Display for Lifetime {
     }
 }
 
+pub enum LifetimeSuggestionPosition {
+    /// The user wrote `'a` or `'_`.
+    Normal,
+    /// The user wrote `&type` or `&mut type`.
+    Ampersand,
+    /// The user wrote `Path` and omitted the `<'_>`.
+    ElidedPath,
+    /// The user wrote `Path<T>`, and omitted the `'_,`.
+    ElidedPathArgument,
+    /// The user wrote `dyn Trait` and omitted the `+ '_`.
+    ObjectDefault,
+}
+
 impl Lifetime {
     pub fn is_elided(&self) -> bool {
         self.res.is_elided()
@@ -144,6 +158,22 @@ impl Lifetime {
         self.ident.name == kw::Empty || self.ident.name == kw::UnderscoreLifetime
     }
 
+    pub fn suggestion_position(&self) -> (LifetimeSuggestionPosition, Span) {
+        if self.ident.name == kw::Empty {
+            if self.ident.span.is_empty() {
+                (LifetimeSuggestionPosition::ElidedPathArgument, self.ident.span)
+            } else {
+                (LifetimeSuggestionPosition::ElidedPath, self.ident.span.shrink_to_hi())
+            }
+        } else if self.res == LifetimeName::ImplicitObjectLifetimeDefault {
+            (LifetimeSuggestionPosition::ObjectDefault, self.ident.span)
+        } else if self.ident.span.is_empty() {
+            (LifetimeSuggestionPosition::Ampersand, self.ident.span)
+        } else {
+            (LifetimeSuggestionPosition::Normal, self.ident.span)
+        }
+    }
+
     pub fn is_static(&self) -> bool {
         self.res == LifetimeName::Static
     }
diff --git a/compiler/rustc_hir_analysis/src/collect/lifetimes.rs b/compiler/rustc_hir_analysis/src/collect/lifetimes.rs
@@ -19,7 +19,7 @@ use rustc_middle::hir::nested_filter;
 use rustc_middle::middle::resolve_lifetime::*;
 use rustc_middle::ty::{self, DefIdTree, TyCtxt, TypeSuperVisitable, TypeVisitor};
 use rustc_span::def_id::DefId;
-use rustc_span::symbol::{kw, sym, Ident};
+use rustc_span::symbol::{sym, Ident};
 use rustc_span::Span;
 use std::fmt;
 
@@ -1218,13 +1218,15 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
                                 (generics.span, "<'a>".to_owned())
                             };
 
-                            let lifetime_sugg = match lifetime_ref.ident.name {
-                                kw::Empty => "'a, ".to_owned(),
-                                kw::UnderscoreLifetime => "'a ".to_owned(),
-                                _ => "'a ".to_owned(),
+                            let lifetime_sugg = match lifetime_ref.suggestion_position() {
+                                (hir::LifetimeSuggestionPosition::Normal, span) => (span, "'a".to_owned()),
+                                (hir::LifetimeSuggestionPosition::Ampersand, span) => (span, "'a ".to_owned()),
+                                (hir::LifetimeSuggestionPosition::ElidedPath, span) => (span, "<'a>".to_owned()),
+                                (hir::LifetimeSuggestionPosition::ElidedPathArgument, span) => (span, "'a, ".to_owned()),
+                                (hir::LifetimeSuggestionPosition::ObjectDefault, span) => (span, "+ 'a".to_owned()),
                             };
                             let suggestions = vec![
-                                (lifetime_ref.ident.span.shrink_to_hi(), lifetime_sugg),
+                                lifetime_sugg,
                                 new_param_sugg,
                             ];
 
diff --git a/compiler/rustc_middle/src/ty/generics.rs b/compiler/rustc_middle/src/ty/generics.rs
@@ -3,7 +3,7 @@ use crate::ty::{EarlyBinder, SubstsRef};
 use rustc_ast as ast;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_hir::def_id::DefId;
-use rustc_span::symbol::Symbol;
+use rustc_span::symbol::{kw, Symbol};
 use rustc_span::Span;
 
 use super::{EarlyBoundRegion, InstantiatedPredicates, ParamConst, ParamTy, Predicate, TyCtxt};
@@ -78,6 +78,15 @@ impl GenericParamDef {
         }
     }
 
+    pub fn is_anonymous_lifetime(&self) -> bool {
+        match self.kind {
+            GenericParamDefKind::Lifetime => {
+                self.name == kw::UnderscoreLifetime || self.name == kw::Empty
+            }
+            _ => false,
+        }
+    }
+
     pub fn default_value<'tcx>(
         &self,
         tcx: TyCtxt<'tcx>,
diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs
@@ -530,7 +530,7 @@ impl ty::EarlyBoundRegion {
     /// Does this early bound region have a name? Early bound regions normally
     /// always have names except when using anonymous lifetimes (`'_`).
     pub fn has_name(&self) -> bool {
-        self.name != kw::UnderscoreLifetime
+        self.name != kw::UnderscoreLifetime && self.name != kw::Empty
     }
 }
 
diff --git a/compiler/rustc_middle/src/ty/print/pretty.rs b/compiler/rustc_middle/src/ty/print/pretty.rs
@@ -1963,17 +1963,13 @@ impl<'tcx> PrettyPrinter<'tcx> for FmtPrinter<'_, 'tcx> {
         let identify_regions = self.tcx.sess.opts.unstable_opts.identify_regions;
 
         match *region {
-            ty::ReEarlyBound(ref data) => {
-                data.name != kw::Empty && data.name != kw::UnderscoreLifetime
-            }
+            ty::ReEarlyBound(ref data) => data.has_name(),
 
             ty::ReLateBound(_, ty::BoundRegion { kind: br, .. })
             | ty::ReFree(ty::FreeRegion { bound_region: br, .. })
             | ty::RePlaceholder(ty::Placeholder { name: br, .. }) => {
-                if let ty::BrNamed(_, name) = br {
-                    if name != kw::Empty && name != kw::UnderscoreLifetime {
-                        return true;
-                    }
+                if br.is_named() {
+                    return true;
                 }
 
                 if let Some((region, _)) = highlight.highlight_bound_region {
@@ -2049,11 +2045,9 @@ impl<'tcx> FmtPrinter<'_, 'tcx> {
             ty::ReLateBound(_, ty::BoundRegion { kind: br, .. })
             | ty::ReFree(ty::FreeRegion { bound_region: br, .. })
             | ty::RePlaceholder(ty::Placeholder { name: br, .. }) => {
-                if let ty::BrNamed(_, name) = br {
-                    if name != kw::Empty && name != kw::UnderscoreLifetime {
-                        p!(write("{}", name));
-                        return Ok(self);
-                    }
+                if let ty::BrNamed(_, name) = br && br.is_named() {
+                    p!(write("{}", name));
+                    return Ok(self);
                 }
 
                 if let Some((region, counter)) = highlight.highlight_bound_region {
@@ -2266,7 +2260,7 @@ impl<'tcx> FmtPrinter<'_, 'tcx> {
 
                         (name, ty::BrNamed(CRATE_DEF_ID.to_def_id(), name))
                     }
-                    ty::BrNamed(def_id, kw::UnderscoreLifetime) => {
+                    ty::BrNamed(def_id, kw::UnderscoreLifetime | kw::Empty) => {
                         let name = next_name(&self);
 
                         if let Some(lt_idx) = lifetime_idx {
diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs
@@ -83,7 +83,9 @@ pub struct BoundRegion {
 impl BoundRegionKind {
     pub fn is_named(&self) -> bool {
         match *self {
-            BoundRegionKind::BrNamed(_, name) => name != kw::UnderscoreLifetime,
+            BoundRegionKind::BrNamed(_, name) => {
+                name != kw::UnderscoreLifetime && name != kw::Empty
+            }
             _ => false,
         }
     }
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
@@ -182,9 +182,7 @@ fn clean_poly_trait_ref_with_bindings<'tcx>(
         .collect_referenced_late_bound_regions(&poly_trait_ref)
         .into_iter()
         .filter_map(|br| match br {
-            ty::BrNamed(_, name) if name != kw::UnderscoreLifetime => {
-                Some(GenericParamDef::lifetime(name))
-            }
+            ty::BrNamed(_, name) if br.is_named() => Some(GenericParamDef::lifetime(name)),
             _ => None,
         })
         .collect();
@@ -233,16 +231,11 @@ pub(crate) fn clean_middle_const<'tcx>(
 pub(crate) fn clean_middle_region<'tcx>(region: ty::Region<'tcx>) -> Option<Lifetime> {
     match *region {
         ty::ReStatic => Some(Lifetime::statik()),
+        _ if !region.has_name() => None,
         ty::ReLateBound(_, ty::BoundRegion { kind: ty::BrNamed(_, name), .. }) => {
-            if name != kw::UnderscoreLifetime { Some(Lifetime(name)) } else { None }
-        }
-        ty::ReEarlyBound(ref data) => {
-            if data.name != kw::UnderscoreLifetime {
-                Some(Lifetime(data.name))
-            } else {
-                None
-            }
+            Some(Lifetime(name))
         }
+        ty::ReEarlyBound(ref data) => Some(Lifetime(data.name)),
         ty::ReLateBound(..)
         | ty::ReFree(..)
         | ty::ReVar(..)
@@ -396,7 +389,7 @@ fn clean_projection_predicate<'tcx>(
         .collect_referenced_late_bound_regions(&pred)
         .into_iter()
         .filter_map(|br| match br {
-            ty::BrNamed(_, name) if name != kw::UnderscoreLifetime => Some(Lifetime(name)),
+            ty::BrNamed(_, name) if br.is_named() => Some(Lifetime(name)),
             _ => None,
         })
         .collect();
@@ -660,7 +653,7 @@ fn clean_ty_generics<'tcx>(
         .params
         .iter()
         .filter_map(|param| match param.kind {
-            ty::GenericParamDefKind::Lifetime if param.name == kw::UnderscoreLifetime => None,
+            ty::GenericParamDefKind::Lifetime if param.is_anonymous_lifetime() => None,
             ty::GenericParamDefKind::Lifetime => Some(clean_generic_param_def(param, cx)),
             ty::GenericParamDefKind::Type { synthetic, .. } => {
                 if param.name == kw::SelfUpper {
@@ -1460,8 +1453,11 @@ fn maybe_expand_private_type_alias<'tcx>(
                 });
                 if let Some(lt) = lifetime {
                     let lt_def_id = cx.tcx.hir().local_def_id(param.hir_id);
-                    let cleaned =
-                        if !lt.is_elided() { clean_lifetime(lt, cx) } else { Lifetime::elided() };
+                    let cleaned = if !lt.is_anonymous() {
+                        clean_lifetime(lt, cx)
+                    } else {
+                        Lifetime::elided()
+                    };
                     substs.insert(lt_def_id.to_def_id(), SubstParam::Lifetime(cleaned));
                 }
                 indices.lifetimes += 1;
@@ -1892,7 +1888,7 @@ fn clean_generic_args<'tcx>(
             .args
             .iter()
             .map(|arg| match arg {
-                hir::GenericArg::Lifetime(lt) if !lt.is_elided() => {
+                hir::GenericArg::Lifetime(lt) if !lt.is_anonymous() => {
                     GenericArg::Lifetime(clean_lifetime(*lt, cx))
                 }
                 hir::GenericArg::Lifetime(_) => GenericArg::Lifetime(Lifetime::elided()),
diff --git a/src/test/ui/suggestions/impl-trait-missing-lifetime-gated.rs b/src/test/ui/suggestions/impl-trait-missing-lifetime-gated.rs
@@ -2,20 +2,62 @@
 // gate-test-anonymous_lifetime_in_impl_trait
 // Verify the behaviour of `feature(anonymous_lifetime_in_impl_trait)`.
 
-fn f(_: impl Iterator<Item = &'_ ()>) {}
-//~^ ERROR anonymous lifetimes in `impl Trait` are unstable
+mod elided {
+    fn f(_: impl Iterator<Item = &()>) {}
+    //~^ ERROR anonymous lifetimes in `impl Trait` are unstable
 
-fn g(x: impl Iterator<Item = &'_ ()>) -> Option<&'_ ()> { x.next() }
-//~^ ERROR anonymous lifetimes in `impl Trait` are unstable
-//~| ERROR missing lifetime specifier
+    fn g(mut x: impl Iterator<Item = &()>) -> Option<&()> { x.next() }
+    //~^ ERROR anonymous lifetimes in `impl Trait` are unstable
+    //~| ERROR missing lifetime specifier
 
-// Anonymous lifetimes in async fn are already allowed.
-// This is understood as `fn foo<'_1>(_: impl Iterator<Item = &'_1 ()>) {}`.
-async fn h(_: impl Iterator<Item = &'_ ()>) {}
+    // Anonymous lifetimes in async fn are already allowed.
+    // This is understood as `fn foo<'_1>(_: impl Iterator<Item = &'_1 ()>) {}`.
+    async fn h(_: impl Iterator<Item = &()>) {}
 
-// Anonymous lifetimes in async fn are already allowed.
-// But that lifetime does not participate in resolution.
-async fn i(x: impl Iterator<Item = &'_ ()>) -> Option<&'_ ()> { x.next() }
-//~^ ERROR missing lifetime specifier
+    // Anonymous lifetimes in async fn are already allowed.
+    // But that lifetime does not participate in resolution.
+    async fn i(mut x: impl Iterator<Item = &()>) -> Option<&()> { x.next() }
+    //~^ ERROR missing lifetime specifier
+}
+
+mod underscore {
+    fn f(_: impl Iterator<Item = &'_ ()>) {}
+    //~^ ERROR anonymous lifetimes in `impl Trait` are unstable
+
+    fn g(mut x: impl Iterator<Item = &'_ ()>) -> Option<&'_ ()> { x.next() }
+    //~^ ERROR anonymous lifetimes in `impl Trait` are unstable
+    //~| ERROR missing lifetime specifier
+
+    // Anonymous lifetimes in async fn are already allowed.
+    // This is understood as `fn foo<'_1>(_: impl Iterator<Item = &'_1 ()>) {}`.
+    async fn h(_: impl Iterator<Item = &'_ ()>) {}
+
+    // Anonymous lifetimes in async fn are already allowed.
+    // But that lifetime does not participate in resolution.
+    async fn i(mut x: impl Iterator<Item = &'_ ()>) -> Option<&'_ ()> { x.next() }
+    //~^ ERROR missing lifetime specifier
+}
+
+mod alone_in_path {
+    trait Foo<'a> { fn next(&mut self) -> Option<&'a ()>; }
+
+    fn f(_: impl Foo) {}
+    //~^ ERROR anonymous lifetimes in `impl Trait` are unstable
+
+    fn g(mut x: impl Foo) -> Option<&()> { x.next() }
+    //~^ ERROR anonymous lifetimes in `impl Trait` are unstable
+    //~| ERROR missing lifetime specifier
+}
+
+mod in_path {
+    trait Foo<'a, T> { fn next(&mut self) -> Option<&'a T>; }
+
+    fn f(_: impl Foo<()>) {}
+    //~^ ERROR anonymous lifetimes in `impl Trait` are unstable
+
+    fn g(mut x: impl Foo<()>) -> Option<&()> { x.next() }
+    //~^ ERROR anonymous lifetimes in `impl Trait` are unstable
+    //~| ERROR missing lifetime specifier
+}
 
 fn main() {}
diff --git a/src/test/ui/suggestions/impl-trait-missing-lifetime-gated.stderr b/src/test/ui/suggestions/impl-trait-missing-lifetime-gated.stderr
diff --git a/src/tools/clippy/clippy_lints/src/lifetimes.rs b/src/tools/clippy/clippy_lints/src/lifetimes.rs

Original file line number	Diff line number	Diff line change
`@@ -2101,7 +2101,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {`
`2101`	`2101`	`hir::LifetimeName::Param(param)`
`2102`	`2102`	`}`
`2103`	`2103`	`LifetimeRes::Fresh { param, .. } => {`
`2104`		`- debug_assert_eq!(ident.name, kw::UnderscoreLifetime);`
`2105`	`2104`	`let param = self.local_def_id(param);`
`2106`	`2105`	`hir::LifetimeName::Param(param)`
`2107`	`2106`	`}`
Original file line number	Diff line number	Diff line change
`@@ -530,7 +530,7 @@ impl ty::EarlyBoundRegion {`
`530`	`530`	`/// Does this early bound region have a name? Early bound regions normally`
`531`	`531`	/// always have names except when using anonymous lifetimes (`'_`).
`532`	`532`	`pub fn has_name(&self) -> bool {`
`533`		`- self.name != kw::UnderscoreLifetime`
	`533`	`+ self.name != kw::UnderscoreLifetime && self.name != kw::Empty`
`534`	`534`	`}`
`535`	`535`	`}`
`536`	`536`
Original file line number	Diff line number	Diff line change
`@@ -83,7 +83,9 @@ pub struct BoundRegion {`
`83`	`83`	`impl BoundRegionKind {`
`84`	`84`	`pub fn is_named(&self) -> bool {`
`85`	`85`	`match *self {`
`86`		`- BoundRegionKind::BrNamed(_, name) => name != kw::UnderscoreLifetime,`
	`86`	`+ BoundRegionKind::BrNamed(_, name) => {`
	`87`	`+ name != kw::UnderscoreLifetime && name != kw::Empty`
	`88`	`+ }`
`87`	`89`	`_ => false,`
`88`	`90`	`}`
`89`	`91`	`}`