Auto merge of #89555 - oli-obk:nll_member_constraint_diag, r=estebank

Remove textual span from diagnostic string

This is an unnecessary repetition, as the diagnostic prints the span anyway in the source path right below the message.

I further removed the identification of the node, as that does not give any new information in any of the cases that are changed in tests.

EDIT: also inserted a suggestion that other diagnostics were already emitting

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`.

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

@@ -116,7 +116,7 @@ pub(super) fn note_and_explain_region(
     emit_msg_span(err, prefix, description, span, suffix);
 }
 
-pub(super) fn note_and_explain_free_region(
+fn explain_free_region(
     tcx: TyCtxt<'tcx>,
     err: &mut DiagnosticBuilder<'_>,
     prefix: &str,
@@ -125,7 +125,7 @@ pub(super) fn note_and_explain_free_region(
 ) {
     let (description, span) = msg_span_from_free_region(tcx, region, None);
 
-    emit_msg_span(err, prefix, description, span, suffix);
+    label_msg_span(err, prefix, description, span, suffix);
 }
 
 fn msg_span_from_free_region(
@@ -135,7 +135,8 @@ fn msg_span_from_free_region(
 ) -> (String, Option<Span>) {
     match *region {
         ty::ReEarlyBound(_) | ty::ReFree(_) => {
-            msg_span_from_early_bound_and_free_regions(tcx, region)
+            let (msg, span) = msg_span_from_early_bound_and_free_regions(tcx, region);
+            (msg, Some(span))
         }
         ty::ReStatic => ("the static lifetime".to_owned(), alt_span),
         ty::ReEmpty(ty::UniverseIndex::ROOT) => ("an empty lifetime".to_owned(), alt_span),
@@ -147,28 +148,20 @@ fn msg_span_from_free_region(
 fn msg_span_from_early_bound_and_free_regions(
     tcx: TyCtxt<'tcx>,
     region: ty::Region<'tcx>,
-) -> (String, Option<Span>) {
+) -> (String, Span) {
     let sm = tcx.sess.source_map();
 
     let scope = region.free_region_binding_scope(tcx);
     let node = tcx.hir().local_def_id_to_hir_id(scope.expect_local());
-    let tag = match tcx.hir().find(node) {
-        Some(Node::Block(_) | Node::Expr(_)) => "body",
-        Some(Node::Item(it)) => item_scope_tag(&it),
-        Some(Node::TraitItem(it)) => trait_item_scope_tag(&it),
-        Some(Node::ImplItem(it)) => impl_item_scope_tag(&it),
-        Some(Node::ForeignItem(it)) => foreign_item_scope_tag(&it),
-        _ => unreachable!(),
-    };
-    let (prefix, span) = match *region {
+    match *region {
         ty::ReEarlyBound(ref br) => {
             let mut sp = sm.guess_head_span(tcx.hir().span(node));
             if let Some(param) =
                 tcx.hir().get_generics(scope).and_then(|generics| generics.get_named(br.name))
             {
                 sp = param.span;
             }
-            (format!("the lifetime `{}` as defined on", br.name), sp)
+            (format!("the lifetime `{}` as defined here", br.name), sp)
         }
         ty::ReFree(ty::FreeRegion {
             bound_region: ty::BoundRegionKind::BrNamed(_, name), ..
@@ -179,28 +172,26 @@ fn msg_span_from_early_bound_and_free_regions(
             {
                 sp = param.span;
             }
-            (format!("the lifetime `{}` as defined on", name), sp)
+            (format!("the lifetime `{}` as defined here", name), sp)
         }
         ty::ReFree(ref fr) => match fr.bound_region {
             ty::BrAnon(idx) => {
                 if let Some((ty, _)) = find_anon_type(tcx, region, &fr.bound_region) {
-                    ("the anonymous lifetime defined on".to_string(), ty.span)
+                    ("the anonymous lifetime defined here".to_string(), ty.span)
                 } else {
                     (
-                        format!("the anonymous lifetime #{} defined on", idx + 1),
+                        format!("the anonymous lifetime #{} defined here", idx + 1),
                         tcx.hir().span(node),
                     )
                 }
             }
             _ => (
-                format!("the lifetime `{}` as defined on", region),
+                format!("the lifetime `{}` as defined here", region),
                 sm.guess_head_span(tcx.hir().span(node)),
             ),
         },
         _ => bug!(),
-    };
-    let (msg, opt_span) = explain_span(tcx, tag, span);
-    (format!("{} {}", prefix, msg), opt_span)
+    }
 }
 
 fn emit_msg_span(
@@ -219,44 +210,22 @@ fn emit_msg_span(
     }
 }
 
-fn item_scope_tag(item: &hir::Item<'_>) -> &'static str {
-    match item.kind {
-        hir::ItemKind::Impl { .. } => "impl",
-        hir::ItemKind::Struct(..) => "struct",
-        hir::ItemKind::Union(..) => "union",
-        hir::ItemKind::Enum(..) => "enum",
-        hir::ItemKind::Trait(..) => "trait",
-        hir::ItemKind::Fn(..) => "function body",
-        _ => "item",
-    }
-}
-
-fn trait_item_scope_tag(item: &hir::TraitItem<'_>) -> &'static str {
-    match item.kind {
-        hir::TraitItemKind::Fn(..) => "method body",
-        hir::TraitItemKind::Const(..) | hir::TraitItemKind::Type(..) => "associated item",
-    }
-}
-
-fn impl_item_scope_tag(item: &hir::ImplItem<'_>) -> &'static str {
-    match item.kind {
-        hir::ImplItemKind::Fn(..) => "method body",
-        hir::ImplItemKind::Const(..) | hir::ImplItemKind::TyAlias(..) => "associated item",
-    }
-}
+fn label_msg_span(
+    err: &mut DiagnosticBuilder<'_>,
+    prefix: &str,
+    description: String,
+    span: Option<Span>,
+    suffix: &str,
+) {
+    let message = format!("{}{}{}", prefix, description, suffix);
 
-fn foreign_item_scope_tag(item: &hir::ForeignItem<'_>) -> &'static str {
-    match item.kind {
-        hir::ForeignItemKind::Fn(..) => "method body",
-        hir::ForeignItemKind::Static(..) | hir::ForeignItemKind::Type => "associated item",
+    if let Some(span) = span {
+        err.span_label(span, &message);
+    } else {
+        err.note(&message);
     }
 }
 
-fn explain_span(tcx: TyCtxt<'tcx>, heading: &str, span: Span) -> (String, Option<Span>) {
-    let lo = tcx.sess.source_map().lookup_char_pos(span.lo());
-    (format!("the {} at {}:{}", heading, lo.line, lo.col.to_usize() + 1), Some(span))
-}
-
 pub fn unexpected_hidden_region_diagnostic(
     tcx: TyCtxt<'tcx>,
     span: Span,
@@ -291,13 +260,25 @@ pub fn unexpected_hidden_region_diagnostic(
             //
             // (*) if not, the `tainted_by_errors` field would be set to
             // `Some(ErrorReported)` in any case, so we wouldn't be here at all.
-            note_and_explain_free_region(
+            explain_free_region(
                 tcx,
                 &mut err,
                 &format!("hidden type `{}` captures ", hidden_ty),
                 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

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()