Account for 'duplicate' closure regions in borrowck diagnostics

src/librustc_mir/borrow_check/constraints/mod.rs

@@ -93,7 +93,11 @@ pub struct OutlivesConstraint {
 
 impl fmt::Debug for OutlivesConstraint {
     fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(formatter, "({:?}: {:?}) due to {:?}", self.sup, self.sub, self.locations)
+        write!(
+            formatter,
+            "({:?}: {:?}) due to {:?} ({:?})",
+            self.sup, self.sub, self.locations, self.category
+        )
     }
 }
 

src/librustc_mir/borrow_check/diagnostics/region_errors.rs

@@ -23,6 +23,7 @@ use crate::borrow_check::{
 };
 
 use super::{OutlivesSuggestionBuilder, RegionErrorNamingCtx, RegionName, RegionNameSource};
+use rustc_data_structures::fx::FxHashSet;
 
 impl ConstraintDescription for ConstraintCategory {
     fn description(&self) -> &'static str {
@@ -146,13 +147,34 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         if self.universal_regions.is_universal_region(r) {
             Some(r)
         } else {
+            debug!("to_error_region_vid(r={:?}={})", r, self.region_value_str(r));
+
+            // A modified version of `universal_upper_bound`, adapted for
+            // diagnostic purposes.
+            let mut lub = self.universal_regions.fr_fn_body;
             let r_scc = self.constraint_sccs.scc(r);
-            let upper_bound = self.universal_upper_bound(r);
-            if self.scc_values.contains(r_scc, upper_bound) {
-                self.to_error_region_vid(upper_bound)
-            } else {
-                None
+
+            // The set of all 'duplicate' regions that we've seen so far.
+            // See the `diagnostic_dup_regions` field docs for more details
+            let mut duplicates: FxHashSet<RegionVid> = Default::default();
+            for ur in self.scc_values.universal_regions_outlived_by(r_scc) {
+                let duplicate_region = duplicates.contains(&ur);
+                debug!("to_error_region_vid: ur={:?}, duplicate_region={}", ur, duplicate_region);
+                if !duplicate_region {
+                    // Since we're computing an upper bound using
+                    // this region, we do *not* want to compute
+                    // upper bounds using any duplicates of it.
+                    // We extend our set of duplicates with all of the duplicates
+                    // correspodnign to this region (if it has any duplicates),
+                    self.universal_regions
+                        .diagnostic_dup_regions
+                        .get(&ur)
+                        .map(|v| duplicates.extend(v));
+                    lub = self.universal_region_relations.postdom_upper_bound(lub, ur);
+                }
             }
+
+            if self.scc_values.contains(r_scc, lub) { self.to_error_region_vid(lub) } else { None }
         }
     }
 

src/librustc_mir/borrow_check/region_infer/mod.rs

@@ -1230,18 +1230,25 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             });
 
         if !universal_outlives {
+            debug!("eval_outlives: universal_outlives=false, returning false");
             return false;
         }
 
         // Now we have to compare all the points in the sub region and make
         // sure they exist in the sup region.
 
         if self.universal_regions.is_universal_region(sup_region) {
+            debug!("eval_outlives: sup_region={:?} is universal, returning true", sup_region);
             // Micro-opt: universal regions contain all points.
             return true;
         }
 
-        self.scc_values.contains_points(sup_region_scc, sub_region_scc)
+        let res = self.scc_values.contains_points(sup_region_scc, sub_region_scc);
+        debug!(
+            "eval_outlives: scc_values.contains_points(sup_region_scc={:?}, sub_region_scc={:?}) = {:?}",
+            sup_region_scc, sub_region_scc, res
+        );
+        res
     }
 
     /// Once regions have been propagated, this method is used to see

src/librustc_mir/borrow_check/type_check/free_region_relations.rs

@@ -96,12 +96,15 @@ impl UniversalRegionRelations<'tcx> {
     /// (See `TransitiveRelation::postdom_upper_bound` for details on
     /// the postdominating upper bound in general.)
     crate fn postdom_upper_bound(&self, fr1: RegionVid, fr2: RegionVid) -> RegionVid {
+        debug!("postdom_upper_bound(fr1={:?}, fr2={:?})", fr1, fr2);
         assert!(self.universal_regions.is_universal_region(fr1));
         assert!(self.universal_regions.is_universal_region(fr2));
-        *self
+        let res = *self
             .inverse_outlives
             .postdom_upper_bound(&fr1, &fr2)
-            .unwrap_or(&self.universal_regions.fr_static)
+            .unwrap_or(&self.universal_regions.fr_static);
+        debug!("postdom_upper_bound(fr1={:?}, fr2={:?}) = {:?}", fr1, fr2, res);
+        res
     }
 
     /// Finds an "upper bound" for `fr` that is not local. In other

src/librustc_mir/borrow_check/universal_regions.rs

@@ -26,6 +26,7 @@ use rustc_index::vec::{Idx, IndexVec};
 use std::iter;
 
 use crate::borrow_check::nll::ToRegionVid;
+use rustc_data_structures::fx::FxHashSet;
 
 #[derive(Debug)]
 pub struct UniversalRegions<'tcx> {
@@ -73,6 +74,37 @@ pub struct UniversalRegions<'tcx> {
     pub unnormalized_input_tys: &'tcx [Ty<'tcx>],
 
     pub yield_ty: Option<Ty<'tcx>>,
+
+    /// Extra information about region relationships, used
+    /// only when printing diagnostics.
+    ///
+    /// When processing closures/generators, we may generate multiple
+    /// region variables that all correspond to the same early-bound region.
+    /// We don't want to record these in `UniversalRegionRelations`,
+    /// as this would interfere with the propagation of closure
+    /// region constraints back to the parent function.
+    ///
+    /// Instead, we record this additional information here.
+    /// We map each region variable to a set of all other
+    /// region variables that correspond to the same early-bound region.
+    ///
+    /// For example, if we generate the following variables:
+    ///
+    /// 'a -> (_#0r, _#1r)
+    /// 'b -> (_#2r, _#3r)
+    ///
+    /// Then the map will look like this:
+    /// _#0r -> _#1r
+    /// _#1r -> _#0r
+    /// _#2r -> _#3r
+    /// _#3r -> _#2r
+    ///
+    /// When we compute upper bounds during diagnostic generation,
+    /// we accumulate a set of 'duplicate' from all non-duplicate
+    /// regions we've seen so far. Before we compute an upper bound,
+    /// we check if the region appears in our duplicates set - if so,
+    /// we skip it.
+    pub diagnostic_dup_regions: FxHashMap<RegionVid, FxHashSet<RegionVid>>,
 }
 
 /// The "defining type" for this MIR. The key feature of the "defining
@@ -234,9 +266,14 @@ impl<'tcx> UniversalRegions<'tcx> {
         assert_eq!(
             region_mapping.len(),
             expected_num_vars,
-            "index vec had unexpected number of variables"
+            "index vec had unexpected number of variables: {:?}",
+            region_mapping
         );
 
+        debug!(
+            "closure_mapping: closure_substs={:?} closure_base_def_id={:?} region_mapping={:?}",
+            closure_substs, closure_base_def_id, region_mapping
+        );
         region_mapping
     }
 
@@ -378,8 +415,8 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
         // add will be external.
         let first_extern_index = self.infcx.num_region_vars();
 
-        let defining_ty = self.defining_ty();
-        debug!("build: defining_ty={:?}", defining_ty);
+        let (defining_ty, dup_regions) = self.defining_ty();
+        debug!("build: defining_ty={:?} dup_regions={:?}", defining_ty, dup_regions);
 
         let mut indices = self.compute_indices(fr_static, defining_ty);
         debug!("build: indices={:?}", indices);
@@ -396,8 +433,12 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
             self.infcx.replace_late_bound_regions_with_nll_infer_vars(self.mir_def_id, &mut indices)
         }
 
+        debug!("build: after closure: indices={:?}", indices);
+
         let bound_inputs_and_output = self.compute_inputs_and_output(&indices, defining_ty);
 
+        debug!("build: compute_inputs_and_output: indices={:?}", indices);
+
         // "Liberate" the late-bound regions. These correspond to
         // "local" free regions.
         let first_local_index = self.infcx.num_region_vars();
@@ -462,13 +503,14 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
             defining_ty,
             unnormalized_output_ty,
             unnormalized_input_tys,
-            yield_ty: yield_ty,
+            yield_ty,
+            diagnostic_dup_regions: dup_regions,
         }
     }
 
     /// Returns the "defining type" of the current MIR;
     /// see `DefiningTy` for details.
-    fn defining_ty(&self) -> DefiningTy<'tcx> {
+    fn defining_ty(&self) -> (DefiningTy<'tcx>, FxHashMap<RegionVid, FxHashSet<RegionVid>>) {
         let tcx = self.infcx.tcx;
         let closure_base_def_id = tcx.closure_base_def_id(self.mir_def_id);
 
@@ -483,10 +525,10 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
 
                 debug!("defining_ty (pre-replacement): {:?}", defining_ty);
 
-                let defining_ty =
+                let (defining_ty, dup_regions) =
                     self.infcx.replace_free_regions_with_nll_infer_vars(FR, &defining_ty);
 
-                match defining_ty.kind {
+                let def_ty = match defining_ty.kind {
                     ty::Closure(def_id, substs) => DefiningTy::Closure(def_id, substs),
                     ty::Generator(def_id, substs, movability) => {
                         DefiningTy::Generator(def_id, substs, movability)
@@ -498,15 +540,16 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
                         self.mir_def_id,
                         defining_ty
                     ),
-                }
+                };
+                (def_ty, dup_regions)
             }
 
             BodyOwnerKind::Const | BodyOwnerKind::Static(..) => {
                 assert_eq!(closure_base_def_id, self.mir_def_id);
                 let identity_substs = InternalSubsts::identity_for_item(tcx, closure_base_def_id);
-                let substs =
+                let (substs, dup_regions) =
                     self.infcx.replace_free_regions_with_nll_infer_vars(FR, &identity_substs);
-                DefiningTy::Const(self.mir_def_id, substs)
+                (DefiningTy::Const(self.mir_def_id, substs), dup_regions)
             }
         }
     }
@@ -609,7 +652,7 @@ trait InferCtxtExt<'tcx> {
         &self,
         origin: NLLRegionVariableOrigin,
         value: &T,
-    ) -> T
+    ) -> (T, FxHashMap<RegionVid, FxHashSet<RegionVid>>)
     where
         T: TypeFoldable<'tcx>;
 
@@ -635,11 +678,35 @@ impl<'cx, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'cx, 'tcx> {
         &self,
         origin: NLLRegionVariableOrigin,
         value: &T,
-    ) -> T
+    ) -> (T, FxHashMap<RegionVid, FxHashSet<RegionVid>>)
     where
         T: TypeFoldable<'tcx>,
     {
-        self.tcx.fold_regions(value, &mut false, |_region, _depth| self.next_nll_region_var(origin))
+        let mut dup_regions_map: FxHashMap<ty::Region<'tcx>, Vec<RegionVid>> = Default::default();
+        let folded = self.tcx.fold_regions(value, &mut false, |region, _depth| {
+            let new_region = self.next_nll_region_var(origin);
+            let new_vid = match new_region {
+                ty::ReVar(vid) => vid,
+                _ => unreachable!(),
+            };
+            dup_regions_map.entry(region).or_insert_with(|| Vec::new()).push(*new_vid);
+            new_region
+        });
+        let mut dup_regions: FxHashMap<RegionVid, FxHashSet<RegionVid>> = Default::default();
+        for region_set in dup_regions_map.into_iter().map(|(_, v)| v) {
+            for first in &region_set {
+                for second in &region_set {
+                    if first != second {
+                        dup_regions.entry(*first).or_default().insert(*second);
+                    }
+                }
+            }
+        }
+        debug!(
+            "replace_free_regions_with_nll_infer_vars({:?}): dup_regions={:?} folded={:?}",
+            value, dup_regions, folded
+        );
+        (folded, dup_regions)
     }
 
     fn replace_bound_regions_with_nll_infer_vars<T>(

src/test/ui/async-await/issue-67765-async-diagnostic.rs

@@ -0,0 +1,13 @@
+// edition:2018
+
+fn main() {}
+
+async fn func<'a>() -> Result<(), &'a str> {
+    let s = String::new();
+
+    let b = &s[..];
+
+    Err(b)?; //~ ERROR cannot return value referencing local variable `s`
+
+    Ok(())
+}

src/test/ui/async-await/issue-67765-async-diagnostic.stderr

@@ -0,0 +1,12 @@
+error[E0515]: cannot return value referencing local variable `s`
+  --> $DIR/issue-67765-async-diagnostic.rs:10:11
+   |
+LL |     let b = &s[..];
+   |              - `s` is borrowed here
+LL | 
+LL |     Err(b)?;
+   |           ^ returns a value referencing data owned by the current function
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0515`.