Re-use logic for adding a suggestion when a lifetime bound is missing on an impl trait

6 files changed, 150 insertions, 91 deletions

diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs
index 5264f7cc326..11d0178e93b 100644
--- a/compiler/rustc_hir/src/hir.rs
+++ b/compiler/rustc_hir/src/hir.rs
@@ -2235,8 +2235,7 @@ pub enum TyKind<'hir> {
     ///
     /// Type parameters may be stored in each `PathSegment`.
     Path(QPath<'hir>),
-    /// An opaque type definition itself. This is currently only used for the
-    /// `opaque type Foo: Trait` item that `impl Trait` in desugars to.
+    /// An opaque type definition itself. This is only used for `impl Trait`.
     ///
     /// The generic argument list contains the lifetimes (and in the future
     /// possibly parameters) that are actually bound on the `impl Trait`.
diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs
index a48e01b1da2..126c25f0c38 100644
--- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs
+++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs
@@ -267,6 +267,18 @@ pub fn unexpected_hidden_region_diagnostic(
                 hidden_region,
                 "",
             );
+            if let Some(reg_info) = tcx.is_suitable_region(hidden_region) {
+                let fn_returns = tcx.return_type_impl_or_dyn_traits(reg_info.def_id);
+                nice_region_error::suggest_new_region_bound(
+                    tcx,
+                    &mut err,
+                    fn_returns,
+                    hidden_region.to_string(),
+                    None,
+                    format!("captures {}", hidden_region),
+                    None,
+                )
+            }
         }
         _ => {
             // Ugh. This is a painful case: the hidden region is not one
diff --git a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/mod.rs
index 3f27bf67b59..6a330977002 100644
--- a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/mod.rs
+++ b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/mod.rs
@@ -14,6 +14,8 @@ mod static_impl_trait;
 mod trait_impl_difference;
 mod util;
 
+pub use static_impl_trait::suggest_new_region_bound;
+
 impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
     pub fn try_report_nice_region_error(&self, error: &RegionResolutionError<'tcx>) -> bool {
         NiceRegionError::new(self, error.clone()).try_report().is_some()
diff --git a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs
index 2d47e72780e..7fba6a823d7 100644
--- a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs
+++ b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs
@@ -217,128 +217,159 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
             ));
         }
 
-        debug!("try_report_static_impl_trait: fn_return={:?}", fn_returns);
-        // FIXME: account for the need of parens in `&(dyn Trait + '_)`
-        let consider = "consider changing the";
-        let declare = "to declare that the";
         let arg = match param.param.pat.simple_ident() {
             Some(simple_ident) => format!("argument `{}`", simple_ident),
             None => "the argument".to_string(),
         };
-        let explicit = format!("you can add an explicit `{}` lifetime bound", lifetime_name);
-        let explicit_static = format!("explicit `'static` bound to the lifetime of {}", arg);
         let captures = format!("captures data from {}", arg);
-        let add_static_bound = "alternatively, add an explicit `'static` bound to this reference";
-        let plus_lt = format!(" + {}", lifetime_name);
-        for fn_return in fn_returns {
-            if fn_return.span.desugaring_kind().is_some() {
-                // Skip `async` desugaring `impl Future`.
-                continue;
-            }
-            match fn_return.kind {
-                TyKind::OpaqueDef(item_id, _) => {
-                    let item = tcx.hir().item(item_id);
-                    let opaque = if let ItemKind::OpaqueTy(opaque) = &item.kind {
-                        opaque
-                    } else {
-                        err.emit();
-                        return Some(ErrorReported);
-                    };
+        suggest_new_region_bound(
+            tcx,
+            &mut err,
+            fn_returns,
+            lifetime_name,
+            Some(arg),
+            captures,
+            Some((param.param_ty_span, param.param_ty.to_string())),
+        );
 
-                    if let Some(span) = opaque
-                        .bounds
-                        .iter()
-                        .filter_map(|arg| match arg {
-                            GenericBound::Outlives(Lifetime {
-                                name: LifetimeName::Static,
-                                span,
-                                ..
-                            }) => Some(*span),
-                            _ => None,
-                        })
-                        .next()
-                    {
+        err.emit();
+        Some(ErrorReported)
+    }
+}
+
+pub fn suggest_new_region_bound(
+    tcx: TyCtxt<'tcx>,
+    err: &mut DiagnosticBuilder<'_>,
+    fn_returns: Vec<&rustc_hir::Ty<'_>>,
+    lifetime_name: String,
+    arg: Option<String>,
+    captures: String,
+    param: Option<(Span, String)>,
+) {
+    debug!("try_report_static_impl_trait: fn_return={:?}", fn_returns);
+    // FIXME: account for the need of parens in `&(dyn Trait + '_)`
+    let consider = "consider changing the";
+    let declare = "to declare that the";
+    let explicit = format!("you can add an explicit `{}` lifetime bound", lifetime_name);
+    let explicit_static =
+        arg.map(|arg| format!("explicit `'static` bound to the lifetime of {}", arg));
+    let add_static_bound = "alternatively, add an explicit `'static` bound to this reference";
+    let plus_lt = format!(" + {}", lifetime_name);
+    for fn_return in fn_returns {
+        if fn_return.span.desugaring_kind().is_some() {
+            // Skip `async` desugaring `impl Future`.
+            continue;
+        }
+        match fn_return.kind {
+            TyKind::OpaqueDef(item_id, _) => {
+                let item = tcx.hir().item(item_id);
+                let opaque = if let ItemKind::OpaqueTy(opaque) = &item.kind {
+                    opaque
+                } else {
+                    return;
+                };
+
+                if let Some(span) = opaque
+                    .bounds
+                    .iter()
+                    .filter_map(|arg| match arg {
+                        GenericBound::Outlives(Lifetime {
+                            name: LifetimeName::Static,
+                            span,
+                            ..
+                        }) => Some(*span),
+                        _ => None,
+                    })
+                    .next()
+                {
+                    if let Some(explicit_static) = &explicit_static {
                         err.span_suggestion_verbose(
                             span,
                             &format!("{} `impl Trait`'s {}", consider, explicit_static),
                             lifetime_name.clone(),
                             Applicability::MaybeIncorrect,
                         );
+                    }
+                    if let Some((param_span, param_ty)) = param.clone() {
                         err.span_suggestion_verbose(
-                            param.param_ty_span,
+                            param_span,
                             add_static_bound,
-                            param.param_ty.to_string(),
-                            Applicability::MaybeIncorrect,
-                        );
-                    } else if opaque
-                        .bounds
-                        .iter()
-                        .filter_map(|arg| match arg {
-                            GenericBound::Outlives(Lifetime { name, span, .. })
-                                if name.ident().to_string() == lifetime_name =>
-                            {
-                                Some(*span)
-                            }
-                            _ => None,
-                        })
-                        .next()
-                        .is_some()
-                    {
-                    } else {
-                        err.span_suggestion_verbose(
-                            fn_return.span.shrink_to_hi(),
-                            &format!(
-                                "{declare} `impl Trait` {captures}, {explicit}",
-                                declare = declare,
-                                captures = captures,
-                                explicit = explicit,
-                            ),
-                            plus_lt.clone(),
+                            param_ty,
                             Applicability::MaybeIncorrect,
                         );
                     }
+                } else if opaque
+                    .bounds
+                    .iter()
+                    .filter_map(|arg| match arg {
+                        GenericBound::Outlives(Lifetime { name, span, .. })
+                            if name.ident().to_string() == lifetime_name =>
+                        {
+                            Some(*span)
+                        }
+                        _ => None,
+                    })
+                    .next()
+                    .is_some()
+                {
+                } else {
+                    err.span_suggestion_verbose(
+                        fn_return.span.shrink_to_hi(),
+                        &format!(
+                            "{declare} `impl Trait` {captures}, {explicit}",
+                            declare = declare,
+                            captures = captures,
+                            explicit = explicit,
+                        ),
+                        plus_lt.clone(),
+                        Applicability::MaybeIncorrect,
+                    );
                 }
-                TyKind::TraitObject(_, lt, _) => match lt.name {
-                    LifetimeName::ImplicitObjectLifetimeDefault => {
-                        err.span_suggestion_verbose(
-                            fn_return.span.shrink_to_hi(),
-                            &format!(
-                                "{declare} trait object {captures}, {explicit}",
-                                declare = declare,
-                                captures = captures,
-                                explicit = explicit,
-                            ),
-                            plus_lt.clone(),
-                            Applicability::MaybeIncorrect,
-                        );
-                    }
-                    name if name.ident().to_string() != lifetime_name => {
-                        // With this check we avoid suggesting redundant bounds. This
-                        // would happen if there are nested impl/dyn traits and only
-                        // one of them has the bound we'd suggest already there, like
-                        // in `impl Foo<X = dyn Bar> + '_`.
+            }
+            TyKind::TraitObject(_, lt, _) => match lt.name {
+                LifetimeName::ImplicitObjectLifetimeDefault => {
+                    err.span_suggestion_verbose(
+                        fn_return.span.shrink_to_hi(),
+                        &format!(
+                            "{declare} trait object {captures}, {explicit}",
+                            declare = declare,
+                            captures = captures,
+                            explicit = explicit,
+                        ),
+                        plus_lt.clone(),
+                        Applicability::MaybeIncorrect,
+                    );
+                }
+                name if name.ident().to_string() != lifetime_name => {
+                    // With this check we avoid suggesting redundant bounds. This
+                    // would happen if there are nested impl/dyn traits and only
+                    // one of them has the bound we'd suggest already there, like
+                    // in `impl Foo<X = dyn Bar> + '_`.
+                    if let Some(explicit_static) = &explicit_static {
                         err.span_suggestion_verbose(
                             lt.span,
                             &format!("{} trait object's {}", consider, explicit_static),
                             lifetime_name.clone(),
                             Applicability::MaybeIncorrect,
                         );
+                    }
+                    if let Some((param_span, param_ty)) = param.clone() {
                         err.span_suggestion_verbose(
-                            param.param_ty_span,
+                            param_span,
                             add_static_bound,
-                            param.param_ty.to_string(),
+                            param_ty,
                             Applicability::MaybeIncorrect,
                         );
                     }
-                    _ => {}
-                },
+                }
                 _ => {}
-            }
+            },
+            _ => {}
         }
-        err.emit();
-        Some(ErrorReported)
     }
+}
 
+impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
     fn get_impl_ident_and_self_ty_from_trait(
         &self,
         def_id: DefId,
diff --git a/src/test/ui/impl-trait/hidden-lifetimes.stderr b/src/test/ui/impl-trait/hidden-lifetimes.stderr
index ba192aa4ab2..60d3409a8ac 100644
--- a/src/test/ui/impl-trait/hidden-lifetimes.stderr
+++ b/src/test/ui/impl-trait/hidden-lifetimes.stderr
@@ -5,6 +5,11 @@ LL | fn hide_ref<'a, 'b, T: 'static>(x: &'a mut &'b T) -> impl Swap + 'a {
    |                 --                                   ^^^^^^^^^^^^^^
    |                 |
    |                 hidden type `&'a mut &'b T` captures the lifetime `'b` as defined here
+   |
+help: to declare that the `impl Trait` captures 'b, you can add an explicit `'b` lifetime bound
+   |
+LL | fn hide_ref<'a, 'b, T: 'static>(x: &'a mut &'b T) -> impl Swap + 'a + 'b {
+   |                                                                     ++++
 
 error[E0700]: hidden type for `impl Trait` captures lifetime that does not appear in bounds
   --> $DIR/hidden-lifetimes.rs:45:70
@@ -13,6 +18,11 @@ LL | fn hide_rc_refcell<'a, 'b: 'a, T: 'static>(x: Rc<RefCell<&'b T>>) -> impl S
    |                        --                                            ^^^^^^^^^^^^^^
    |                        |
    |                        hidden type `Rc<RefCell<&'b T>>` captures the lifetime `'b` as defined here
+   |
+help: to declare that the `impl Trait` captures 'b, you can add an explicit `'b` lifetime bound
+   |
+LL | fn hide_rc_refcell<'a, 'b: 'a, T: 'static>(x: Rc<RefCell<&'b T>>) -> impl Swap + 'a + 'b {
+   |                                                                                     ++++
 
 error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/impl-trait/region-escape-via-bound.stderr b/src/test/ui/impl-trait/region-escape-via-bound.stderr
index b9359455278..9dc2ea5bc82 100644
--- a/src/test/ui/impl-trait/region-escape-via-bound.stderr
+++ b/src/test/ui/impl-trait/region-escape-via-bound.stderr
@@ -6,6 +6,11 @@ LL | fn foo(x: Cell<&'x u32>) -> impl Trait<'y>
 LL |
 LL | where 'x: 'y
    |       -- hidden type `Cell<&'x u32>` captures the lifetime `'x` as defined here
+   |
+help: to declare that the `impl Trait` captures 'x, you can add an explicit `'x` lifetime bound
+   |
+LL | fn foo(x: Cell<&'x u32>) -> impl Trait<'y> + 'x
+   |                                            ++++
 
 error: aborting due to previous error