Auto merge of #58281 - mark-i-m:synthesis, r=estebank

Add outlives suggestions for some lifetime errors

This PR implements suggestion diagnostics for some lifetime mismatch errors. When the borrow checker finds that some lifetime 'a doesn't outlive some other lifetime 'b that it should outlive, then in addition to the current lifetime error, we also emit a suggestion for how to fix the problem by adding a bound:

- If a and b are normal named regions, suggest to add the bound `'a: 'b`
- If b is static, suggest to replace a with static
- If b also needs to outlive a, they must be the same, so suggest unifying  them

We start with a simpler implementation that avoids diagnostic regression or implementation complexity:
- We only makes suggestions for lifetimes the user can already name (eg not closure regions or elided regions)
- For now, we only emit a help note, not an actually suggestion because it is significantly easier.

Finally, there is one hack: it seems that implicit regions in async fn are given the name '_ incorrectly. To avoid suggesting '_: 'x, we simply filter out such lifetimes by name.

For more info, see this internals thread:

https://internals.rust-lang.org/t/mechanical-suggestions-for-some-borrow-checker-errors/9049/3

TL;DR Make suggestions to add a `where 'a: 'b` constraint for some lifetime errors. Details are in the paper linked from the internals thread above.

r? @estebank

TODO
- [x] Clean up code
- [x] Only make idiomatic suggestions
     - [x] don't suggest naming `&'a self`
     - [x] rather than `'a: 'static`, suggest replacing `'a` with `'static`
     - [x] rather than `'a: 'b, 'b: 'a`, suggest replacing `'a` with `'b` or vice versa
- [x] Performance (maybe need a perf run when this is closer to the finish line?)
     - perf run was clean...
     - EDIT: perf run seems to only check non-error performance... How do we check that error performance didn't regress?
- [x] Needs ui tests
- [x] Integrate the `help` message into the main lifetime `error`

src/librustc_errors/diagnostic.rs

@@ -6,7 +6,7 @@ use crate::Applicability;
 use crate::Level;
 use crate::snippet::Style;
 use std::fmt;
-use syntax_pos::{MultiSpan, Span};
+use syntax_pos::{MultiSpan, Span, DUMMY_SP};
 
 #[must_use]
 #[derive(Clone, Debug, PartialEq, Hash, RustcEncodable, RustcDecodable)]
@@ -17,6 +17,11 @@ pub struct Diagnostic {
     pub span: MultiSpan,
     pub children: Vec<SubDiagnostic>,
     pub suggestions: Vec<CodeSuggestion>,
+
+    /// This is not used for highlighting or rendering any error message.  Rather, it can be used
+    /// as a sort key to sort a buffer of diagnostics.  By default, it is the primary span of
+    /// `span` if there is one.  Otherwise, it is `DUMMY_SP`.
+    pub sort_span: Span,
 }
 
 #[derive(Clone, Debug, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)]
@@ -87,6 +92,7 @@ impl Diagnostic {
             span: MultiSpan::new(),
             children: vec![],
             suggestions: vec![],
+            sort_span: DUMMY_SP,
         }
     }
 
@@ -118,6 +124,11 @@ impl Diagnostic {
         self.level == Level::Cancelled
     }
 
+    /// Set the sorting span.
+    pub fn set_sort_span(&mut self, sp: Span) {
+        self.sort_span = sp;
+    }
+
     /// Adds a span/label to be included in the resulting snippet.
     /// This label will be shown together with the original span/label used when creating the
     /// diagnostic, *not* a span added by one of the `span_*` methods.
@@ -457,6 +468,9 @@ impl Diagnostic {
 
     pub fn set_span<S: Into<MultiSpan>>(&mut self, sp: S) -> &mut Self {
         self.span = sp.into();
+        if let Some(span) = self.span.primary_span() {
+            self.sort_span = span;
+        }
         self
     }
 

src/librustc_errors/lib.rs

@@ -575,6 +575,11 @@ impl Handler {
         DiagnosticBuilder::new(self, Level::Fatal, msg)
     }
 
+    /// Construct a builder at the `Help` level with the `msg`.
+    pub fn struct_help(&self, msg: &str) -> DiagnosticBuilder<'_> {
+        DiagnosticBuilder::new(self, Level::Help, msg)
+    }
+
     pub fn span_fatal(&self, span: impl Into<MultiSpan>, msg: &str) -> FatalError {
         self.emit_diag_at_span(Diagnostic::new(Fatal, msg), span);
         FatalError

src/librustc_mir/borrow_check/mod.rs

@@ -367,7 +367,7 @@ fn do_mir_borrowck<'a, 'tcx>(
     }
 
     if !mbcx.errors_buffer.is_empty() {
-        mbcx.errors_buffer.sort_by_key(|diag| diag.span.primary_span());
+        mbcx.errors_buffer.sort_by_key(|diag| diag.sort_span);
 
         for diag in mbcx.errors_buffer.drain(..) {
             mbcx.infcx.tcx.sess.diagnostic().emit_diagnostic(&diag);

src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs

@@ -18,6 +18,10 @@ use syntax::errors::Applicability;
 use syntax::symbol::kw;
 use syntax_pos::Span;
 
+use self::outlives_suggestion::OutlivesSuggestionBuilder;
+
+pub mod outlives_suggestion;
+
 mod region_name;
 mod var_name;
 
@@ -56,7 +60,6 @@ enum Trace {
 /// Various pieces of state used when reporting borrow checker errors.
 pub struct ErrorReportingCtx<'a, 'b, 'tcx> {
     /// The region inference context used for borrow chekcing this MIR body.
-    #[allow(dead_code)] // FIXME(mark-i-m): used by outlives suggestions
     region_infcx: &'b RegionInferenceContext<'tcx>,
 
     /// The inference context used for type checking.
@@ -370,6 +373,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         fr: RegionVid,
         fr_origin: NLLRegionVariableOrigin,
         outlived_fr: RegionVid,
+        outlives_suggestion: &mut OutlivesSuggestionBuilder,
         renctx: &mut RegionErrorNamingCtx,
     ) -> DiagnosticBuilder<'a> {
         debug!("report_error(fr={:?}, outlived_fr={:?})", fr, outlived_fr);
@@ -415,9 +419,22 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                 self.report_fnmut_error(&errctx, &errci, renctx)
             }
             (ConstraintCategory::Assignment, true, false)
-            | (ConstraintCategory::CallArgument, true, false) =>
-                self.report_escaping_data_error(&errctx, &errci, renctx),
-            _ => self.report_general_error(&errctx, &errci, renctx),
+            | (ConstraintCategory::CallArgument, true, false) => {
+                let mut db = self.report_escaping_data_error(&errctx, &errci, renctx);
+
+                outlives_suggestion.intermediate_suggestion(&errctx, &errci, renctx, &mut db);
+                outlives_suggestion.collect_constraint(fr, outlived_fr);
+
+                db
+            }
+            _ => {
+                let mut db = self.report_general_error(&errctx, &errci, renctx);
+
+                outlives_suggestion.intermediate_suggestion(&errctx, &errci, renctx, &mut db);
+                outlives_suggestion.collect_constraint(fr, outlived_fr);
+
+                db
+            }
         }
     }