Refactor region naming for control of diagnostics.

Previously, region naming would always highlight the source of the
region name it found. Now, region naming returns the name as part
of a larger structure that encodes the source of the region naming
such that a region name can be optionally added to the diagnostic.

2 files changed, 168 insertions, 125 deletions

diff --git a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs
index d287116029a..31d7c7c631e 100644
--- a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs
+++ b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs
@@ -339,10 +339,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         );
 
         let counter = &mut 1;
-        let fr_name = self.give_region_a_name(
-            infcx, mir, mir_def_id, fr, counter, &mut diag);
+        let fr_name = self.give_region_a_name(infcx, mir, mir_def_id, fr, counter);
+        fr_name.highlight_region_name(&mut diag);
         let outlived_fr_name = self.give_region_a_name(
-            infcx, mir, mir_def_id, outlived_fr, counter, &mut diag);
+            infcx, mir, mir_def_id, outlived_fr, counter);
+        outlived_fr_name.highlight_region_name(&mut diag);
 
         let mir_def_name = if infcx.tcx.is_closure(mir_def_id) { "closure" } else { "function" };
 
@@ -430,10 +431,12 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                     // Otherwise, we should suggest adding a constraint on the return type.
                     let span = infcx.tcx.def_span(*did);
                     if let Ok(snippet) = infcx.tcx.sess.source_map().span_to_snippet(span) {
-                        let suggestable_fr_name = match fr_name {
-                            RegionName::Named(name) => format!("{}", name),
-                            RegionName::Synthesized(_) => "'_".to_string(),
+                        let suggestable_fr_name = if fr_name.was_named() {
+                            format!("{}", fr_name)
+                        } else {
+                            "'_".to_string()
                         };
+
                         diag.span_suggestion_with_applicability(
                             span,
                             &format!(
diff --git a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs
index f704ee61c88..875348ac819 100644
--- a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs
+++ b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs
@@ -25,29 +25,102 @@ use syntax::symbol::keywords;
 use syntax_pos::Span;
 use syntax_pos::symbol::InternedString;
 
-/// Name of a region used in error reporting. Variants denote the source of the region name -
-/// whether it was synthesized for the error message and therefore should not be used in
-/// suggestions; or whether it was found from the region.
 #[derive(Debug)]
-pub(crate) enum RegionName {
-    Named(InternedString),
-    Synthesized(InternedString),
+crate struct RegionName {
+    name: InternedString,
+    source: RegionNameSource,
+}
+
+#[derive(Debug)]
+crate enum RegionNameSource {
+    NamedEarlyBoundRegion(Span),
+    NamedFreeRegion(Span),
+    Static,
+    SynthesizedFreeEnvRegion(Span, String),
+    CannotMatchHirTy(Span, String),
+    MatchedHirTy(Span),
+    MatchedAdtAndSegment(Span),
+    AnonRegionFromUpvar(Span, String),
+    AnonRegionFromOutput(Span, String, String),
 }
 
 impl RegionName {
-    fn as_interned_string(&self) -> &InternedString {
-        match self {
-            RegionName::Named(name) | RegionName::Synthesized(name) => name,
+    #[allow(dead_code)]
+    crate fn was_named(&self) -> bool {
+        match self.source {
+            RegionNameSource::NamedEarlyBoundRegion(..) |
+            RegionNameSource::NamedFreeRegion(..) |
+            RegionNameSource::Static => true,
+            RegionNameSource::SynthesizedFreeEnvRegion(..) |
+            RegionNameSource::CannotMatchHirTy(..) |
+            RegionNameSource::MatchedHirTy(..) |
+            RegionNameSource::MatchedAdtAndSegment(..) |
+            RegionNameSource::AnonRegionFromUpvar(..) |
+            RegionNameSource::AnonRegionFromOutput(..) => false,
+        }
+    }
+
+    #[allow(dead_code)]
+    crate fn was_synthesized(&self) -> bool {
+        !self.was_named()
+    }
+
+    #[allow(dead_code)]
+    crate fn name(&self) -> &InternedString {
+        &self.name
+    }
+
+    crate fn highlight_region_name(
+        &self,
+        diag: &mut DiagnosticBuilder<'_>
+    ) {
+        match &self.source {
+            RegionNameSource::NamedFreeRegion(span) |
+            RegionNameSource::NamedEarlyBoundRegion(span) => {
+                diag.span_label(
+                    *span,
+                    format!("lifetime `{}` defined here", self),
+                );
+            },
+            RegionNameSource::SynthesizedFreeEnvRegion(span, note) => {
+                diag.span_label(
+                    *span,
+                    format!("lifetime `{}` represents this closure's body", self),
+                );
+                diag.note(&note);
+            },
+            RegionNameSource::CannotMatchHirTy(span, type_name) => {
+                diag.span_label(*span, format!("has type `{}`", type_name));
+            },
+            RegionNameSource::MatchedHirTy(span) => {
+                diag.span_label(
+                    *span,
+                    format!("let's call the lifetime of this reference `{}`", self),
+                );
+            },
+            RegionNameSource::MatchedAdtAndSegment(span) => {
+                diag.span_label(*span, format!("let's call this `{}`", self));
+            },
+            RegionNameSource::AnonRegionFromUpvar(span, upvar_name) => {
+                diag.span_label(
+                    *span,
+                    format!("lifetime `{}` appears in the type of `{}`", self, upvar_name),
+                );
+            },
+            RegionNameSource::AnonRegionFromOutput(span, mir_description, type_name) => {
+                diag.span_label(
+                    *span,
+                    format!("return type{} is {}", mir_description, type_name),
+                );
+            },
+            RegionNameSource::Static => {},
         }
     }
 }
 
 impl Display for RegionName {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match self {
-            RegionName::Named(name) | RegionName::Synthesized(name) =>
-                write!(f, "{}", name),
-        }
+        write!(f, "{}", self.name)
     }
 }
 
@@ -84,26 +157,25 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         mir_def_id: DefId,
         fr: RegionVid,
         counter: &mut usize,
-        diag: &mut DiagnosticBuilder,
     ) -> RegionName {
         debug!("give_region_a_name(fr={:?}, counter={})", fr, counter);
 
         assert!(self.universal_regions.is_universal_region(fr));
 
-        let value = self.give_name_from_error_region(infcx.tcx, mir_def_id, fr, counter, diag)
+        let value = self.give_name_from_error_region(infcx.tcx, mir_def_id, fr, counter)
             .or_else(|| {
                 self.give_name_if_anonymous_region_appears_in_arguments(
-                    infcx, mir, mir_def_id, fr, counter, diag,
+                    infcx, mir, mir_def_id, fr, counter,
                 )
             })
             .or_else(|| {
                 self.give_name_if_anonymous_region_appears_in_upvars(
-                    infcx.tcx, mir, fr, counter, diag,
+                    infcx.tcx, mir, fr, counter,
                 )
             })
             .or_else(|| {
                 self.give_name_if_anonymous_region_appears_in_output(
-                    infcx, mir, mir_def_id, fr, counter, diag,
+                    infcx, mir, mir_def_id, fr, counter,
                 )
             })
             .unwrap_or_else(|| span_bug!(mir.span, "can't make a name for free region {:?}", fr));
@@ -122,7 +194,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         mir_def_id: DefId,
         fr: RegionVid,
         counter: &mut usize,
-        diag: &mut DiagnosticBuilder<'_>,
     ) -> Option<RegionName> {
         let error_region = self.to_error_region(fr)?;
 
@@ -130,23 +201,28 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         match error_region {
             ty::ReEarlyBound(ebr) => {
                 if ebr.has_name() {
-                    let name = RegionName::Named(ebr.name);
-                    self.highlight_named_span(tcx, error_region, &name, diag);
-                    Some(name)
+                    let span = self.get_named_span(tcx, error_region, &ebr.name);
+                    Some(RegionName {
+                        name: ebr.name,
+                        source: RegionNameSource::NamedEarlyBoundRegion(span)
+                    })
                 } else {
                     None
                 }
             }
 
-            ty::ReStatic => Some(RegionName::Named(
-                keywords::StaticLifetime.name().as_interned_str()
-            )),
+            ty::ReStatic => Some(RegionName {
+                name: keywords::StaticLifetime.name().as_interned_str(),
+                source: RegionNameSource::Static
+            }),
 
             ty::ReFree(free_region) => match free_region.bound_region {
                 ty::BoundRegion::BrNamed(_, name) => {
-                    let name = RegionName::Named(name);
-                    self.highlight_named_span(tcx, error_region, &name, diag);
-                    Some(name)
+                    let span = self.get_named_span(tcx, error_region, &name);
+                    Some(RegionName {
+                        name,
+                        source: RegionNameSource::NamedFreeRegion(span),
+                    })
                 },
 
                 ty::BoundRegion::BrEnv => {
@@ -162,13 +238,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                             bug!("Closure is not defined by a closure expr");
                         };
                         let region_name = self.synthesize_region_name(counter);
-                        diag.span_label(
-                            args_span,
-                            format!(
-                                "lifetime `{}` represents this closure's body",
-                                region_name
-                            ),
-                        );
 
                         let closure_kind_ty = substs.closure_kind_ty(def_id, tcx);
                         let note = match closure_kind_ty.to_opt_closure_kind() {
@@ -186,9 +255,13 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                             None => bug!("Closure kind not inferred in borrow check"),
                         };
 
-                        diag.note(note);
-
-                        Some(region_name)
+                        Some(RegionName {
+                            name: region_name,
+                            source: RegionNameSource::SynthesizedFreeEnvRegion(
+                                args_span,
+                                note.to_string()
+                            ),
+                        })
                     } else {
                         // Can't have BrEnv in functions, constants or generators.
                         bug!("BrEnv outside of closure.");
@@ -209,27 +282,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         }
     }
 
-    /// Get the span of a named region.
-    pub(super) fn get_span_of_named_region(
-        &self,
-        tcx: TyCtxt<'_, '_, 'tcx>,
-        error_region: &RegionKind,
-        name: &RegionName,
-    ) -> Span {
-        let scope = error_region.free_region_binding_scope(tcx);
-        let node = tcx.hir.as_local_node_id(scope).unwrap_or(DUMMY_NODE_ID);
-
-        let span = tcx.sess.source_map().def_span(tcx.hir.span(node));
-        if let Some(param) = tcx.hir.get_generics(scope).and_then(|generics| {
-            generics.get_named(name.as_interned_string())
-        }) {
-            param.span
-        } else {
-            span
-        }
-    }
-
-    /// Highlight a named span to provide context for error messages that
+    /// Get a span of a named region to provide context for error messages that
     /// mention that span, for example:
     ///
     /// ```
@@ -243,19 +296,24 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     ///  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ argument requires that `'b` must
     ///  |                                                         outlive `'a`
     /// ```
-    fn highlight_named_span(
+    fn get_named_span(
         &self,
         tcx: TyCtxt<'_, '_, 'tcx>,
         error_region: &RegionKind,
-        name: &RegionName,
-        diag: &mut DiagnosticBuilder<'_>,
-    ) {
-        let span = self.get_span_of_named_region(tcx, error_region, name);
+        name: &InternedString,
+    ) -> Span {
+        let scope = error_region.free_region_binding_scope(tcx);
+        let node = tcx.hir.as_local_node_id(scope).unwrap_or(DUMMY_NODE_ID);
 
-        diag.span_label(
-            span,
-            format!("lifetime `{}` defined here", name),
-        );
+        let span = tcx.sess.source_map().def_span(tcx.hir.span(node));
+        if let Some(param) = tcx.hir
+            .get_generics(scope)
+            .and_then(|generics| generics.get_named(name))
+        {
+            param.span
+        } else {
+            span
+        }
     }
 
     /// Find an argument that contains `fr` and label it with a fully
@@ -273,7 +331,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         mir_def_id: DefId,
         fr: RegionVid,
         counter: &mut usize,
-        diag: &mut DiagnosticBuilder<'_>,
     ) -> Option<RegionName> {
         let implicit_inputs = self.universal_regions.defining_ty.implicit_inputs();
         let argument_index = self.get_argument_index_for_region(infcx.tcx, fr)?;
@@ -288,12 +345,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             arg_ty,
             argument_index,
             counter,
-            diag,
         ) {
             return Some(region_name);
         }
 
-        self.give_name_if_we_cannot_match_hir_ty(infcx, mir, fr, arg_ty, counter, diag)
+        self.give_name_if_we_cannot_match_hir_ty(infcx, mir, fr, arg_ty, counter)
     }
 
     fn give_name_if_we_can_match_hir_ty_from_argument(
@@ -305,7 +361,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         argument_ty: Ty<'tcx>,
         argument_index: usize,
         counter: &mut usize,
-        diag: &mut DiagnosticBuilder<'_>,
     ) -> Option<RegionName> {
         let mir_node_id = infcx.tcx.hir.as_local_node_id(mir_def_id)?;
         let fn_decl = infcx.tcx.hir.fn_decl(mir_node_id)?;
@@ -320,7 +375,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                 needle_fr,
                 argument_ty,
                 counter,
-                diag,
             ),
 
             _ => self.give_name_if_we_can_match_hir_ty(
@@ -329,7 +383,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                 argument_ty,
                 argument_hir_ty,
                 counter,
-                diag,
             ),
         }
     }
@@ -352,7 +405,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         needle_fr: RegionVid,
         argument_ty: Ty<'tcx>,
         counter: &mut usize,
-        diag: &mut DiagnosticBuilder<'_>,
     ) -> Option<RegionName> {
         let type_name = with_highlight_region(needle_fr, *counter, || {
             infcx.extract_type_name(&argument_ty)
@@ -366,12 +418,14 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             // Only add a label if we can confirm that a region was labelled.
             let argument_index = self.get_argument_index_for_region(infcx.tcx, needle_fr)?;
             let (_, span) = self.get_argument_name_and_span_for_region(mir, argument_index);
-            diag.span_label(span, format!("has type `{}`", type_name));
 
-            // This counter value will already have been used, so this function will increment it
-            // so the next value will be used next and return the region name that would have been
-            // used.
-            Some(self.synthesize_region_name(counter))
+            Some(RegionName {
+                // This counter value will already have been used, so this function will increment it
+                // so the next value will be used next and return the region name that would have been
+                // used.
+                name: self.synthesize_region_name(counter),
+                source: RegionNameSource::CannotMatchHirTy(span, type_name),
+            })
         } else {
             None
         };
@@ -407,7 +461,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         argument_ty: Ty<'tcx>,
         argument_hir_ty: &hir::Ty,
         counter: &mut usize,
-        diag: &mut DiagnosticBuilder<'_>,
     ) -> Option<RegionName> {
         let search_stack: &mut Vec<(Ty<'tcx>, &hir::Ty)> = &mut Vec::new();
 
@@ -432,15 +485,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                         let source_map = tcx.sess.source_map();
                         let ampersand_span = source_map.start_point(hir_ty.span);
 
-                        diag.span_label(
-                            ampersand_span,
-                            format!(
-                                "let's call the lifetime of this reference `{}`",
-                                region_name
-                            ),
-                        );
-
-                        return Some(region_name);
+                        return Some(RegionName {
+                            name: region_name,
+                            source: RegionNameSource::MatchedHirTy(ampersand_span),
+                        });
                     }
 
                     // Otherwise, let's descend into the referent types.
@@ -464,7 +512,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                                 needle_fr,
                                 last_segment,
                                 counter,
-                                diag,
                                 search_stack,
                             ) {
                                 return Some(name);
@@ -509,7 +556,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         needle_fr: RegionVid,
         last_segment: &'hir hir::PathSegment,
         counter: &mut usize,
-        diag: &mut DiagnosticBuilder<'_>,
         search_stack: &mut Vec<(Ty<'tcx>, &'hir hir::Ty)>,
     ) -> Option<RegionName> {
         // Did the user give explicit arguments? (e.g., `Foo<..>`)
@@ -521,11 +567,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             | hir::LifetimeName::Underscore => {
                 let region_name = self.synthesize_region_name(counter);
                 let ampersand_span = lifetime.span;
-                diag.span_label(
-                    ampersand_span,
-                    format!("let's call this `{}`", region_name)
-                );
-                return Some(region_name);
+                return Some(RegionName {
+                    name: region_name,
+                    source: RegionNameSource::MatchedAdtAndSegment(ampersand_span),
+                });
             }
 
             hir::LifetimeName::Implicit => {
@@ -600,22 +645,16 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         mir: &Mir<'tcx>,
         fr: RegionVid,
         counter: &mut usize,
-        diag: &mut DiagnosticBuilder<'_>,
     ) -> Option<RegionName> {
         let upvar_index = self.get_upvar_index_for_region(tcx, fr)?;
         let (upvar_name, upvar_span) =
             self.get_upvar_name_and_span_for_region(tcx, mir, upvar_index);
         let region_name = self.synthesize_region_name(counter);
 
-        diag.span_label(
-            upvar_span,
-            format!(
-                "lifetime `{}` appears in the type of `{}`",
-                region_name, upvar_name
-            ),
-        );
-
-        Some(region_name)
+        Some(RegionName {
+            name: region_name,
+            source: RegionNameSource::AnonRegionFromUpvar(upvar_span, upvar_name.to_string()),
+        })
     }
 
     /// Check for arguments appearing in the (closure) return type. It
@@ -629,7 +668,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         mir_def_id: DefId,
         fr: RegionVid,
         counter: &mut usize,
-        diag: &mut DiagnosticBuilder<'_>,
     ) -> Option<RegionName> {
         let tcx = infcx.tcx;
 
@@ -666,23 +704,25 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                 (mir.span, "")
             };
 
-        diag.span_label(
-            return_span,
-            format!("return type{} is {}", mir_description, type_name),
-        );
-
-        // This counter value will already have been used, so this function will increment it
-        // so the next value will be used next and return the region name that would have been
-        // used.
-        Some(self.synthesize_region_name(counter))
+        Some(RegionName {
+            // This counter value will already have been used, so this function will increment it
+            // so the next value will be used next and return the region name that would have been
+            // used.
+            name: self.synthesize_region_name(counter),
+            source: RegionNameSource::AnonRegionFromOutput(
+                return_span,
+                mir_description.to_string(),
+                type_name
+            ),
+        })
     }
 
     /// Create a synthetic region named `'1`, incrementing the
     /// counter.
-    fn synthesize_region_name(&self, counter: &mut usize) -> RegionName {
+    fn synthesize_region_name(&self, counter: &mut usize) -> InternedString {
         let c = *counter;
         *counter += 1;
 
-        RegionName::Synthesized(Name::intern(&format!("'{:?}", c)).as_interned_str())
+        Name::intern(&format!("'{:?}", c)).as_interned_str()
     }
 }