Classify outlives constraints when type checking

The MIR/NLL type checker is in a much better position to classify
constraints and already has to classify into boring and interesting.
Adds spans to Locations::All for error reporting
Adds more constraint categories

Author: matthewjasper

src/librustc/infer/opaque_types/mod.rs

@@ -284,7 +284,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         }
     }
 
-    fn constrain_opaque_type<FRR: FreeRegionRelations<'tcx>>(
+    pub fn constrain_opaque_type<FRR: FreeRegionRelations<'tcx>>(
         &self,
         def_id: DefId,
         opaque_defn: &OpaqueTypeDecl<'tcx>,

src/librustc_mir/borrow_check/nll/constraints/graph.rs

@@ -9,10 +9,12 @@
 // except according to those terms.
 
 use borrow_check::nll::type_check::Locations;
-use borrow_check::nll::constraints::{ConstraintIndex, ConstraintSet, OutlivesConstraint};
+use borrow_check::nll::constraints::{ConstraintCategory, ConstraintIndex};
+use borrow_check::nll::constraints::{ConstraintSet, OutlivesConstraint};
 use rustc::ty::RegionVid;
 use rustc_data_structures::graph;
 use rustc_data_structures::indexed_vec::IndexVec;
+use syntax_pos::DUMMY_SP;
 
 /// The construct graph organizes the constraints by their end-points.
 /// It can be used to view a `R1: R2` constraint as either an edge `R1
@@ -174,7 +176,8 @@ impl<'s, D: ConstraintGraphDirecton> Iterator for Edges<'s, D> {
             Some(OutlivesConstraint {
                 sup: self.static_region,
                 sub: next_static_idx.into(),
-                locations: Locations::All,
+                locations: Locations::All(DUMMY_SP),
+                category: ConstraintCategory::Internal,
             })
         } else {
             None

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

@@ -23,6 +23,42 @@ crate struct ConstraintSet {
     constraints: IndexVec<ConstraintIndex, OutlivesConstraint>,
 }
 
+/// Constraints can be categorized to determine whether and why they are
+/// interesting. Order of variants indicates sort order of the category,
+/// thereby influencing diagnostic output.
+#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)]
+pub enum ConstraintCategory {
+    Return,
+    TypeAnnotation,
+    Cast,
+    CallArgument,
+
+    /// A constraint that came from checking the body of a closure.
+    ///
+    /// Ideally we would give an explanation that points to the relevant part
+    /// of the closure's body.
+    ClosureBounds,
+    CopyBound,
+    SizedBound,
+    Assignment,
+    OpaqueType,
+
+    /// A "boring" constraint (caused by the given location) is one that
+    /// the user probably doesn't want to see described in diagnostics,
+    /// because it is kind of an artifact of the type system setup.
+    /// Example: `x = Foo { field: y }` technically creates
+    /// intermediate regions representing the "type of `Foo { field: y
+    /// }`", and data flows from `y` into those variables, but they
+    /// are not very interesting. The assignment into `x` on the other
+    /// hand might be.
+    Boring,
+    // Boring and applicable everywhere.
+    BoringNoLocation,
+
+    /// A constraint that doesn't correspond to anything the user sees.
+    Internal,
+}
+
 impl ConstraintSet {
     crate fn push(&mut self, constraint: OutlivesConstraint) {
         debug!(
@@ -87,6 +123,9 @@ pub struct OutlivesConstraint {
 
     /// Where did this constraint arise?
     pub locations: Locations,
+
+    /// What caused this constraint?
+    pub category: ConstraintCategory,
 }
 
 impl fmt::Debug for OutlivesConstraint {

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

@@ -88,11 +88,13 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                 sup,
                 sub,
                 locations,
+                category,
             } = constraint;
             with_msg(&format!(
-                "{:?}: {:?} due to {:?}",
+                "{:?}: {:?} due to {:?} at {:?}",
                 sup,
                 sub,
+                category,
                 locations,
             ))?;
         }

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

@@ -8,14 +8,13 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use borrow_check::nll::constraints::OutlivesConstraint;
+use borrow_check::nll::constraints::{OutlivesConstraint, ConstraintCategory};
 use borrow_check::nll::region_infer::RegionInferenceContext;
-use borrow_check::nll::type_check::Locations;
 use rustc::hir::def_id::DefId;
 use rustc::infer::error_reporting::nice_region_error::NiceRegionError;
 use rustc::infer::InferCtxt;
-use rustc::mir::{self, Location, Mir, Place, Rvalue, StatementKind, TerminatorKind};
-use rustc::ty::{self, TyCtxt, RegionVid};
+use rustc::mir::{Location, Mir};
+use rustc::ty::{self, RegionVid};
 use rustc_data_structures::indexed_vec::IndexVec;
 use rustc_errors::{Diagnostic, DiagnosticBuilder};
 use std::collections::VecDeque;
@@ -28,19 +27,6 @@ mod var_name;
 
 use self::region_name::RegionName;
 
-/// Constraints that are considered interesting can be categorized to
-/// determine why they are interesting. Order of variants indicates
-/// sort order of the category, thereby influencing diagnostic output.
-#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)]
-enum ConstraintCategory {
-    Cast,
-    Assignment,
-    Return,
-    CallArgument,
-    Other,
-    Boring,
-}
-
 impl fmt::Display for ConstraintCategory {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         // Must end with a space. Allows for empty names to be provided.
@@ -49,7 +35,14 @@ impl fmt::Display for ConstraintCategory {
             ConstraintCategory::Return => write!(f, "returning this value "),
             ConstraintCategory::Cast => write!(f, "cast "),
             ConstraintCategory::CallArgument => write!(f, "argument "),
-            _ => write!(f, ""),
+            ConstraintCategory::TypeAnnotation => write!(f, "type annotation "),
+            ConstraintCategory::ClosureBounds => write!(f, "closure body "),
+            ConstraintCategory::SizedBound => write!(f, "proving this value is `Sized` "),
+            ConstraintCategory::CopyBound => write!(f, "copying this value "),
+            ConstraintCategory::OpaqueType => write!(f, "opaque type "),
+            ConstraintCategory::Boring
+            | ConstraintCategory::BoringNoLocation
+            | ConstraintCategory::Internal => write!(f, ""),
         }
     }
 }
@@ -71,7 +64,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     fn best_blame_constraint(
         &self,
         mir: &Mir<'tcx>,
-        tcx: TyCtxt<'_, '_, 'tcx>,
         from_region: RegionVid,
         target_test: impl Fn(RegionVid) -> bool,
     ) -> (ConstraintCategory, Span, RegionVid) {
@@ -96,7 +88,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         // Classify each of the constraints along the path.
         let mut categorized_path: Vec<(ConstraintCategory, Span)> = path
             .iter()
-            .map(|&index| self.classify_constraint(index, mir, tcx))
+            .map(|constraint| (constraint.category, constraint.locations.span(mir)))
             .collect();
         debug!(
             "best_blame_constraint: categorized_path={:#?}",
@@ -129,12 +121,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             let constraint_sup_scc = self.constraint_sccs.scc(constraint.sup);
 
             match categorized_path[i].0 {
-                ConstraintCategory::Boring => false,
-                ConstraintCategory::Other => {
-                    // other isn't interesting when the two lifetimes
-                    // are unified.
-                    constraint_sup_scc != self.constraint_sccs.scc(constraint.sub)
-                }
+                ConstraintCategory::OpaqueType
+                | ConstraintCategory::Boring
+                | ConstraintCategory::BoringNoLocation
+                | ConstraintCategory::Internal => false,
                 _ => constraint_sup_scc != target_scc,
             }
         });
@@ -220,106 +210,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         None
     }
 
-    /// This function will return true if a constraint is interesting and false if a constraint
-    /// is not. It is useful in filtering constraint paths to only interesting points.
-    fn constraint_is_interesting(&self, constraint: OutlivesConstraint) -> bool {
-        debug!(
-            "constraint_is_interesting: locations={:?} constraint={:?}",
-            constraint.locations, constraint
-        );
-
-        match constraint.locations {
-            Locations::Interesting(_) | Locations::All => true,
-            _ => false,
-        }
-    }
-
-    /// This function classifies a constraint from a location.
-    fn classify_constraint(
-        &self,
-        constraint: OutlivesConstraint,
-        mir: &Mir<'tcx>,
-        tcx: TyCtxt<'_, '_, 'tcx>,
-    ) -> (ConstraintCategory, Span) {
-        debug!("classify_constraint: constraint={:?}", constraint);
-        let span = constraint.locations.span(mir);
-        let location = constraint
-            .locations
-            .from_location()
-            .unwrap_or(Location::START);
-
-        if !self.constraint_is_interesting(constraint) {
-            return (ConstraintCategory::Boring, span);
-        }
-
-        let data = &mir[location.block];
-        debug!(
-            "classify_constraint: location={:?} data={:?}",
-            location, data
-        );
-        let category = if location.statement_index == data.statements.len() {
-            if let Some(ref terminator) = data.terminator {
-                debug!("classify_constraint: terminator.kind={:?}", terminator.kind);
-                match terminator.kind {
-                    TerminatorKind::DropAndReplace { .. } => ConstraintCategory::Assignment,
-                    // Classify calls differently depending on whether or not
-                    // the sub region appears in the destination type (so the
-                    // sup region is in the return type). If the return type
-                    // contains the sub-region, then this is either an
-                    // assignment or a return, depending on whether we are
-                    // writing to the RETURN_PLACE or not.
-                    //
-                    // The idea here is that the region is being propagated
-                    // from an input into the output place, so it's a kind of
-                    // assignment. Otherwise, if the sub-region only appears in
-                    // the argument types, then use the CallArgument
-                    // classification.
-                    TerminatorKind::Call { destination: Some((ref place, _)), .. } => {
-                        if tcx.any_free_region_meets(
-                            &place.ty(mir, tcx).to_ty(tcx),
-                            |region| self.to_region_vid(region) == constraint.sub,
-                        ) {
-                            match place {
-                                Place::Local(mir::RETURN_PLACE) => ConstraintCategory::Return,
-                                _ => ConstraintCategory::Assignment,
-                            }
-                        } else {
-                            ConstraintCategory::CallArgument
-                        }
-                    }
-                    TerminatorKind::Call { destination: None, .. } => {
-                        ConstraintCategory::CallArgument
-                    }
-                    _ => ConstraintCategory::Other,
-                }
-            } else {
-                ConstraintCategory::Other
-            }
-        } else {
-            let statement = &data.statements[location.statement_index];
-            debug!("classify_constraint: statement.kind={:?}", statement.kind);
-            match statement.kind {
-                StatementKind::Assign(ref place, ref rvalue) => {
-                    debug!("classify_constraint: place={:?} rvalue={:?}", place, rvalue);
-                    if *place == Place::Local(mir::RETURN_PLACE) {
-                        ConstraintCategory::Return
-                    } else {
-                        match rvalue {
-                            Rvalue::Cast(..) => ConstraintCategory::Cast,
-                            Rvalue::Use(..) | Rvalue::Aggregate(..) => {
-                                ConstraintCategory::Assignment
-                            }
-                            _ => ConstraintCategory::Other,
-                        }
-                    }
-                }
-                _ => ConstraintCategory::Other,
-            }
-        };
-
-        (category, span)
-    }
-
     /// Report an error because the universal region `fr` was required to outlive
     /// `outlived_fr` but it is not known to do so. For example:
     ///
@@ -341,7 +231,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
 
         let (category, span, _) = self.best_blame_constraint(
             mir,
-            infcx.tcx,
             fr,
             |r| r == outlived_fr
         );
@@ -574,11 +463,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     crate fn find_outlives_blame_span(
         &self,
         mir: &Mir<'tcx>,
-        tcx: TyCtxt<'_, '_, 'tcx>,
         fr1: RegionVid,
         fr2: RegionVid,
     ) -> Span {
-        let (_, span, _) = self.best_blame_constraint(mir, tcx, fr1, |r| r == fr2);
+        let (_, span, _) = self.best_blame_constraint(mir, fr1, |r| r == fr2);
         span
     }
 }

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

@@ -1062,7 +1062,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                 longer_fr, shorter_fr,
             );
 
-            let blame_span = self.find_outlives_blame_span(mir, infcx.tcx, longer_fr, shorter_fr);
+            let blame_span = 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).
@@ -1147,7 +1147,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         };
 
         // Find the code to blame for the fact that `longer_fr` outlives `error_fr`.
-        let span = self.find_outlives_blame_span(mir, infcx.tcx, longer_fr, error_region);
+        let span = self.find_outlives_blame_span(mir, longer_fr, error_region);
 
         // Obviously, this error message is far from satisfactory.
         // At present, though, it only appears in unit tests --

src/librustc_mir/borrow_check/nll/type_check/constraint_conversion.rs

@@ -9,7 +9,7 @@
 // except according to those terms.
 
 use borrow_check::location::LocationTable;
-use borrow_check::nll::constraints::{ConstraintSet, OutlivesConstraint};
+use borrow_check::nll::constraints::{ConstraintCategory, ConstraintSet, OutlivesConstraint};
 use borrow_check::nll::facts::AllFacts;
 use borrow_check::nll::region_infer::{RegionTest, TypeTest};
 use borrow_check::nll::type_check::Locations;
@@ -30,6 +30,7 @@ crate struct ConstraintConversion<'a, 'gcx: 'tcx, 'tcx: 'a> {
     implicit_region_bound: Option<ty::Region<'tcx>>,
     param_env: ty::ParamEnv<'tcx>,
     locations: Locations,
+    category: ConstraintCategory,
     outlives_constraints: &'a mut ConstraintSet,
     type_tests: &'a mut Vec<TypeTest<'tcx>>,
     all_facts: &'a mut Option<AllFacts>,
@@ -44,6 +45,7 @@ impl<'a, 'gcx, 'tcx> ConstraintConversion<'a, 'gcx, 'tcx> {
         implicit_region_bound: Option<ty::Region<'tcx>>,
         param_env: ty::ParamEnv<'tcx>,
         locations: Locations,
+        category: ConstraintCategory,
         outlives_constraints: &'a mut ConstraintSet,
         type_tests: &'a mut Vec<TypeTest<'tcx>>,
         all_facts: &'a mut Option<AllFacts>,
@@ -56,6 +58,7 @@ impl<'a, 'gcx, 'tcx> ConstraintConversion<'a, 'gcx, 'tcx> {
             implicit_region_bound,
             param_env,
             locations,
+            category,
             outlives_constraints,
             type_tests,
             all_facts,
@@ -183,6 +186,7 @@ impl<'a, 'gcx, 'tcx> ConstraintConversion<'a, 'gcx, 'tcx> {
     fn add_outlives(&mut self, sup: ty::RegionVid, sub: ty::RegionVid) {
         self.outlives_constraints.push(OutlivesConstraint {
             locations: self.locations,
+            category: self.category,
             sub,
             sup,
         });