Rollup merge of rust-lang#73496 - estebank:opaque-missing-lts-in-fn-3…

…, r=nikomatsakis

Account for multiple impl/dyn Trait in return type when suggesting `'_`

Make `impl` and `dyn` Trait lifetime suggestions a bit more resilient.

Follow up to rust-lang#72804.

r? @nikomatsakis

src/librustc_infer/infer/error_reporting/nice_region_error/find_anon_type.rs

@@ -26,7 +26,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
         &self,
         region: Region<'tcx>,
         br: &ty::BoundRegion,
-    ) -> Option<(&hir::Ty<'_>, &hir::FnDecl<'_>)> {
+    ) -> Option<(&hir::Ty<'tcx>, &hir::FnDecl<'tcx>)> {
         if let Some(anon_reg) = self.tcx().is_suitable_region(region) {
             let def_id = anon_reg.def_id;
             if let Some(def_id) = def_id.as_local() {

src/librustc_infer/infer/error_reporting/nice_region_error/named_anon_conflict.rs

@@ -2,7 +2,8 @@
 //! where one region is named and the other is anonymous.
 use crate::infer::error_reporting::nice_region_error::NiceRegionError;
 use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder};
-use rustc_hir::{FnRetTy, TyKind};
+use rustc_hir::intravisit::Visitor;
+use rustc_hir::FnRetTy;
 use rustc_middle::ty;
 
 impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
@@ -80,16 +81,21 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
             }
 
             if let FnRetTy::Return(ty) = &fndecl.output {
-                let mut v = ty::TraitObjectVisitor(vec![]);
-                rustc_hir::intravisit::walk_ty(&mut v, ty);
+                let mut v = ty::TraitObjectVisitor(vec![], self.tcx().hir());
+                v.visit_ty(ty);
 
                 debug!("try_report_named_anon_conflict: ret ty {:?}", ty);
                 if sub == &ty::ReStatic
-                    && (matches!(ty.kind, TyKind::OpaqueDef(_, _)) || v.0.len() == 1)
+                    && v.0
+                        .into_iter()
+                        .filter(|t| t.span.desugaring_kind().is_none())
+                        .next()
+                        .is_some()
                 {
+                    // If the failure is due to a `'static` requirement coming from a `dyn` or
+                    // `impl` Trait that *isn't* caused by `async fn` desugaring, handle this case
+                    // better in `static_impl_trait`.
                     debug!("try_report_named_anon_conflict: impl Trait + 'static");
-                    // This is an `impl Trait` or `dyn Trait` return that evaluates de need of
-                    // `'static`. We handle this case better in `static_impl_trait`.
                     return None;
                 }
             }

src/librustc_infer/infer/error_reporting/nice_region_error/static_impl_trait.rs

@@ -26,8 +26,11 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
                 );
                 let anon_reg_sup = self.tcx().is_suitable_region(sup_r)?;
                 debug!("try_report_static_impl_trait: anon_reg_sup={:?}", anon_reg_sup);
-                let fn_return = self.tcx().return_type_impl_or_dyn_trait(anon_reg_sup.def_id)?;
-                debug!("try_report_static_impl_trait: fn_return={:?}", fn_return);
+                let fn_returns = self.tcx().return_type_impl_or_dyn_traits(anon_reg_sup.def_id);
+                if fn_returns.is_empty() {
+                    return None;
+                }
+                debug!("try_report_static_impl_trait: fn_return={:?}", fn_returns);
                 if **sub_r == RegionKind::ReStatic {
                     let sp = var_origin.span();
                     let return_sp = sub_origin.span();
@@ -98,25 +101,26 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
                         );
                     }
 
-                    // only apply this suggestion onto functions with
-                    // explicit non-desugar'able return.
-                    if fn_return.span.desugaring_kind().is_none() {
-                        // 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_info.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);
+                    // 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_info.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 = self.tcx().hir().item(item_id.id);
@@ -143,7 +147,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
                                     err.span_suggestion_verbose(
                                         span,
                                         &format!("{} `impl Trait`'s {}", consider, explicit_static),
-                                        lifetime_name,
+                                        lifetime_name.clone(),
                                         Applicability::MaybeIncorrect,
                                     );
                                     err.span_suggestion_verbose(
@@ -152,6 +156,19 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
                                         param_info.param_ty.to_string(),
                                         Applicability::MaybeIncorrect,
                                     );
+                                } else if let Some(_) = opaque
+                                    .bounds
+                                    .iter()
+                                    .filter_map(|arg| match arg {
+                                        GenericBound::Outlives(Lifetime { name, span, .. })
+                                            if name.ident().to_string() == lifetime_name =>
+                                        {
+                                            Some(*span)
+                                        }
+                                        _ => None,
+                                    })
+                                    .next()
+                                {
                                 } else {
                                     err.span_suggestion_verbose(
                                         fn_return.span.shrink_to_hi(),
@@ -161,10 +178,10 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
                                             captures = captures,
                                             explicit = explicit,
                                         ),
-                                        plus_lt,
+                                        plus_lt.clone(),
                                         Applicability::MaybeIncorrect,
                                     );
-                                };
+                                }
                             }
                             TyKind::TraitObject(_, lt) => match lt.name {
                                 LifetimeName::ImplicitObjectLifetimeDefault => {
@@ -176,15 +193,19 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
                                             captures = captures,
                                             explicit = explicit,
                                         ),
-                                        plus_lt,
+                                        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> + '_`.
                                     err.span_suggestion_verbose(
                                         lt.span,
                                         &format!("{} trait object's {}", consider, explicit_static),
-                                        lifetime_name,
+                                        lifetime_name.clone(),
                                         Applicability::MaybeIncorrect,
                                     );
                                     err.span_suggestion_verbose(
@@ -194,6 +215,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
                                         Applicability::MaybeIncorrect,
                                     );
                                 }
+                                _ => {}
                             },
                             _ => {}
                         }

src/librustc_middle/ty/context.rs

@@ -38,6 +38,7 @@ use rustc_hir as hir;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, LocalDefId, LOCAL_CRATE};
 use rustc_hir::definitions::{DefPathHash, Definitions};
+use rustc_hir::intravisit::Visitor;
 use rustc_hir::lang_items::{self, PanicLocationLangItem};
 use rustc_hir::{HirId, ItemKind, ItemLocalId, ItemLocalMap, ItemLocalSet, Node, TraitCandidate};
 use rustc_index::vec::{Idx, IndexVec};
@@ -1427,10 +1428,8 @@ impl<'tcx> TyCtxt<'tcx> {
         })
     }
 
-    pub fn return_type_impl_or_dyn_trait(
-        &self,
-        scope_def_id: DefId,
-    ) -> Option<&'tcx hir::Ty<'tcx>> {
+    /// Given a `DefId` for an `fn`, return all the `dyn` and `impl` traits in its return type.
+    pub fn return_type_impl_or_dyn_traits(&self, scope_def_id: DefId) -> Vec<&'tcx hir::Ty<'tcx>> {
         let hir_id = self.hir().as_local_hir_id(scope_def_id.expect_local());
         let hir_output = match self.hir().get(hir_id) {
             Node::Item(hir::Item {
@@ -1466,30 +1465,12 @@ impl<'tcx> TyCtxt<'tcx> {
                     ),
                 ..
             }) => ty,
-            _ => return None,
+            _ => return vec![],
         };
 
-        let ret_ty = self.type_of(scope_def_id);
-        match ret_ty.kind {
-            ty::FnDef(_, _) => {
-                let sig = ret_ty.fn_sig(*self);
-                let output = self.erase_late_bound_regions(&sig.output());
-                if output.is_impl_trait() {
-                    let fn_decl = self.hir().fn_decl_by_hir_id(hir_id).unwrap();
-                    if let hir::FnRetTy::Return(ty) = fn_decl.output {
-                        return Some(ty);
-                    }
-                } else {
-                    let mut v = TraitObjectVisitor(vec![]);
-                    rustc_hir::intravisit::walk_ty(&mut v, hir_output);
-                    if v.0.len() == 1 {
-                        return Some(v.0[0]);
-                    }
-                }
-                None
-            }
-            _ => None,
-        }
+        let mut v = TraitObjectVisitor(vec![], self.hir());
+        v.visit_ty(hir_output);
+        v.0
     }
 
     pub fn return_type_impl_trait(&self, scope_def_id: DefId) -> Option<(Ty<'tcx>, Span)> {

src/librustc_middle/ty/diagnostics.rs

@@ -236,7 +236,9 @@ pub fn suggest_constraining_type_param(
     }
 }
 
-pub struct TraitObjectVisitor<'tcx>(pub Vec<&'tcx hir::Ty<'tcx>>);
+/// Collect al types that have an implicit `'static` obligation that we could suggest `'_` for.
+pub struct TraitObjectVisitor<'tcx>(pub Vec<&'tcx hir::Ty<'tcx>>, pub crate::hir::map::Map<'tcx>);
+
 impl<'v> hir::intravisit::Visitor<'v> for TraitObjectVisitor<'v> {
     type Map = rustc_hir::intravisit::ErasedMap<'v>;
 
@@ -245,15 +247,24 @@ impl<'v> hir::intravisit::Visitor<'v> for TraitObjectVisitor<'v> {
     }
 
     fn visit_ty(&mut self, ty: &'v hir::Ty<'v>) {
-        if let hir::TyKind::TraitObject(
-            _,
-            hir::Lifetime {
-                name: hir::LifetimeName::ImplicitObjectLifetimeDefault | hir::LifetimeName::Static,
-                ..
-            },
-        ) = ty.kind
-        {
-            self.0.push(ty);
+        match ty.kind {
+            hir::TyKind::TraitObject(
+                _,
+                hir::Lifetime {
+                    name:
+                        hir::LifetimeName::ImplicitObjectLifetimeDefault | hir::LifetimeName::Static,
+                    ..
+                },
+            ) => {
+                self.0.push(ty);
+            }
+            hir::TyKind::OpaqueDef(item_id, _) => {
+                self.0.push(ty);
+                let item = self.1.expect_item(item_id.id);
+                hir::intravisit::walk_item(self, item);
+            }
+            _ => {}
         }
+        hir::intravisit::walk_ty(self, ty);
     }
 }

src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr

@@ -53,7 +53,15 @@ LL | fn foo<'a>(x: &i32) -> impl Copy + 'a { x }
    |               help: add explicit lifetime `'a` to the type of `x`: `&'a i32`
 
 error: lifetime may not live long enough
-  --> $DIR/must_outlive_least_region_or_bound.rs:33:69
+  --> $DIR/must_outlive_least_region_or_bound.rs:30:24
+   |
+LL | fn elided5(x: &i32) -> (Box<dyn Debug>, impl Debug) { (Box::new(x), x) }
+   |               -        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ opaque type requires that `'1` must outlive `'static`
+   |               |
+   |               let's call the lifetime of this reference `'1`
+
+error: lifetime may not live long enough
+  --> $DIR/must_outlive_least_region_or_bound.rs:37:69
    |
 LL | fn with_bound<'a>(x: &'a i32) -> impl LifetimeTrait<'a> + 'static { x }
    |               -- lifetime `'a` defined here                         ^ returning this value requires that `'a` must outlive `'static`
@@ -62,7 +70,7 @@ LL | fn with_bound<'a>(x: &'a i32) -> impl LifetimeTrait<'a> + 'static { x }
    = help: consider replacing `'a` with `'static`
 
 error: lifetime may not live long enough
-  --> $DIR/must_outlive_least_region_or_bound.rs:38:61
+  --> $DIR/must_outlive_least_region_or_bound.rs:42:61
    |
 LL | fn move_lifetime_into_fn<'a, 'b>(x: &'a u32, y: &'b u32) -> impl Fn(&'a u32) {
    |                          --  -- lifetime `'b` defined here  ^^^^^^^^^^^^^^^^ opaque type requires that `'b` must outlive `'a`
@@ -72,14 +80,14 @@ LL | fn move_lifetime_into_fn<'a, 'b>(x: &'a u32, y: &'b u32) -> impl Fn(&'a u32
    = help: consider adding the following bound: `'b: 'a`
 
 error[E0310]: the parameter type `T` may not live long enough
-  --> $DIR/must_outlive_least_region_or_bound.rs:43:51
+  --> $DIR/must_outlive_least_region_or_bound.rs:47:51
    |
 LL | fn ty_param_wont_outlive_static<T:Debug>(x: T) -> impl Debug + 'static {
    |                                                   ^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider adding an explicit lifetime bound `T: 'static`...
 
-error: aborting due to 8 previous errors
+error: aborting due to 9 previous errors
 
 Some errors have detailed explanations: E0310, E0621.
 For more information about an error, try `rustc --explain E0310`.

src/test/ui/impl-trait/must_outlive_least_region_or_bound.rs

@@ -27,6 +27,10 @@ fn elided4(x: &i32) -> Box<dyn Debug + 'static> { Box::new(x) }
 fn explicit4<'a>(x: &'a i32) -> Box<dyn Debug + 'static> { Box::new(x) }
 //~^ ERROR cannot infer an appropriate lifetime
 
+fn elided5(x: &i32) -> (Box<dyn Debug>, impl Debug) { (Box::new(x), x) }
+//~^ ERROR cannot infer an appropriate lifetime
+//~| ERROR cannot infer an appropriate lifetime
+
 trait LifetimeTrait<'a> {}
 impl<'a> LifetimeTrait<'a> for &'a i32 {}