Auto merge of #105805 - yanchen4791:issue-105227-fix, r=estebank

Suggest adding named lifetime when the return contains value borrowed from more than one lifetimes of function inputs

fix for #105227.

The problem: The suggestion of adding an explicit `'_` lifetime bound is **incorrect** when the function's return type contains a value which could be borrowed from more than one lifetimes of the function's inputs. Instead, a named lifetime parameter can be introduced in such a case.

The solution: Checking the number of elided lifetimes in the function signature. If more than one lifetimes found in the function inputs when the suggestion of adding explicit `'_` lifetime, change it to using named lifetime parameter `'a` instead.

Author: bors

compiler/rustc_borrowck/src/diagnostics/region_errors.rs

@@ -192,6 +192,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
         // buffered in the `MirBorrowckCtxt`.
 
         let mut outlives_suggestion = OutlivesSuggestionBuilder::default();
+        let mut last_unexpected_hidden_region: Option<(Span, Ty<'_>, ty::OpaqueTypeKey<'tcx>)> =
+            None;
 
         for nll_error in nll_errors.into_iter() {
             match nll_error {
@@ -234,13 +236,19 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                     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);
-                    self.buffer_error(unexpected_hidden_region_diagnostic(
+                    let mut diag = unexpected_hidden_region_diagnostic(
                         self.infcx.tcx,
                         span,
                         named_ty,
                         named_region,
                         named_key,
-                    ));
+                    );
+                    if last_unexpected_hidden_region != Some((span, named_ty, named_key)) {
+                        self.buffer_error(diag);
+                        last_unexpected_hidden_region = Some((span, named_ty, named_key));
+                    } else {
+                        diag.delay_as_bug();
+                    }
                 }
 
                 RegionErrorKind::BoundUniversalRegionError {
@@ -730,6 +738,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                 Some(arg),
                 captures,
                 Some((param.param_ty_span, param.param_ty.to_string())),
+                self.infcx.tcx.is_suitable_region(f).map(|r| r.def_id),
             );
         }
     }

compiler/rustc_infer/src/infer/error_reporting/mod.rs

@@ -303,6 +303,7 @@ pub fn unexpected_hidden_region_diagnostic<'tcx>(
                     None,
                     format!("captures `{}`", hidden_region),
                     None,
+                    Some(reg_info.def_id),
                 )
             }
         }

compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs

@@ -8,13 +8,17 @@ use rustc_data_structures::fx::FxIndexSet;
 use rustc_errors::{struct_span_err, Applicability, Diagnostic, ErrorGuaranteed, MultiSpan};
 use rustc_hir::def_id::DefId;
 use rustc_hir::intravisit::{walk_ty, Visitor};
-use rustc_hir::{self as hir, GenericBound, Item, ItemKind, Lifetime, LifetimeName, Node, TyKind};
+use rustc_hir::{
+    self as hir, GenericBound, GenericParamKind, Item, ItemKind, Lifetime, LifetimeName, Node,
+    TyKind,
+};
 use rustc_middle::ty::{
     self, AssocItemContainer, StaticLifetimeVisitor, Ty, TyCtxt, TypeSuperVisitable, TypeVisitor,
 };
 use rustc_span::symbol::Ident;
 use rustc_span::Span;
 
+use rustc_span::def_id::LocalDefId;
 use std::ops::ControlFlow;
 
 impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
@@ -268,6 +272,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
             Some(arg),
             captures,
             Some((param.param_ty_span, param.param_ty.to_string())),
+            Some(anon_reg_sup.def_id),
         );
 
         let reported = err.emit();
@@ -283,6 +288,7 @@ pub fn suggest_new_region_bound(
     arg: Option<String>,
     captures: String,
     param: Option<(Span, String)>,
+    scope_def_id: Option<LocalDefId>,
 ) {
     debug!("try_report_static_impl_trait: fn_return={:?}", fn_returns);
     // FIXME: account for the need of parens in `&(dyn Trait + '_)`
@@ -340,12 +346,69 @@ pub fn suggest_new_region_bound(
                     _ => false,
                 }) {
                 } else {
-                    err.span_suggestion_verbose(
-                        fn_return.span.shrink_to_hi(),
-                        &format!("{declare} `{ty}` {captures}, {explicit}",),
-                        &plus_lt,
-                        Applicability::MaybeIncorrect,
-                    );
+                    // get a lifetime name of existing named lifetimes if any
+                    let existing_lt_name = if let Some(id) = scope_def_id
+                        && let Some(generics) = tcx.hir().get_generics(id)
+                        && let named_lifetimes = generics
+                        .params
+                        .iter()
+                        .filter(|p| matches!(p.kind, GenericParamKind::Lifetime { kind: hir::LifetimeParamKind::Explicit }))
+                        .map(|p| { if let hir::ParamName::Plain(name) = p.name {Some(name.to_string())} else {None}})
+                        .filter(|n| ! matches!(n, None))
+                        .collect::<Vec<_>>()
+                        && named_lifetimes.len() > 0 {
+                        named_lifetimes[0].clone()
+                    } else {
+                        None
+                    };
+                    let name = if let Some(name) = &existing_lt_name {
+                        format!("{}", name)
+                    } else {
+                        format!("'a")
+                    };
+                    // if there are more than one elided lifetimes in inputs, the explicit `'_` lifetime cannot be used.
+                    // introducing a new lifetime `'a` or making use of one from existing named lifetimes if any
+                    if let Some(id) = scope_def_id
+                        && let Some(generics) = tcx.hir().get_generics(id)
+                        && let mut spans_suggs = generics
+                            .params
+                            .iter()
+                            .filter(|p| p.is_elided_lifetime())
+                            .map(|p|
+                                  if p.span.hi() - p.span.lo() == rustc_span::BytePos(1) { // Ampersand (elided without '_)
+                                      (p.span.shrink_to_hi(),format!("{name} "))
+                                  } else { // Underscore (elided with '_)
+                                      (p.span, format!("{name}"))
+                                  }
+                            )
+                            .collect::<Vec<_>>()
+                        && spans_suggs.len() > 1
+                    {
+                        let use_lt =
+                        if existing_lt_name == None {
+                            spans_suggs.push((generics.span.shrink_to_hi(), format!("<{name}>")));
+                            format!("you can introduce a named lifetime parameter `{name}`")
+                        } else {
+                            // make use the existing named lifetime
+                            format!("you can use the named lifetime parameter `{name}`")
+                        };
+                        spans_suggs
+                            .push((fn_return.span.shrink_to_hi(), format!(" + {name} ")));
+                        err.multipart_suggestion_verbose(
+                            &format!(
+                                "{declare} `{ty}` {captures}, {use_lt}",
+                            ),
+                            spans_suggs,
+                            Applicability::MaybeIncorrect,
+                        );
+                    } else {
+                        err.span_suggestion_verbose(
+                            fn_return.span.shrink_to_hi(),
+                            &format!("{declare} `{ty}` {captures}, {explicit}",),
+                            &plus_lt,
+                            Applicability::MaybeIncorrect,
+                        );
+                    }
                 }
             }
             TyKind::TraitObject(_, lt, _) => {

src/test/ui/impl-trait/static-return-lifetime-infered.rs

@@ -6,12 +6,10 @@ impl A {
     fn iter_values_anon(&self) -> impl Iterator<Item=u32> {
         self.x.iter().map(|a| a.0)
         //~^ ERROR: captures lifetime that does not appear in bounds
-        //~| ERROR: captures lifetime that does not appear in bounds
     }
     fn iter_values<'a>(&'a self) -> impl Iterator<Item=u32> {
         self.x.iter().map(|a| a.0)
         //~^ ERROR: captures lifetime that does not appear in bounds
-        //~| ERROR: captures lifetime that does not appear in bounds
     }
 }
 

src/test/ui/impl-trait/static-return-lifetime-infered.stderr

@@ -12,36 +12,10 @@ LL |     fn iter_values_anon(&self) -> impl Iterator<Item=u32> + '_ {
    |                                                           ++++
 
 error[E0700]: hidden type for `impl Iterator<Item = u32>` captures lifetime that does not appear in bounds
-  --> $DIR/static-return-lifetime-infered.rs:7:9
-   |
-LL |     fn iter_values_anon(&self) -> impl Iterator<Item=u32> {
-   |                         ----- hidden type `Map<std::slice::Iter<'_, (u32, u32)>, [closure@$DIR/static-return-lifetime-infered.rs:7:27: 7:30]>` captures the anonymous lifetime defined here
-LL |         self.x.iter().map(|a| a.0)
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-help: to declare that `impl Iterator<Item = u32>` captures `'_`, you can add an explicit `'_` lifetime bound
-   |
-LL |     fn iter_values_anon(&self) -> impl Iterator<Item=u32> + '_ {
-   |                                                           ++++
-
-error[E0700]: hidden type for `impl Iterator<Item = u32>` captures lifetime that does not appear in bounds
-  --> $DIR/static-return-lifetime-infered.rs:12:9
-   |
-LL |     fn iter_values<'a>(&'a self) -> impl Iterator<Item=u32> {
-   |                    -- hidden type `Map<std::slice::Iter<'a, (u32, u32)>, [closure@$DIR/static-return-lifetime-infered.rs:12:27: 12:30]>` captures the lifetime `'a` as defined here
-LL |         self.x.iter().map(|a| a.0)
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-help: to declare that `impl Iterator<Item = u32>` captures `'a`, you can add an explicit `'a` lifetime bound
-   |
-LL |     fn iter_values<'a>(&'a self) -> impl Iterator<Item=u32> + 'a {
-   |                                                             ++++
-
-error[E0700]: hidden type for `impl Iterator<Item = u32>` captures lifetime that does not appear in bounds
-  --> $DIR/static-return-lifetime-infered.rs:12:9
+  --> $DIR/static-return-lifetime-infered.rs:11:9
    |
 LL |     fn iter_values<'a>(&'a self) -> impl Iterator<Item=u32> {
-   |                    -- hidden type `Map<std::slice::Iter<'a, (u32, u32)>, [closure@$DIR/static-return-lifetime-infered.rs:12:27: 12:30]>` captures the lifetime `'a` as defined here
+   |                    -- hidden type `Map<std::slice::Iter<'a, (u32, u32)>, [closure@$DIR/static-return-lifetime-infered.rs:11:27: 11:30]>` captures the lifetime `'a` as defined here
 LL |         self.x.iter().map(|a| a.0)
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
@@ -50,6 +24,6 @@ help: to declare that `impl Iterator<Item = u32>` captures `'a`, you can add an
 LL |     fn iter_values<'a>(&'a self) -> impl Iterator<Item=u32> + 'a {
    |                                                             ++++
 
-error: aborting due to 4 previous errors
+error: aborting due to 2 previous errors
 
 For more information about this error, try `rustc --explain E0700`.

src/test/ui/lifetimes/issue-105227.fixed

@@ -0,0 +1,26 @@
+// Regression test for issue #105227.
+
+// run-rustfix
+#![allow(warnings)]
+fn chars0<'a>(v :(&'a  str, &'a str)) -> impl Iterator<Item = char> + 'a  {
+//~^ HELP to declare that `impl Iterator<Item = char>` captures `'_`, you can introduce a named lifetime parameter `'a`
+    v.0.chars().chain(v.1.chars())
+    //~^ ERROR hidden type for `impl Iterator<Item = char>` captures lifetime that does not appear in bounds
+}
+
+fn chars1<'a>(v0 : &'a  str, v1 : &'a str) -> impl Iterator<Item = char> + 'a  {
+//~^ HELP to declare that `impl Iterator<Item = char>` captures `'_`, you can introduce a named lifetime parameter `'a`
+    v0.chars().chain(v1.chars())
+    //~^ ERROR hidden type for `impl Iterator<Item = char>` captures lifetime that does not appear in bound
+}
+
+fn chars2<'b>(v0 : &'b str, v1 : &'b str, v2 : &'b str) ->
+//~^ HELP to declare that `impl Iterator<Item = char>` captures `'_`, you can use the named lifetime parameter `'b`
+    (impl Iterator<Item = char> + 'b , &'b str)
+{
+    (v0.chars().chain(v1.chars()), v2)
+    //~^ ERROR hidden type for `impl Iterator<Item = char>` captures lifetime that does not appear in bound
+}
+
+fn main() {
+}

src/test/ui/lifetimes/issue-105227.rs

@@ -0,0 +1,26 @@
+// Regression test for issue #105227.
+
+// run-rustfix
+#![allow(warnings)]
+fn chars0(v :(& str, &str)) -> impl Iterator<Item = char> {
+//~^ HELP to declare that `impl Iterator<Item = char>` captures `'_`, you can introduce a named lifetime parameter `'a`
+    v.0.chars().chain(v.1.chars())
+    //~^ ERROR hidden type for `impl Iterator<Item = char>` captures lifetime that does not appear in bounds
+}
+
+fn chars1(v0 : & str, v1 : &str) -> impl Iterator<Item = char> {
+//~^ HELP to declare that `impl Iterator<Item = char>` captures `'_`, you can introduce a named lifetime parameter `'a`
+    v0.chars().chain(v1.chars())
+    //~^ ERROR hidden type for `impl Iterator<Item = char>` captures lifetime that does not appear in bound
+}
+
+fn chars2<'b>(v0 : &str, v1 : &'_ str, v2 : &'b str) ->
+//~^ HELP to declare that `impl Iterator<Item = char>` captures `'_`, you can use the named lifetime parameter `'b`
+    (impl Iterator<Item = char>, &'b str)
+{
+    (v0.chars().chain(v1.chars()), v2)
+    //~^ ERROR hidden type for `impl Iterator<Item = char>` captures lifetime that does not appear in bound
+}
+
+fn main() {
+}

src/test/ui/lifetimes/issue-105227.stderr

@@ -0,0 +1,47 @@
+error[E0700]: hidden type for `impl Iterator<Item = char>` captures lifetime that does not appear in bounds
+  --> $DIR/issue-105227.rs:7:5
+   |
+LL | fn chars0(v :(& str, &str)) -> impl Iterator<Item = char> {
+   |               ----- hidden type `std::iter::Chain<Chars<'_>, Chars<'_>>` captures the anonymous lifetime defined here
+LL |
+LL |     v.0.chars().chain(v.1.chars())
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: to declare that `impl Iterator<Item = char>` captures `'_`, you can introduce a named lifetime parameter `'a`
+   |
+LL | fn chars0<'a>(v :(&'a  str, &'a str)) -> impl Iterator<Item = char> + 'a  {
+   |          ++++      ++        ++                                     ++++
+
+error[E0700]: hidden type for `impl Iterator<Item = char>` captures lifetime that does not appear in bounds
+  --> $DIR/issue-105227.rs:13:5
+   |
+LL | fn chars1(v0 : & str, v1 : &str) -> impl Iterator<Item = char> {
+   |                ----- hidden type `std::iter::Chain<Chars<'_>, Chars<'_>>` captures the anonymous lifetime defined here
+LL |
+LL |     v0.chars().chain(v1.chars())
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: to declare that `impl Iterator<Item = char>` captures `'_`, you can introduce a named lifetime parameter `'a`
+   |
+LL | fn chars1<'a>(v0 : &'a  str, v1 : &'a str) -> impl Iterator<Item = char> + 'a  {
+   |          ++++       ++             ++                                    ++++
+
+error[E0700]: hidden type for `impl Iterator<Item = char>` captures lifetime that does not appear in bounds
+  --> $DIR/issue-105227.rs:21:5
+   |
+LL | fn chars2<'b>(v0 : &str, v1 : &'_ str, v2 : &'b str) ->
+   |                    ---- hidden type `std::iter::Chain<Chars<'_>, Chars<'_>>` captures the anonymous lifetime defined here
+...
+LL |     (v0.chars().chain(v1.chars()), v2)
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: to declare that `impl Iterator<Item = char>` captures `'_`, you can use the named lifetime parameter `'b`
+   |
+LL ~ fn chars2<'b>(v0 : &'b str, v1 : &'b str, v2 : &'b str) ->
+LL |
+LL ~     (impl Iterator<Item = char> + 'b , &'b str)
+   |
+
+error: aborting due to 3 previous errors
+
+For more information about this error, try `rustc --explain E0700`.