suggest removing unnecessary lifetime annotations

Author: aliemjay

compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs

@@ -21,7 +21,7 @@ use crate::{
     WriteKind,
 };
 
-use super::{find_use, RegionName, UseSpans};
+use super::{find_use, region_errors, RegionName, UseSpans};
 
 #[derive(Debug)]
 pub(crate) enum BorrowExplanation<'tcx> {
@@ -268,6 +268,10 @@ impl<'tcx> BorrowExplanation<'tcx> {
                     );
                 };
 
+                if let ConstraintCategory::TypeAnnotation = category {
+                    err.span_help(span, "you can remove unnecessary lifetime annotations here");
+                    region_errors::suggest_relax_self(tcx, err, body.source.def_id(), span);
+                }
                 self.add_lifetime_bound_suggestion_to_diagnostic(err, &category, span, region_name);
             }
             _ => {}

compiler/rustc_borrowck/src/diagnostics/region_errors.rs

@@ -2,6 +2,7 @@
 
 use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan};
+use rustc_hir::def::DefKind;
 use rustc_hir::def_id::DefId;
 use rustc_hir::intravisit::Visitor;
 use rustc_hir::{self as hir, Item, ItemKind, Node};
@@ -18,7 +19,7 @@ use rustc_middle::mir::{ConstraintCategory, ReturnConstraint};
 use rustc_middle::ty::subst::InternalSubsts;
 use rustc_middle::ty::Region;
 use rustc_middle::ty::TypeVisitor;
-use rustc_middle::ty::{self, RegionVid, Ty};
+use rustc_middle::ty::{self, DefIdTree, RegionVid, Ty, TyCtxt};
 use rustc_span::symbol::{kw, sym, Ident};
 use rustc_span::Span;
 
@@ -715,12 +716,53 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
             }
         }
 
+        if let ConstraintCategory::TypeAnnotation = category {
+            self.handle_annotation_error(errci, &mut diag);
+        }
         self.add_static_impl_trait_suggestion(&mut diag, *fr, fr_name, *outlived_fr);
         self.suggest_adding_lifetime_params(&mut diag, *fr, *outlived_fr);
 
         diag
     }
 
+    /// Adds a secondary label to lifetime annotation errors:
+    /// ```
+    /// fn test<'a, 'b>(s: &'a str) {
+    ///     let closure = |_: &'b str| {}; // Primary label: type annotation requires 'a: 'b.
+    ///     closure(s); // Secondary label: because of this call.
+    /// }
+    /// ```
+    ///
+    /// Also Suggests replacing `Self`. See `suggest_relax_self`.
+    fn handle_annotation_error(&self, errci: &ErrorConstraintInfo<'tcx>, err: &mut Diagnostic) {
+        let ErrorConstraintInfo { fr, outlived_fr, span, category: _, .. } = errci.clone();
+
+        // Try to report the second most relevant constraint to provide more context.
+        let constraint2 = self.regioncx.best_blame_constraint_filtered(
+            &self.body,
+            fr,
+            NllRegionVariableOrigin::FreeRegion,
+            |r| self.regioncx.provides_universal_region(r, fr, outlived_fr),
+            |constraint| match constraint.category {
+                ConstraintCategory::TypeAnnotation => false,
+                _ => true,
+            },
+        );
+        debug!("suggest_relax_annotation: constraint2={:?}", constraint2);
+        let map = self.infcx.tcx.sess.source_map();
+        let same_line = map
+            .lookup_line(constraint2.cause.span.hi())
+            .and_then(|line1| map.lookup_line(span.hi()).map(|line2| line1.line == line2.line))
+            .unwrap_or(false);
+        let desc = constraint2.category.description();
+        if !desc.is_empty() && !same_line {
+            err.span_label(constraint2.cause.span, format!("because of {desc}here"));
+        }
+
+        let body_did = self.body.source.def_id();
+        suggest_relax_self(self.infcx.tcx, err, body_did, span);
+    }
+
     /// Adds a suggestion to errors where an `impl Trait` is returned.
     ///
     /// ```text
@@ -903,3 +945,46 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
         suggest_adding_lifetime_params(self.infcx.tcx, sub, ty_sup, ty_sub, diag);
     }
 }
+
+/// Use of `Self` can impose unnecessary region constraints:
+/// ```
+/// struct MyStruct<'a>(&'a str);
+/// impl<'a> MyStruct<'a> {
+///     fn test<'x>(s: &'x str) {
+///         let _ = Self(s); // requires 'x: 'a.
+///     }
+/// }
+/// ```
+/// Here we suggest replacing `Self` with `MyStruct<'_>`.
+/// Assumes that `span` points to hir::Path that may name `Self`.
+pub(crate) fn suggest_relax_self<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    err: &mut Diagnostic,
+    body_did: DefId,
+    span: Span,
+) {
+    use rustc_lexer as lex;
+    fn tokenize(mut input: &str) -> impl Iterator<Item = (lex::TokenKind, &str)> {
+        std::iter::from_fn(move || {
+            if input.is_empty() {
+                return None;
+            }
+            let token = lex::first_token(input);
+            let token_str = &input[..(token.len as usize)];
+            input = &input[(token.len as usize)..];
+            Some((token.kind, token_str))
+        })
+    }
+
+    let snippet = tcx.sess.source_map().span_to_snippet(span).unwrap_or_default();
+    let has_self = tokenize(&snippet)
+        .find(|(tok, s)| *tok == lex::TokenKind::Ident && *s == "Self")
+        .and_then(|_| tcx.opt_parent(tcx.typeck_root_def_id(body_did)))
+        .filter(|parent_did| tcx.def_kind(parent_did) == DefKind::Impl);
+
+    if let Some(impl_did) = has_self {
+        let suggested_ty =
+            tcx.fold_regions(tcx.type_of(impl_did), |_, _| tcx.mk_region(ty::ReErased));
+        err.help(format!("consider replacing `Self` with `{suggested_ty}`"));
+    }
+}

compiler/rustc_borrowck/src/diagnostics/region_name.rs

@@ -867,7 +867,8 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> {
         };
 
         let tcx = self.infcx.tcx;
-        let body_parent_did = tcx.opt_parent(self.mir_def_id().to_def_id())?;
+        let typeck_root_did = tcx.typeck_root_def_id(self.mir_def_id().to_def_id());
+        let body_parent_did = tcx.opt_parent(typeck_root_did)?;
         if tcx.parent(region.def_id) != body_parent_did
             || tcx.def_kind(body_parent_did) != DefKind::Impl
         {

compiler/rustc_borrowck/src/region_infer/mod.rs

@@ -2066,6 +2066,25 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         from_region: RegionVid,
         from_region_origin: NllRegionVariableOrigin,
         target_test: impl Fn(RegionVid) -> bool,
+    ) -> BlameConstraint<'tcx> {
+        self.best_blame_constraint_filtered(
+            body,
+            from_region,
+            from_region_origin,
+            target_test,
+            |_| true,
+        )
+    }
+
+    /// Same as `best_blame_constraint` but this can exclude some constraints from being reported
+    /// as the "best" according to `filter`.
+    pub(crate) fn best_blame_constraint_filtered(
+        &self,
+        body: &Body<'tcx>,
+        from_region: RegionVid,
+        from_region_origin: NllRegionVariableOrigin,
+        target_test: impl Fn(RegionVid) -> bool,
+        filter: impl Fn(&BlameConstraint<'tcx>) -> bool,
     ) -> BlameConstraint<'tcx> {
         debug!(
             "best_blame_constraint(from_region={:?}, from_region_origin={:?})",
@@ -2127,6 +2146,13 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                     }
                 }
             })
+            // Downgrade the category of filtered constraints to be ignored later.
+            .map(|mut blame| {
+                if !filter(&blame) {
+                    blame.category = ConstraintCategory::Internal;
+                }
+                blame
+            })
             .collect();
         debug!("best_blame_constraint: categorized_path={:#?}", categorized_path);
 

src/test/ui/associated-types/associated-types-subtyping-1.stderr

@@ -8,6 +8,9 @@ LL | fn method2<'a,'b,T>(x: &'a T, y: &'b T)
 ...
 LL |     let a: <T as Trait<'a>>::Type = make_any();
    |            ^^^^^^^^^^^^^^^^^^^^^^ type annotation requires that `'b` must outlive `'a`
+...
+LL |     let _c: <T as Trait<'b>>::Type = a;
+   |                                      - because of assignment here
    |
    = help: consider adding the following bound: `'b: 'a`
 

src/test/ui/consts/const-eval/const-eval-intrinsic-promotion.stderr

@@ -7,6 +7,12 @@ LL |         &std::intrinsics::size_of::<i32>();
    |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ creates a temporary which is freed while still in use
 LL | }
    | - temporary value is freed at the end of this statement
+   |
+help: you can remove unnecessary lifetime annotations here
+  --> $DIR/const-eval-intrinsic-promotion.rs:4:12
+   |
+LL |     let x: &'static usize =
+   |            ^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 

src/test/ui/consts/const-eval/dont_promote_unstable_const_fn.stderr

@@ -15,6 +15,12 @@ LL |     let _: &'static u32 = &foo();
    |            type annotation requires that borrow lasts for `'static`
 LL | }
    | - temporary value is freed at the end of this statement
+   |
+help: you can remove unnecessary lifetime annotations here
+  --> $DIR/dont_promote_unstable_const_fn.rs:17:12
+   |
+LL |     let _: &'static u32 = &foo();
+   |            ^^^^^^^^^^^^
 
 error[E0716]: temporary value dropped while borrowed
   --> $DIR/dont_promote_unstable_const_fn.rs:21:28
@@ -26,6 +32,12 @@ LL |     let _: &'static u32 = &meh();
 ...
 LL | }
    | - temporary value is freed at the end of this statement
+   |
+help: you can remove unnecessary lifetime annotations here
+  --> $DIR/dont_promote_unstable_const_fn.rs:21:12
+   |
+LL |     let _: &'static u32 = &meh();
+   |            ^^^^^^^^^^^^
 
 error[E0716]: temporary value dropped while borrowed
   --> $DIR/dont_promote_unstable_const_fn.rs:22:26
@@ -37,6 +49,12 @@ LL |     let x: &'static _ = &std::time::Duration::from_millis(42).subsec_millis
 LL |
 LL | }
    | - temporary value is freed at the end of this statement
+   |
+help: you can remove unnecessary lifetime annotations here
+  --> $DIR/dont_promote_unstable_const_fn.rs:22:12
+   |
+LL |     let x: &'static _ = &std::time::Duration::from_millis(42).subsec_millis();
+   |            ^^^^^^^^^^
 
 error: aborting due to 4 previous errors
 

src/test/ui/consts/const-eval/dont_promote_unstable_const_fn_cross_crate.stderr

@@ -8,6 +8,12 @@ LL |     let _: &'static u32 = &foo();
 LL |     let _x: &'static u32 = &foo();
 LL | }
    | - temporary value is freed at the end of this statement
+   |
+help: you can remove unnecessary lifetime annotations here
+  --> $DIR/dont_promote_unstable_const_fn_cross_crate.rs:8:12
+   |
+LL |     let _: &'static u32 = &foo();
+   |            ^^^^^^^^^^^^
 
 error[E0716]: temporary value dropped while borrowed
   --> $DIR/dont_promote_unstable_const_fn_cross_crate.rs:9:29
@@ -18,6 +24,12 @@ LL |     let _x: &'static u32 = &foo();
    |             type annotation requires that borrow lasts for `'static`
 LL | }
    | - temporary value is freed at the end of this statement
+   |
+help: you can remove unnecessary lifetime annotations here
+  --> $DIR/dont_promote_unstable_const_fn_cross_crate.rs:9:13
+   |
+LL |     let _x: &'static u32 = &foo();
+   |             ^^^^^^^^^^^^
 
 error: aborting due to 2 previous errors
 

src/test/ui/consts/const-eval/promoted_const_fn_fail.stderr

@@ -8,6 +8,12 @@ LL |     let x: &'static u8 = &(bar() + 1);
 ...
 LL | }
    | - temporary value is freed at the end of this statement
+   |
+help: you can remove unnecessary lifetime annotations here
+  --> $DIR/promoted_const_fn_fail.rs:19:12
+   |
+LL |     let x: &'static u8 = &(bar() + 1);
+   |            ^^^^^^^^^^^
 
 error: aborting due to previous error
 

src/test/ui/consts/const-eval/promoted_const_fn_fail_deny_const_err.stderr

@@ -8,6 +8,12 @@ LL |     let x: &'static u8 = &(bar() + 1);
 ...
 LL | }
    | - temporary value is freed at the end of this statement
+   |
+help: you can remove unnecessary lifetime annotations here
+  --> $DIR/promoted_const_fn_fail_deny_const_err.rs:20:12
+   |
+LL |     let x: &'static u8 = &(bar() + 1);
+   |            ^^^^^^^^^^^
 
 error: aborting due to previous error