More separation of error reporting from region inference

Author: mark-i-m

src/librustc_mir/borrow_check/diagnostics/region_errors.rs

@@ -1,8 +1,7 @@
 //! Error reporting machinery for lifetime errors.
 
 use rustc::infer::{
-    error_reporting::nice_region_error::NiceRegionError, region_constraints::GenericKind,
-    InferCtxt, NLLRegionVariableOrigin,
+    error_reporting::nice_region_error::NiceRegionError, InferCtxt, NLLRegionVariableOrigin,
 };
 use rustc::mir::{Body, ConstraintCategory, Location};
 use rustc::ty::{self, RegionVid, Ty};
@@ -16,8 +15,11 @@ use std::collections::VecDeque;
 use crate::util::borrowck_errors;
 
 use crate::borrow_check::{
-    constraints::OutlivesConstraint, nll::ConstraintDescription,
-    region_infer::RegionInferenceContext, type_check::Locations, universal_regions::DefiningTy,
+    constraints::OutlivesConstraint,
+    nll::ConstraintDescription,
+    region_infer::{values::RegionElement, RegionInferenceContext, TypeTest},
+    type_check::Locations,
+    universal_regions::DefiningTy,
     MirBorrowckCtxt,
 };
 
@@ -62,23 +64,8 @@ crate type RegionErrors<'tcx> = Vec<RegionErrorKind<'tcx>>;
 
 #[derive(Clone, Debug)]
 crate enum RegionErrorKind<'tcx> {
-    /// An error for a type test: `T: 'a` does not live long enough.
-    TypeTestDoesNotLiveLongEnough {
-        /// The span of the type test.
-        span: Span,
-        /// The generic type of the type test.
-        generic: GenericKind<'tcx>,
-    },
-
-    /// A generic bound failure for a type test.
-    TypeTestGenericBoundError {
-        /// The span of the type test.
-        span: Span,
-        /// The generic type of the type test.
-        generic: GenericKind<'tcx>,
-        /// The lower bound region.
-        lower_bound_region: ty::Region<'tcx>,
-    },
+    /// A generic bound failure for a type test (`T: 'a`).
+    TypeTestError { type_test: TypeTest<'tcx> },
 
     /// An unexpected hidden region for an opaque type.
     UnexpectedHiddenRegion {
@@ -94,8 +81,8 @@ crate enum RegionErrorKind<'tcx> {
     BoundUniversalRegionError {
         /// The placeholder free region.
         longer_fr: RegionVid,
-        /// The region that erroneously must be outlived by `longer_fr`.
-        error_region: RegionVid,
+        /// The region element that erroneously must be outlived by `longer_fr`.
+        error_element: RegionElement,
         /// The origin of the placeholder region.
         fr_origin: NLLRegionVariableOrigin,
     },

src/librustc_mir/borrow_check/mod.rs

@@ -631,7 +631,7 @@ impl<'cx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tcx
 
                 debug!(
                     "visit_terminator_drop \
-                        loc: {:?} term: {:?} drop_place: {:?} drop_place_ty: {:?} span: {:?}",
+                     loc: {:?} term: {:?} drop_place: {:?} drop_place_ty: {:?} span: {:?}",
                     loc, term, drop_place, drop_place_ty, span
                 );
 
@@ -1477,38 +1477,42 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
 
         for nll_error in nll_errors.into_iter() {
             match nll_error {
-                RegionErrorKind::TypeTestDoesNotLiveLongEnough { span, generic } => {
-                    // FIXME. We should handle this case better. It
-                    // indicates that we have e.g., some region variable
-                    // whose value is like `'a+'b` where `'a` and `'b` are
-                    // distinct unrelated univesal regions that are not
-                    // known to outlive one another. It'd be nice to have
-                    // some examples where this arises to decide how best
-                    // to report it; we could probably handle it by
-                    // iterating over the universal regions and reporting
-                    // an error that multiple bounds are required.
-                    self.infcx
-                        .tcx
-                        .sess
-                        .struct_span_err(span, &format!("`{}` does not live long enough", generic))
-                        .buffer(&mut self.errors_buffer);
-                }
-
-                RegionErrorKind::TypeTestGenericBoundError {
-                    span,
-                    generic,
-                    lower_bound_region,
-                } => {
-                    let region_scope_tree = &self.infcx.tcx.region_scope_tree(self.mir_def_id);
-                    self.infcx
-                        .construct_generic_bound_failure(
-                            region_scope_tree,
-                            span,
-                            None,
-                            generic,
-                            lower_bound_region,
-                        )
-                        .buffer(&mut self.errors_buffer);
+                RegionErrorKind::TypeTestError { type_test } => {
+                    // Try to convert the lower-bound region into something named we can print for the user.
+                    let lower_bound_region =
+                        self.nonlexical_regioncx.to_error_region(type_test.lower_bound);
+
+                    // Skip duplicate-ish errors.
+                    let type_test_span = type_test.locations.span(&self.body);
+
+                    if let Some(lower_bound_region) = lower_bound_region {
+                        let region_scope_tree = &self.infcx.tcx.region_scope_tree(self.mir_def_id);
+                        self.infcx
+                            .construct_generic_bound_failure(
+                                region_scope_tree,
+                                type_test_span,
+                                None,
+                                type_test.generic_kind,
+                                lower_bound_region,
+                            )
+                            .buffer(&mut self.errors_buffer);
+                    } else {
+                        // FIXME. We should handle this case better. It indicates that we have
+                        // e.g., some region variable whose value is like `'a+'b` where `'a` and
+                        // `'b` are distinct unrelated univesal regions that are not known to
+                        // outlive one another. It'd be nice to have some examples where this
+                        // arises to decide how best to report it; we could probably handle it by
+                        // iterating over the universal regions and reporting an error that
+                        // multiple bounds are required.
+                        self.infcx
+                            .tcx
+                            .sess
+                            .struct_span_err(
+                                type_test_span,
+                                &format!("`{}` does not live long enough", type_test.generic_kind),
+                            )
+                            .buffer(&mut self.errors_buffer);
+                    }
                 }
 
                 RegionErrorKind::UnexpectedHiddenRegion {
@@ -1530,8 +1534,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 RegionErrorKind::BoundUniversalRegionError {
                     longer_fr,
                     fr_origin,
-                    error_region,
+                    error_element,
                 } => {
+                    let error_region =
+                        self.nonlexical_regioncx.region_from_element(longer_fr, error_element);
+
                     // Find the code to blame for the fact that `longer_fr` outlives `error_fr`.
                     let (_, span) = self.nonlexical_regioncx.find_outlives_blame_span(
                         &self.body,
@@ -2225,7 +2232,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                             let upvar = &self.upvars[field.index()];
                             debug!(
                                 "upvar.mutability={:?} local_mutation_is_allowed={:?} \
-                                place={:?}",
+                                 place={:?}",
                                 upvar, is_local_mutation_allowed, place
                             );
                             match (upvar.mutability, is_local_mutation_allowed) {

src/librustc_mir/borrow_check/region_infer/mod.rs

@@ -838,39 +838,20 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             }
 
             // Type-test failed. Report the error.
-
-            // Try to convert the lower-bound region into something named we can print for the user.
-            let lower_bound_region = self.to_error_region(type_test.lower_bound);
-
-            // Skip duplicate-ish errors.
-            let type_test_span = type_test.locations.span(body);
-            let erased_generic_kind = tcx.erase_regions(&type_test.generic_kind);
-            if !deduplicate_errors.insert((
+            let erased_generic_kind = infcx.tcx.erase_regions(&type_test.generic_kind);
+            if deduplicate_errors.insert((
                 erased_generic_kind,
-                lower_bound_region,
+                type_test.lower_bound,
                 type_test.locations,
             )) {
-                continue;
-            } else {
                 debug!(
                     "check_type_test: reporting error for erased_generic_kind={:?}, \
                      lower_bound_region={:?}, \
                      type_test.locations={:?}",
-                    erased_generic_kind, lower_bound_region, type_test.locations,
+                    erased_generic_kind, type_test.lower_bound, type_test.locations,
                 );
-            }
 
-            if let Some(lower_bound_region) = lower_bound_region {
-                errors_buffer.push(RegionErrorKind::TypeTestGenericBoundError {
-                    span: type_test_span,
-                    generic: type_test.generic_kind,
-                    lower_bound_region,
-                });
-            } else {
-                errors_buffer.push(RegionErrorKind::TypeTestDoesNotLiveLongEnough {
-                    span: type_test_span,
-                    generic: type_test.generic_kind,
-                });
+                errors_buffer.push(RegionErrorKind::TypeTestError { type_test: type_test.clone() });
             }
         }
     }
@@ -1355,7 +1336,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         for (longer_fr, shorter_fr) in subset_errors.into_iter() {
             debug!(
                 "check_polonius_subset_errors: subset_error longer_fr={:?},\
-                shorter_fr={:?}",
+                 shorter_fr={:?}",
                 longer_fr, shorter_fr
             );
 
@@ -1572,23 +1553,9 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         debug!("check_bound_universal_region: error_element = {:?}", error_element);
 
         // Find the region that introduced this `error_element`.
-        let error_region = match error_element {
-            RegionElement::Location(l) => self.find_sub_region_live_at(longer_fr, l),
-            RegionElement::RootUniversalRegion(r) => r,
-            RegionElement::PlaceholderRegion(error_placeholder) => self
-                .definitions
-                .iter_enumerated()
-                .filter_map(|(r, definition)| match definition.origin {
-                    NLLRegionVariableOrigin::Placeholder(p) if p == error_placeholder => Some(r),
-                    _ => None,
-                })
-                .next()
-                .unwrap(),
-        };
-
         errors_buffer.push(RegionErrorKind::BoundUniversalRegionError {
             longer_fr,
-            error_region,
+            error_element,
             fr_origin: NLLRegionVariableOrigin::Placeholder(placeholder),
         });
     }
@@ -1628,6 +1595,23 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             });
         }
     }
+
+    /// Get the region outlived by `longer_fr` and live at `element`.
+    crate fn region_from_element(&self, longer_fr: RegionVid, element: RegionElement) -> RegionVid {
+        match element {
+            RegionElement::Location(l) => self.find_sub_region_live_at(longer_fr, l),
+            RegionElement::RootUniversalRegion(r) => r,
+            RegionElement::PlaceholderRegion(error_placeholder) => self
+                .definitions
+                .iter_enumerated()
+                .filter_map(|(r, definition)| match definition.origin {
+                    NLLRegionVariableOrigin::Placeholder(p) if p == error_placeholder => Some(r),
+                    _ => None,
+                })
+                .next()
+                .unwrap(),
+        }
+    }
 }
 
 impl<'tcx> RegionDefinition<'tcx> {

src/librustc_mir/borrow_check/region_infer/values.rs

@@ -114,7 +114,7 @@ rustc_index::newtype_index! {
 
 /// An individual element in a region value -- the value of a
 /// particular region variable consists of a set of these elements.
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 crate enum RegionElement {
     /// A point in the control-flow graph.
     Location(Location),