Split elided_lifetime_in_paths into tied and untied

Types that contain a reference can be confusing when lifetime elision
occurs:

```rust
// Confusing
fn foo(_: &u8) -> Bar { todo!() }

// Less confusing
fn foo(_: &u8) -> Bar<'_> { todo!() }
```

However, the previous lint did not distinguish when these types were
not "tying" lifetimes across the function inputs / outputs:

```rust
// Maybe a little confusing
fn foo(_: Bar) {}

// More explicit but noisier with less obvious value
fn foo(_: Bar<'_>) {}
```

We now report different lints for each case, hopefully paving the way
to marking the first case (when lifetimes are tied together) as
warn-by-default.

Author: shepmaster

compiler/rustc_lint/src/lib.rs

@@ -303,7 +303,8 @@ fn register_builtins(store: &mut LintStore) {
         BARE_TRAIT_OBJECTS,
         UNUSED_EXTERN_CRATES,
         ELLIPSIS_INCLUSIVE_RANGE_PATTERNS,
-        ELIDED_LIFETIMES_IN_PATHS,
+        ELIDED_LIFETIMES_IN_PATHS_TIED,
+        ELIDED_LIFETIMES_IN_PATHS_UNTIED,
         EXPLICIT_OUTLIVES_REQUIREMENTS,
         // FIXME(#52665, #47816) not always applicable and not all
         // macros are ready for this yet.
@@ -313,6 +314,12 @@ fn register_builtins(store: &mut LintStore) {
         // MACRO_USE_EXTERN_CRATE
     );
 
+    add_lint_group!(
+        "elided_lifetimes_in_paths",
+        ELIDED_LIFETIMES_IN_PATHS_TIED,
+        ELIDED_LIFETIMES_IN_PATHS_UNTIED,
+    );
+
     // Register renamed and removed lints.
     store.register_renamed("single_use_lifetime", "single_use_lifetimes");
     // store.register_renamed("elided_lifetime_in_path", "elided_lifetimes_in_paths");

compiler/rustc_lint_defs/src/builtin.rs

@@ -40,7 +40,8 @@ declare_lint_pass! {
         DEPRECATED_WHERE_CLAUSE_LOCATION,
         DUPLICATE_MACRO_ATTRIBUTES,
         ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT,
-        ELIDED_LIFETIMES_IN_PATHS,
+        ELIDED_LIFETIMES_IN_PATHS_TIED,
+        ELIDED_LIFETIMES_IN_PATHS_UNTIED,
         EXPORTED_PRIVATE_DEPENDENCIES,
         FFI_UNWIND_CALLS,
         FORBIDDEN_LINT_GROUPS,
@@ -1701,8 +1702,9 @@ declare_lint! {
 }
 
 declare_lint! {
-    /// The `elided_lifetimes_in_paths` lint detects the use of hidden
-    /// lifetime parameters.
+    /// The `elided_lifetimes_in_paths_tied` lint detects the use of
+    /// hidden lifetime parameters when those lifetime parameters tie
+    /// an input lifetime parameter to an output lifetime parameter.
     ///
     /// ### Example
     ///
@@ -1713,7 +1715,8 @@ declare_lint! {
     ///     x: &'a u32
     /// }
     ///
-    /// fn foo(x: &Foo) {
+    /// fn foo(x: Foo) -> &u32 {
+    ///     &x.0
     /// }
     /// ```
     ///
@@ -1730,12 +1733,51 @@ declare_lint! {
     /// may require a significant transition for old code.
     ///
     /// [placeholder lifetime]: https://doc.rust-lang.org/reference/lifetime-elision.html#lifetime-elision-in-functions
-    pub ELIDED_LIFETIMES_IN_PATHS,
+    pub ELIDED_LIFETIMES_IN_PATHS_TIED,
     Allow,
     "hidden lifetime parameters in types are deprecated",
     crate_level_only
 }
 
+declare_lint! {
+    /// The `elided_lifetimes_in_paths_untied` lint detects the use of
+    /// hidden lifetime parameters when those lifetime parameters do
+    /// not tie an input lifetime parameter to an output lifetime
+    /// parameter.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,compile_fail
+    /// #![deny(elided_lifetimes_in_paths)]
+    /// #![deny(warnings)]
+    /// struct Foo<'a> {
+    ///     x: &'a u32
+    /// }
+    ///
+    /// fn foo(x: Foo) -> u32 {
+    ///     x.0
+    /// }
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// Elided lifetime parameters can make it difficult to see at a glance
+    /// that borrowing is occurring. This lint ensures that lifetime
+    /// parameters are always explicitly stated, even if it is the `'_`
+    /// [placeholder lifetime].
+    ///
+    /// This lint is "allow" by default because it has some known issues, and
+    /// may require a significant transition for old code.
+    ///
+    /// [placeholder lifetime]: https://doc.rust-lang.org/reference/lifetime-elision.html#lifetime-elision-in-functions
+    pub ELIDED_LIFETIMES_IN_PATHS_UNTIED,
+    Allow,
+    "hidden lifetime parameters in types make it hard to tell when borrowing is happening",
+    crate_level_only
+}
+
 declare_lint! {
     /// The `bare_trait_objects` lint suggests using `dyn Trait` for trait
     /// objects.

compiler/rustc_resolve/src/late.rs

@@ -646,7 +646,22 @@ struct DiagnosticMetadata<'ast> {
 }
 
 #[derive(Debug)]
-struct ResolvedNestedElisionTarget {
+enum ResolvedElisionTarget {
+    TopLevel(NodeId),
+    Nested(NestedResolvedElisionTarget),
+}
+
+impl ResolvedElisionTarget {
+    fn node_id(&self) -> NodeId {
+        match *self {
+            Self::TopLevel(n) => n,
+            Self::Nested(NestedResolvedElisionTarget { segment_id, .. }) => segment_id,
+        }
+    }
+}
+
+#[derive(Debug)]
+struct NestedResolvedElisionTarget {
     segment_id: NodeId,
     elided_lifetime_span: Span,
     diagnostic: lint::BuiltinLintDiagnostics,
@@ -694,7 +709,7 @@ struct LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
     lifetime_uses: FxHashMap<LocalDefId, LifetimeUseSet>,
 
     /// Track which types participated in lifetime elision
-    resolved_lifetime_elisions: Vec<ResolvedNestedElisionTarget>,
+    resolved_lifetime_elisions: Vec<ResolvedElisionTarget>,
 }
 
 /// Walks the whole crate in DFS order, visiting each item, resolving names as it goes.
@@ -1745,6 +1760,8 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
             LifetimeElisionCandidate::Ignore,
         );
         self.resolve_anonymous_lifetime(&lt, true);
+
+        self.resolved_lifetime_elisions.push(ResolvedElisionTarget::TopLevel(anchor_id));
     }
 
     #[instrument(level = "debug", skip(self))]
@@ -1957,16 +1974,18 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
             }
 
             if should_lint {
-                self.resolved_lifetime_elisions.push(ResolvedNestedElisionTarget {
-                    segment_id,
-                    elided_lifetime_span,
-                    diagnostic: lint::BuiltinLintDiagnostics::ElidedLifetimesInPaths(
-                        expected_lifetimes,
-                        path_span,
-                        !segment.has_generic_args,
+                self.resolved_lifetime_elisions.push(ResolvedElisionTarget::Nested(
+                    NestedResolvedElisionTarget {
+                        segment_id,
                         elided_lifetime_span,
-                    ),
-                });
+                        diagnostic: lint::BuiltinLintDiagnostics::ElidedLifetimesInPaths(
+                            expected_lifetimes,
+                            path_span,
+                            !segment.has_generic_args,
+                            elided_lifetime_span,
+                        ),
+                    },
+                ));
             }
         }
     }
@@ -2028,22 +2047,80 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
 
         let elision_failures =
             replace(&mut self.diagnostic_metadata.current_elision_failures, outer_failures);
-        if !elision_failures.is_empty() {
-            let Err(failure_info) = elision_lifetime else { bug!() };
-            self.report_missing_lifetime_specifiers(elision_failures, Some(failure_info));
-        }
+
+        let elision_lifetime = match (elision_failures.is_empty(), elision_lifetime) {
+            (true, Ok(lifetime)) => Some(lifetime),
+
+            (true, Err(_)) => None,
+
+            (false, Ok(_)) => bug!(),
+
+            (false, Err(failure_info)) => {
+                self.report_missing_lifetime_specifiers(elision_failures, Some(failure_info));
+                None
+            }
+        };
+
+        // We've recorded all elisions that occurred in the params and
+        // outputs, categorized by top-level or nested.
+        //
+        // Our primary lint case is when an output lifetime is tied to
+        // an input lifetime. In that case, we want to warn about any
+        // parameters that had a nested elision.
+        //
+        // The secondary case is for nested elisions that are not part
+        // of the tied lifetime relationship.
 
         let output_resolved_lifetime_elisions =
             replace(&mut self.resolved_lifetime_elisions, outer_resolved_lifetime_elisions);
 
-        let resolved_lifetime_elisions =
-            param_resolved_lifetime_elisions.into_iter().chain(output_resolved_lifetime_elisions);
+        match (output_resolved_lifetime_elisions.is_empty(), elision_lifetime) {
+            (true, _) | (_, None) => {
+                // Treat all parameters as untied
+                self.report_elided_lifetimes_in_paths(
+                    param_resolved_lifetime_elisions,
+                    lint::builtin::ELIDED_LIFETIMES_IN_PATHS_UNTIED,
+                );
+            }
+            (false, Some(elision_lifetime)) => {
+                let (primary, secondary): (Vec<_>, Vec<_>) =
+                    param_resolved_lifetime_elisions.into_iter().partition(|re| {
+                        // Recover the `NodeId` of an elided lifetime
+                        let lvl1 = &self.r.lifetimes_res_map[&re.node_id()];
+                        let lvl2 = match lvl1 {
+                            LifetimeRes::ElidedAnchor { start, .. } => {
+                                &self.r.lifetimes_res_map[&start]
+                            }
+                            o => o,
+                        };
+
+                        lvl2 == &elision_lifetime
+                    });
+
+                self.report_elided_lifetimes_in_paths(
+                    primary.into_iter().chain(output_resolved_lifetime_elisions),
+                    lint::builtin::ELIDED_LIFETIMES_IN_PATHS_TIED,
+                );
+                self.report_elided_lifetimes_in_paths(
+                    secondary,
+                    lint::builtin::ELIDED_LIFETIMES_IN_PATHS_UNTIED,
+                );
+            }
+        }
+    }
+
+    fn report_elided_lifetimes_in_paths(
+        &mut self,
+        resolved_elisions: impl IntoIterator<Item = ResolvedElisionTarget>,
+        lint: &'static lint::Lint,
+    ) {
+        for re in resolved_elisions {
+            let ResolvedElisionTarget::Nested(d) = re else { continue };
 
-        for re in resolved_lifetime_elisions {
-            let ResolvedNestedElisionTarget { segment_id, elided_lifetime_span, diagnostic } = re;
+            let NestedResolvedElisionTarget { segment_id, elided_lifetime_span, diagnostic } = d;
 
             self.r.lint_buffer.buffer_lint_with_diagnostic(
-                lint::builtin::ELIDED_LIFETIMES_IN_PATHS,
+                lint,
                 segment_id,
                 elided_lifetime_span,
                 "hidden lifetime parameters in types are deprecated",

src/tools/lint-docs/src/groups.rs

@@ -11,6 +11,7 @@ static GROUP_DESCRIPTIONS: &[(&str, &str)] = &[
     ("let-underscore", "Lints that detect wildcard let bindings that are likely to be invalid"),
     ("rustdoc", "Rustdoc-specific lints"),
     ("rust-2018-idioms", "Lints to nudge you toward idiomatic features of Rust 2018"),
+    ("elided-lifetimes-in-paths", "Lints that detect the use of hidden lifetime parameters"),
     ("nonstandard-style", "Violation of standard naming conventions"),
     ("future-incompatible", "Lints that detect code that has future-compatibility problems"),
     ("rust-2018-compatibility", "Lints used to transition code from the 2015 edition to 2018"),