reliably detect unnecessary lifetime annotations

register lifetime annotations from closure signature and UFCS calls
under `ConstraintCategory::TypeAnnotation` and make sure we don't report
type annotation errors unless thy're the only thing to blame.

Author: aliemjay

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),
+                    }
                 }
             };
 

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> {

src/test/ui/borrowck/borrowck-loan-of-static-data-issue-27616.stderr

@@ -2,12 +2,13 @@ error[E0506]: cannot assign to `*s` because it is borrowed
   --> $DIR/borrowck-loan-of-static-data-issue-27616.rs:16:5
    |
 LL |     let alias: &'static mut String = s;
-   |                -------------------   - borrow of `*s` occurs here
-   |                |
-   |                type annotation requires that `*s` is borrowed for `'static`
+   |                                      - borrow of `*s` occurs here
 ...
 LL |     *s = String::new();
    |     ^^ assignment to borrowed `*s` occurs here
+...
+LL |     println!("{}", inner);
+   |                    ----- borrow later used here
 
 error: aborting due to previous error
 

src/test/ui/closure-expected-type/expect-fn-supply-fn.stderr

@@ -8,7 +8,7 @@ LL |     with_closure_expecting_fn_with_free_region(|x: fn(&'x u32), y| {});
    |                                                 ^
    |                                                 |
    |                                                 has type `fn(&'1 u32)`
-   |                                                 requires that `'1` must outlive `'x`
+   |                                                 type annotation requires that `'1` must outlive `'x`
 
 error: lifetime may not live long enough
   --> $DIR/expect-fn-supply-fn.rs:16:49
@@ -17,7 +17,7 @@ LL | fn expect_free_supply_free_from_fn<'x>(x: &'x u32) {
    |                                    -- lifetime `'x` defined here
 ...
 LL |     with_closure_expecting_fn_with_free_region(|x: fn(&'x u32), y| {});
-   |                                                 ^ requires that `'x` must outlive `'static`
+   |                                                 ^ type annotation requires that `'x` must outlive `'static`
 
 error[E0308]: mismatched types
   --> $DIR/expect-fn-supply-fn.rs:32:49

src/test/ui/closures/closure-expected-type/expect-region-supply-region-2.stderr

@@ -7,7 +7,7 @@ LL | fn expect_bound_supply_named<'x>() {
 LL |     closure_expecting_bound(|x: &'x u32| {
    |                              ^  - let's call the lifetime of this reference `'1`
    |                              |
-   |                              requires that `'1` must outlive `'x`
+   |                              type annotation requires that `'1` must outlive `'x`
 
 error: lifetime may not live long enough
   --> $DIR/expect-region-supply-region-2.rs:14:30
@@ -16,7 +16,7 @@ LL | fn expect_bound_supply_named<'x>() {
    |                              -- lifetime `'x` defined here
 ...
 LL |     closure_expecting_bound(|x: &'x u32| {
-   |                              ^ requires that `'x` must outlive `'static`
+   |                              ^ type annotation requires that `'x` must outlive `'static`
 
 error: aborting due to 2 previous errors
 

src/test/ui/fn/implied-bounds-unnorm-associated-type-2.stderr

@@ -6,12 +6,9 @@ LL | fn g<'a, 'b>() {
    |      |
    |      lifetime `'a` defined here
 LL |     f::<'a, 'b>(());
-   |     ^^^^^^^^^^^^^^^ requires that `'b` must outlive `'a`
+   |     ^^^^^^^^^^^ type annotation requires that `'b` must outlive `'a`
    |
    = help: consider adding the following bound: `'b: 'a`
-   = note: requirement occurs because of a function pointer to `f`
-   = note: the function `f` is invariant over the parameter `'a`
-   = help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
 
 error: aborting due to previous error
 

src/test/ui/higher-rank-trait-bounds/hrtb-cache-issue-54302.stderr

@@ -4,7 +4,7 @@ error: implementation of `Deserialize` is not general enough
 LL |     assert_deserialize_owned::<&'static str>();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Deserialize` is not general enough
    |
-   = note: `&'static str` must implement `Deserialize<'0>`, for any lifetime `'0`...
+   = note: `&str` must implement `Deserialize<'0>`, for any lifetime `'0`...
    = note: ...but `&str` actually implements `Deserialize<'1>`, for some specific lifetime `'1`
 
 error: aborting due to previous error

src/test/ui/higher-rank-trait-bounds/hrtb-just-for-static.stderr

@@ -13,7 +13,7 @@ error: lifetime may not live long enough
 LL | fn give_some<'a>() {
    |              -- lifetime `'a` defined here
 LL |     want_hrtb::<&'a u32>()
-   |     ^^^^^^^^^^^^^^^^^^^^ requires that `'a` must outlive `'static`
+   |     ^^^^^^^^^^^^^^^^^^^^ type annotation requires that `'a` must outlive `'static`
 
 error: implementation of `Foo` is not general enough
   --> $DIR/hrtb-just-for-static.rs:30:5