improve lifetime annotation diagnostics

compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs

@@ -56,6 +56,8 @@ impl<'tcx> UniverseInfo<'tcx> {
     ) {
         match self.0 {
             UniverseInfoInner::RelateTys { expected, found } => {
+                let (expected, found) =
+                    mbcx.regioncx.name_regions(mbcx.infcx.tcx, (expected, found));
                 let err = mbcx.infcx.report_mismatched_types(
                     &cause,
                     expected,

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;
 
@@ -208,9 +209,10 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                 }
 
                 RegionErrorKind::UnexpectedHiddenRegion { span, hidden_ty, key, member_region } => {
-                    let named_ty = self.regioncx.name_regions(self.infcx.tcx, hidden_ty);
-                    let named_key = self.regioncx.name_regions(self.infcx.tcx, key);
-                    let named_region = self.regioncx.name_regions(self.infcx.tcx, member_region);
+                    let named_ty = self.regioncx.name_regions_opaque(self.infcx.tcx, hidden_ty);
+                    let named_key = self.regioncx.name_regions_opaque(self.infcx.tcx, key);
+                    let named_region =
+                        self.regioncx.name_regions_opaque(self.infcx.tcx, member_region);
                     self.buffer_error(unexpected_hidden_region_diagnostic(
                         self.infcx.tcx,
                         span,
@@ -714,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
@@ -902,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

@@ -1875,6 +1875,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     /// `results`. The paths are stored as a series of
     /// `ConstraintIndex` values -- in other words, a list of *edges*.
     ///
+    /// We have a special handling for `TypeAnnotation` constraints in that we don't report a path
+    /// with such constraint unless we're sure that no other path exists.
+    /// This enables us to say that a lifetime annotation is unnecessarily restrictive if it
+    /// appears in the constraint path, and thus we can safely suggest removing it.
+    ///
     /// Returns: a series of constraints as well as the region `R`
     /// that passed the target test.
     pub(crate) fn find_constraint_paths_between_regions(
@@ -1888,10 +1893,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         // Use a deque so that we do a breadth-first search. We will
         // stop at the first match, which ought to be the shortest
         // path (fewest constraints).
-        let mut deque = VecDeque::new();
-        deque.push_back(from_region);
+        let mut deque_p0 = VecDeque::new(); // Higher priority queue.
+        let mut deque_p1 = VecDeque::new(); // Lower priority queue. See method docs.
+        deque_p0.push_back(from_region);
 
-        while let Some(r) = deque.pop_front() {
+        while let Some(r) = deque_p0.pop_front().or_else(|| deque_p1.pop_front()) {
             debug!(
                 "find_constraint_paths_between_regions: from_region={:?} r={:?} value={}",
                 from_region,
@@ -1939,8 +1945,14 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                 debug_assert_eq!(constraint.sup, r);
                 let sub_region = constraint.sub;
                 if let Trace::NotVisited = context[sub_region] {
+                    let constraint_category = constraint.category;
                     context[sub_region] = Trace::FromOutlivesConstraint(constraint);
-                    deque.push_back(sub_region);
+                    match constraint_category {
+                        ConstraintCategory::TypeAnnotation => deque_p1.push_back(sub_region),
+                        // FIXME A `ClosureBounds` constraint can be mapped to `TypeAnnotation`
+                        // later. It should be treated as such here but we're ingoring that.
+                        _ => deque_p0.push_back(sub_region),
+                    }
                 }
             };
 
@@ -2054,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={:?})",
@@ -2115,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);
 
@@ -2259,6 +2297,24 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         categorized_path.remove(0)
     }
 
+    /// Replaces region variables with equal universal regions where possible.
+    pub(crate) fn name_regions<T>(&self, tcx: TyCtxt<'tcx>, ty: T) -> T
+    where
+        T: TypeFoldable<'tcx>,
+    {
+        tcx.fold_regions(ty, |region, _| match *region {
+            ty::ReVar(vid) => {
+                let scc = self.constraint_sccs.scc(vid);
+                let normalized = self.scc_representatives[scc];
+                match &self.definitions[normalized].external_name {
+                    Some(universal_r) => *universal_r,
+                    None => region,
+                }
+            }
+            _ => region,
+        })
+    }
+
     pub(crate) fn universe_info(&self, universe: ty::UniverseIndex) -> UniverseInfo<'tcx> {
         self.universe_causes[&universe].clone()
     }

compiler/rustc_borrowck/src/region_infer/opaque_types.rs

@@ -153,7 +153,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     /// that the regions produced are in fact equal to the named region they are
     /// replaced with. This is fine because this function is only to improve the
     /// region names in error messages.
-    pub(crate) fn name_regions<T>(&self, tcx: TyCtxt<'tcx>, ty: T) -> T
+    pub(crate) fn name_regions_opaque<T>(&self, tcx: TyCtxt<'tcx>, ty: T) -> T
     where
         T: TypeFoldable<'tcx>,
     {

compiler/rustc_borrowck/src/type_check/input_output.rs

@@ -101,9 +101,12 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
                 // argument N is stored in local N+2.
                 let local = Local::new(argument_index + 2);
                 let mir_input_ty = body.local_decls[local].ty;
+                // FIXME span should point to the type annotation, not the argument.
                 let mir_input_span = body.local_decls[local].source_info.span;
 
                 // If the user explicitly annotated the input types, enforce those.
+                let user_provided_input_ty =
+                    self.extract_annotations(user_provided_input_ty, mir_input_span);
                 let user_provided_input_ty =
                     self.normalize(user_provided_input_ty, Locations::All(mir_input_span));
 

compiler/rustc_borrowck/src/type_check/mod.rs

@@ -494,7 +494,7 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'tcx> {
                     ty::Variance::Invariant,
                     user_ty,
                     Locations::All(*span),
-                    ConstraintCategory::TypeAnnotation,
+                    ConstraintCategory::Boring,
                 ) {
                     span_mirbug!(
                         self,
@@ -1076,6 +1076,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
             let CanonicalUserTypeAnnotation { span, ref user_ty, inferred_ty } = *user_annotation;
             let inferred_ty = self.normalize(inferred_ty, Locations::All(span));
             let annotation = self.instantiate_canonical_with_fresh_inference_vars(span, user_ty);
+            let annotation = self.extract_annotations(annotation, span);
             match annotation {
                 UserType::Ty(mut ty) => {
                     ty = self.normalize(ty, Locations::All(span));

compiler/rustc_borrowck/src/type_check/relate_tys.rs

@@ -4,7 +4,7 @@ use rustc_infer::traits::ObligationCause;
 use rustc_middle::mir::ConstraintCategory;
 use rustc_middle::ty::error::TypeError;
 use rustc_middle::ty::relate::TypeRelation;
-use rustc_middle::ty::{self, Const, Ty};
+use rustc_middle::ty::{self, Const, Ty, TypeFoldable};
 use rustc_span::Span;
 use rustc_trait_selection::traits::query::Fallible;
 
@@ -55,6 +55,29 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
         .relate(a, b)?;
         Ok(())
     }
+
+    /// Given a user annotation, `val`, this registers the minimum necessary region constraints
+    /// under the category `ConstraintCategory::TypeAnnotation` and replaces annotated lifetimes
+    /// with fresh inference vars.
+    pub(super) fn extract_annotations<T: TypeFoldable<'tcx>>(&mut self, val: T, span: Span) -> T {
+        let tcx = self.infcx.tcx;
+        let mut relate = NllTypeRelatingDelegate::new(
+            self,
+            Locations::All(span),
+            ConstraintCategory::TypeAnnotation,
+            UniverseInfo::other(),
+        );
+        tcx.fold_regions(val, |region, _| match region.kind() {
+            ty::ReVar(_) => region,
+            ty::ReFree(_) | ty::ReEarlyBound(_) | ty::ReStatic => {
+                let var = relate.next_existential_region_var(false);
+                relate.push_outlives(region, var, ty::VarianceDiagInfo::default());
+                relate.push_outlives(var, region, ty::VarianceDiagInfo::default());
+                var
+            }
+            _ => bug!("unexpected region in type annotation {:?}", region),
+        })
+    }
 }
 
 struct NllTypeRelatingDelegate<'me, 'bccx, 'tcx> {

compiler/rustc_hir/src/hir.rs

@@ -242,6 +242,13 @@ impl<'hir> PathSegment<'hir> {
             DUMMY
         }
     }
+
+    pub fn span(&self) -> Span {
+        match self.args {
+            Some(ref args) => self.ident.span.to(args.span_ext),
+            None => self.ident.span,
+        }
+    }
 }
 
 #[derive(Encodable, Debug, HashStable_Generic)]

compiler/rustc_middle/src/thir.rs

@@ -132,6 +132,8 @@ pub struct Adt<'tcx> {
     /// Optional user-given substs: for something like `let x =
     /// Bar::<T> { ... }`.
     pub user_ty: Option<Canonical<'tcx, UserType<'tcx>>>,
+    /// Constructor span, e.g. `Foo::<u8>` in `Foo::<u8>(1)`.
+    pub ctor_span: Span,
 
     pub fields: Box<[FieldExpr]>,
     /// The base, e.g. `Foo {x: 1, .. base}`.

compiler/rustc_middle/src/thir/visit.rs

@@ -115,6 +115,7 @@ pub fn walk_expr<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, expr: &Exp
             variant_index: _,
             substs: _,
             user_ty: _,
+            ctor_span: _,
         }) => {
             for field in &**fields {
                 visitor.visit_expr(&visitor.thir()[field.expr]);