diff --git a/src/librustc_mir/borrow_check/constraints/mod.rs b/src/librustc_mir/borrow_check/constraints/mod.rs index 48defecd28cb0..a8f94508add71 100644 --- a/src/librustc_mir/borrow_check/constraints/mod.rs +++ b/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 + ) } } diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index 8d534b6ec8e8e..a78fa363bc107 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/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 = 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 } } } diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index 73267b0f39973..81cfc0c315c26 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -1230,6 +1230,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { }); if !universal_outlives { + debug!("eval_outlives: universal_outlives=false, returning false"); return false; } @@ -1237,11 +1238,17 @@ impl<'tcx> RegionInferenceContext<'tcx> { // 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 diff --git a/src/librustc_mir/borrow_check/type_check/free_region_relations.rs b/src/librustc_mir/borrow_check/type_check/free_region_relations.rs index 0e4801b88d87e..84964557b4409 100644 --- a/src/librustc_mir/borrow_check/type_check/free_region_relations.rs +++ b/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 diff --git a/src/librustc_mir/borrow_check/universal_regions.rs b/src/librustc_mir/borrow_check/universal_regions.rs index c7ef017215e0c..6258b0eff5080 100644 --- a/src/librustc_mir/borrow_check/universal_regions.rs +++ b/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>, + + /// 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>, } /// 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>) { 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>) 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>) 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, Vec> = 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> = Default::default(); + for region_set in dup_regions_map.into_iter().map(|(_, v)| v) { + for first in ®ion_set { + for second in ®ion_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( diff --git a/src/test/ui/async-await/issue-67765-async-diagnostic.rs b/src/test/ui/async-await/issue-67765-async-diagnostic.rs new file mode 100644 index 0000000000000..3885de564d3fb --- /dev/null +++ b/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(()) +} diff --git a/src/test/ui/async-await/issue-67765-async-diagnostic.stderr b/src/test/ui/async-await/issue-67765-async-diagnostic.stderr new file mode 100644 index 0000000000000..0cea366952c1f --- /dev/null +++ b/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`.