Rollup merge of rust-lang#58347 - matthewjasper:closure-bounds-fixes, r=pnkfelix

Closure bounds fixes

* Ensures that "nice region errors" are buffered so that they are sorted and migrated correctly.
* Propagates fewer constraints for closures (cc rust-lang#58178)
* Propagate constraints from closures more precisely (rust-lang#58127)

Closes rust-lang#58127

r? @nikomatsakis

Author: Centril

src/librustc/infer/error_reporting/nice_region_error/mod.rs

@@ -1,9 +1,10 @@
 use crate::infer::InferCtxt;
 use crate::infer::lexical_region_resolve::RegionResolutionError;
 use crate::infer::lexical_region_resolve::RegionResolutionError::*;
-use syntax::source_map::Span;
 use crate::ty::{self, TyCtxt};
 use crate::util::common::ErrorReported;
+use errors::DiagnosticBuilder;
+use syntax::source_map::Span;
 
 mod different_lifetimes;
 mod find_anon_type;
@@ -59,7 +60,7 @@ impl<'cx, 'gcx, 'tcx> NiceRegionError<'cx, 'gcx, 'tcx> {
         self.infcx.tcx
     }
 
-    pub fn try_report_from_nll(&self) -> Option<ErrorReported> {
+    pub fn try_report_from_nll(&self) -> Option<DiagnosticBuilder<'cx>> {
         // Due to the improved diagnostics returned by the MIR borrow checker, only a subset of
         // the nice region errors are required when running under the MIR borrow checker.
         self.try_report_named_anon_conflict()
@@ -68,6 +69,7 @@ impl<'cx, 'gcx, 'tcx> NiceRegionError<'cx, 'gcx, 'tcx> {
 
     pub fn try_report(&self) -> Option<ErrorReported> {
         self.try_report_from_nll()
+            .map(|mut diag| { diag.emit(); ErrorReported })
             .or_else(|| self.try_report_anon_anon_conflict())
             .or_else(|| self.try_report_outlives_closure())
             .or_else(|| self.try_report_static_impl_trait())

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

@@ -2,13 +2,12 @@
 //! where one region is named and the other is anonymous.
 use crate::infer::error_reporting::nice_region_error::NiceRegionError;
 use crate::ty;
-use crate::util::common::ErrorReported;
-use errors::Applicability;
+use errors::{Applicability, DiagnosticBuilder};
 
 impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> {
     /// When given a `ConcreteFailure` for a function with arguments containing a named region and
     /// an anonymous region, emit an descriptive diagnostic error.
-    pub(super) fn try_report_named_anon_conflict(&self) -> Option<ErrorReported> {
+    pub(super) fn try_report_named_anon_conflict(&self) -> Option<DiagnosticBuilder<'a>> {
         let (span, sub, sup) = self.get_regions();
 
         debug!(
@@ -96,21 +95,23 @@ impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> {
             ("parameter type".to_owned(), "type".to_owned())
         };
 
-        struct_span_err!(
+        let mut diag = struct_span_err!(
             self.tcx().sess,
             span,
             E0621,
             "explicit lifetime required in {}",
             error_var
-        ).span_suggestion(
-            new_ty_span,
-            &format!("add explicit lifetime `{}` to {}", named, span_label_var),
-            new_ty.to_string(),
-            Applicability::Unspecified,
-        )
-        .span_label(span, format!("lifetime `{}` required", named))
-        .emit();
-        return Some(ErrorReported);
+        );
+
+        diag.span_suggestion(
+                new_ty_span,
+                &format!("add explicit lifetime `{}` to {}", named, span_label_var),
+                new_ty.to_string(),
+                Applicability::Unspecified,
+            )
+            .span_label(span, format!("lifetime `{}` required", named));
+
+        Some(diag)
     }
 
     // This method returns whether the given Region is Named

src/librustc/infer/error_reporting/nice_region_error/placeholder_error.rs

@@ -8,13 +8,12 @@ use crate::traits::{ObligationCause, ObligationCauseCode};
 use crate::ty;
 use crate::ty::error::ExpectedFound;
 use crate::ty::subst::Substs;
-use crate::util::common::ErrorReported;
 use crate::util::ppaux::RegionHighlightMode;
 
 impl NiceRegionError<'me, 'gcx, 'tcx> {
     /// When given a `ConcreteFailure` for a function with arguments containing a named region and
     /// an anonymous region, emit a descriptive diagnostic error.
-    pub(super) fn try_report_placeholder_conflict(&self) -> Option<ErrorReported> {
+    pub(super) fn try_report_placeholder_conflict(&self) -> Option<DiagnosticBuilder<'me>> {
         match &self.error {
             ///////////////////////////////////////////////////////////////////////////
             // NB. The ordering of cases in this match is very
@@ -178,7 +177,7 @@ impl NiceRegionError<'me, 'gcx, 'tcx> {
         trait_def_id: DefId,
         expected_substs: &'tcx Substs<'tcx>,
         actual_substs: &'tcx Substs<'tcx>,
-    ) -> ErrorReported {
+    ) -> DiagnosticBuilder<'me> {
         debug!(
             "try_report_placeholders_trait(\
              vid={:?}, \
@@ -295,8 +294,7 @@ impl NiceRegionError<'me, 'gcx, 'tcx> {
             any_self_ty_has_vid,
         );
 
-        err.emit();
-        ErrorReported
+        err
     }
 
     /// Add notes with details about the expected and actual trait refs, with attention to cases

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

@@ -244,7 +244,8 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         if let (Some(f), Some(o)) = (self.to_error_region(fr), self.to_error_region(outlived_fr)) {
             let tables = infcx.tcx.typeck_tables_of(mir_def_id);
             let nice = NiceRegionError::new_from_span(infcx, span, o, f, Some(tables));
-            if let Some(_error_reported) = nice.try_report_from_nll() {
+            if let Some(diag) = nice.try_report_from_nll() {
+                diag.buffer(errors_buffer);
                 return;
             }
         }

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

@@ -15,7 +15,7 @@ use rustc::mir::{
     ConstraintCategory, Local, Location, Mir,
 };
 use rustc::ty::{self, RegionVid, Ty, TyCtxt, TypeFoldable};
-use rustc::util::common;
+use rustc::util::common::{self, ErrorReported};
 use rustc_data_structures::bit_set::BitSet;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::graph::scc::Sccs;
@@ -763,20 +763,26 @@ impl<'tcx> RegionInferenceContext<'tcx> {
 
             debug!("try_promote_type_test: ur={:?}", ur);
 
-            let non_local_ub = self.universal_region_relations.non_local_upper_bound(ur);
+            let non_local_ub = self.universal_region_relations.non_local_upper_bounds(&ur);
             debug!("try_promote_type_test: non_local_ub={:?}", non_local_ub);
 
-            assert!(self.universal_regions.is_universal_region(non_local_ub));
-            assert!(!self.universal_regions.is_local_free_region(non_local_ub));
-
-            let requirement = ClosureOutlivesRequirement {
-                subject,
-                outlived_free_region: non_local_ub,
-                blame_span: locations.span(mir),
-                category: ConstraintCategory::Boring,
-            };
-            debug!("try_promote_type_test: pushing {:#?}", requirement);
-            propagated_outlives_requirements.push(requirement);
+            // This is slightly too conservative. To show T: '1, given `'2: '1`
+            // and `'3: '1` we only need to prove that T: '2 *or* T: '3, but to
+            // avoid potential non-determinism we approximate this by requiring
+            // T: '1 and T: '2.
+            for &upper_bound in non_local_ub {
+                debug_assert!(self.universal_regions.is_universal_region(upper_bound));
+                debug_assert!(!self.universal_regions.is_local_free_region(upper_bound));
+
+                let requirement = ClosureOutlivesRequirement {
+                    subject,
+                    outlived_free_region: upper_bound,
+                    blame_span: locations.span(mir),
+                    category: ConstraintCategory::Boring,
+                };
+                debug!("try_promote_type_test: pushing {:#?}", requirement);
+                propagated_outlives_requirements.push(requirement);
+            }
         }
         true
     }
@@ -1157,63 +1163,109 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                 .is_none()
         );
 
+        // Only check all of the relations for the main representative of each
+        // SCC, otherwise just check that we outlive said representative. This
+        // reduces the number of redundant relations propagated out of
+        // closures.
+        // Note that the representative will be a universal region if there is
+        // one in this SCC, so we will always check the representative here.
+        let representative = self.scc_representatives[longer_fr_scc];
+        if representative != longer_fr {
+            self.check_universal_region_relation(
+                longer_fr,
+                representative,
+                infcx,
+                mir,
+                mir_def_id,
+                propagated_outlives_requirements,
+                errors_buffer,
+            );
+            return;
+        }
+
         // Find every region `o` such that `fr: o`
         // (because `fr` includes `end(o)`).
         for shorter_fr in self.scc_values.universal_regions_outlived_by(longer_fr_scc) {
-            // If it is known that `fr: o`, carry on.
-            if self.universal_region_relations
-                .outlives(longer_fr, shorter_fr)
-            {
-                continue;
+            if let Some(ErrorReported) = self.check_universal_region_relation(
+                longer_fr,
+                shorter_fr,
+                infcx,
+                mir,
+                mir_def_id,
+                propagated_outlives_requirements,
+                errors_buffer,
+            ) {
+                // continuing to iterate just reports more errors than necessary
+                return;
             }
+        }
+    }
 
-            debug!(
-                "check_universal_region: fr={:?} does not outlive shorter_fr={:?}",
-                longer_fr, shorter_fr,
-            );
+    fn check_universal_region_relation(
+        &self,
+        longer_fr: RegionVid,
+        shorter_fr: RegionVid,
+        infcx: &InferCtxt<'_, 'gcx, 'tcx>,
+        mir: &Mir<'tcx>,
+        mir_def_id: DefId,
+        propagated_outlives_requirements: &mut Option<&mut Vec<ClosureOutlivesRequirement<'gcx>>>,
+        errors_buffer: &mut Vec<Diagnostic>,
+    ) -> Option<ErrorReported> {
+        // If it is known that `fr: o`, carry on.
+        if self.universal_region_relations
+            .outlives(longer_fr, shorter_fr)
+        {
+            return None;
+        }
 
-            let blame_span_category = self.find_outlives_blame_span(mir, longer_fr, shorter_fr);
-
-            if let Some(propagated_outlives_requirements) = propagated_outlives_requirements {
-                // Shrink `fr` until we find a non-local region (if we do).
-                // We'll call that `fr-` -- it's ever so slightly smaller than `fr`.
-                if let Some(fr_minus) = self.universal_region_relations
-                    .non_local_lower_bound(longer_fr)
-                {
-                    debug!("check_universal_region: fr_minus={:?}", fr_minus);
-
-                    // Grow `shorter_fr` until we find a non-local
-                    // region. (We always will.)  We'll call that
-                    // `shorter_fr+` -- it's ever so slightly larger than
-                    // `fr`.
-                    let shorter_fr_plus = self.universal_region_relations
-                        .non_local_upper_bound(shorter_fr);
-                    debug!(
-                        "check_universal_region: shorter_fr_plus={:?}",
-                        shorter_fr_plus
-                    );
+        debug!(
+            "check_universal_region_relation: fr={:?} does not outlive shorter_fr={:?}",
+            longer_fr, shorter_fr,
+        );
 
+        if let Some(propagated_outlives_requirements) = propagated_outlives_requirements {
+            // Shrink `longer_fr` until we find a non-local region (if we do).
+            // We'll call it `fr-` -- it's ever so slightly smaller than
+            // `longer_fr`.
+
+            if let Some(fr_minus) = self
+                .universal_region_relations
+                .non_local_lower_bound(longer_fr)
+            {
+                debug!("check_universal_region: fr_minus={:?}", fr_minus);
+
+                let blame_span_category = self.find_outlives_blame_span(mir, longer_fr, shorter_fr);
+
+                // Grow `shorter_fr` until we find some non-local regions. (We
+                // always will.)  We'll call them `shorter_fr+` -- they're ever
+                // so slightly larger than `shorter_fr`.
+                let shorter_fr_plus = self.universal_region_relations
+                    .non_local_upper_bounds(&shorter_fr);
+                debug!(
+                    "check_universal_region: shorter_fr_plus={:?}",
+                    shorter_fr_plus
+                );
+                for &&fr in &shorter_fr_plus {
                     // Push the constraint `fr-: shorter_fr+`
                     propagated_outlives_requirements.push(ClosureOutlivesRequirement {
                         subject: ClosureOutlivesSubject::Region(fr_minus),
-                        outlived_free_region: shorter_fr_plus,
+                        outlived_free_region: fr,
                         blame_span: blame_span_category.1,
                         category: blame_span_category.0,
                     });
-                    continue;
                 }
+                return None;
             }
-
-            // If we are not in a context where we can propagate
-            // errors, or we could not shrink `fr` to something
-            // smaller, then just report an error.
-            //
-            // Note: in this case, we use the unapproximated regions
-            // to report the error. This gives better error messages
-            // in some cases.
-            self.report_error(mir, infcx, mir_def_id, longer_fr, shorter_fr, errors_buffer);
-            return; // continuing to iterate just reports more errors than necessary
         }
+
+        // If we are not in a context where we can't propagate errors, or we
+        // could not shrink `fr` to something smaller, then just report an
+        // error.
+        //
+        // Note: in this case, we use the unapproximated regions to report the
+        // error. This gives better error messages in some cases.
+        self.report_error(mir, infcx, mir_def_id, longer_fr, shorter_fr, errors_buffer);
+        Some(ErrorReported)
     }
 
     fn check_bound_universal_region<'gcx>(