Auto merge of #53745 - nikomatsakis:nll-issue-53570, r=pnkfelix

do not propagate closure requirements if we can prove them locally

Fixes #53570

cc @mikhail-m1 -- you're the one who last touched this code

r? @pnkfelix

Author: bors

src/librustc_mir/borrow_check/nll/mod.rs

@@ -276,7 +276,7 @@ fn dump_annotation<'a, 'gcx, 'tcx>(
     infcx: &InferCtxt<'a, 'gcx, 'tcx>,
     mir: &Mir<'tcx>,
     mir_def_id: DefId,
-    regioncx: &RegionInferenceContext,
+    regioncx: &RegionInferenceContext<'tcx>,
     closure_region_requirements: &Option<ClosureRegionRequirements>,
     errors_buffer: &mut Vec<Diagnostic>,
 ) {
@@ -299,7 +299,7 @@ fn dump_annotation<'a, 'gcx, 'tcx>(
             .diagnostic()
             .span_note_diag(mir.span, "External requirements");
 
-        regioncx.annotate(&mut err);
+        regioncx.annotate(tcx, &mut err);
 
         err.note(&format!(
             "number of external vids: {}",
@@ -319,7 +319,7 @@ fn dump_annotation<'a, 'gcx, 'tcx>(
             .sess
             .diagnostic()
             .span_note_diag(mir.span, "No external requirements");
-        regioncx.annotate(&mut err);
+        regioncx.annotate(tcx, &mut err);
 
         err.buffer(errors_buffer);
     }

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

@@ -29,11 +29,10 @@ use rustc::util::common;
 use rustc_data_structures::graph::scc::Sccs;
 use rustc_data_structures::indexed_set::IdxSet;
 use rustc_data_structures::indexed_vec::IndexVec;
-use rustc_errors::Diagnostic;
+use rustc_errors::{DiagnosticBuilder, Diagnostic};
 
 use std::rc::Rc;
 
-mod annotation;
 mod dump_mir;
 mod error_reporting;
 mod graphviz;
@@ -359,6 +358,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         self.universal_regions.to_region_vid(r)
     }
 
+    /// Add annotations for `#[rustc_regions]`; see `UniversalRegions::annotate`.
+    crate fn annotate(&self, tcx: TyCtxt<'_, '_, 'tcx>, err: &mut DiagnosticBuilder<'_>) {
+        self.universal_regions.annotate(tcx, err)
+    }
+
     /// Returns true if the region `r` contains the point `p`.
     ///
     /// Panics if called before `solve()` executes,
@@ -686,7 +690,6 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             test: _,
         } = type_test;
 
-
         let generic_ty = generic_kind.to_ty(tcx);
         let subject = match self.try_promote_type_test_subject(infcx, generic_ty) {
             Some(s) => s,
@@ -698,20 +701,38 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         // `ClosureOutlivesRequirement`.
         let r_scc = self.constraint_sccs.scc(*lower_bound);
         for ur in self.scc_values.universal_regions_outlived_by(r_scc) {
+            // Check whether we can already prove that the "subject" outlives `ur`.
+            // If so, we don't have to propagate this requirement to our caller.
+            //
+            // To continue the example from the function, if we are trying to promote
+            // a requirement that `T: 'X`, and we know that `'X = '1 + '2` (i.e., the union
+            // `'1` and `'2`), then in this loop `ur` will be `'1` (and `'2`). So here
+            // we check whether `T: '1` is something we *can* prove. If so, no need
+            // to propagate that requirement.
+            //
+            // This is needed because -- particularly in the case
+            // where `ur` is a local bound -- we are sometimes in a
+            // position to prove things that our caller cannot.  See
+            // #53570 for an example.
+            if self.eval_region_test(mir, ur, &type_test.test) {
+                continue;
+            }
+
+            debug!("try_promote_type_test: ur={:?}", ur);
+
             let non_local_ub = self.universal_region_relations.non_local_upper_bound(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)
-            );
+            assert!(!self.universal_regions.is_local_free_region(non_local_ub));
 
-            propagated_outlives_requirements.push(ClosureOutlivesRequirement {
+            let requirement = ClosureOutlivesRequirement {
                 subject,
                 outlived_free_region: non_local_ub,
                 blame_span: locations.span(mir),
-            });
+            };
+            debug!("try_promote_type_test: pushing {:#?}", requirement);
+            propagated_outlives_requirements.push(requirement);
         }
         true
     }
@@ -917,8 +938,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         // now). Therefore, the sup-region outlives the sub-region if,
         // for each universal region R1 in the sub-region, there
         // exists some region R2 in the sup-region that outlives R1.
-        let universal_outlives = self
-            .scc_values
+        let universal_outlives = self.scc_values
             .universal_regions_outlived_by(sub_region_scc)
             .all(|r1| {
                 self.scc_values
@@ -1029,8 +1049,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         // (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
+            if self.universal_region_relations
                 .outlives(longer_fr, shorter_fr)
             {
                 continue;
@@ -1046,8 +1065,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             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
+                if let Some(fr_minus) = self.universal_region_relations
                     .non_local_lower_bound(longer_fr)
                 {
                     debug!("check_universal_region: fr_minus={:?}", fr_minus);
@@ -1056,8 +1074,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                     // 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
+                    let shorter_fr_plus = self.universal_region_relations
                         .non_local_upper_bound(shorter_fr);
                     debug!(
                         "check_universal_region: shorter_fr_plus={:?}",
@@ -1117,8 +1134,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         let error_region = match error_element {
             RegionElement::Location(l) => self.find_sub_region_live_at(longer_fr, l),
             RegionElement::RootUniversalRegion(r) => r,
-            RegionElement::SubUniversalRegion(error_ui) => self
-                .definitions
+            RegionElement::SubUniversalRegion(error_ui) => self.definitions
                 .iter_enumerated()
                 .filter_map(|(r, definition)| match definition.origin {
                     NLLRegionVariableOrigin::BoundRegion(ui) if error_ui == ui => Some(r),
@@ -1215,7 +1231,11 @@ impl<'gcx, 'tcx> ClosureRegionRequirementsExt<'gcx, 'tcx> for ClosureRegionRequi
         // into a vector.  These are the regions that we will be
         // relating to one another.
         let closure_mapping = &UniversalRegions::closure_mapping(
-            tcx, user_closure_ty, self.num_external_vids, tcx.closure_base_def_id(closure_def_id));
+            tcx,
+            user_closure_ty,
+            self.num_external_vids,
+            tcx.closure_base_def_id(closure_def_id),
+        );
         debug!("apply_requirements: closure_mapping={:?}", closure_mapping);
 
         // Create the predicates.

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

@@ -31,6 +31,7 @@ use rustc::ty::subst::Substs;
 use rustc::ty::{self, ClosureSubsts, GeneratorSubsts, RegionVid, Ty, TyCtxt};
 use rustc::util::nodemap::FxHashMap;
 use rustc_data_structures::indexed_vec::{Idx, IndexVec};
+use rustc_errors::DiagnosticBuilder;
 use std::iter;
 use syntax::ast;
 
@@ -310,6 +311,69 @@ impl<'tcx> UniversalRegions<'tcx> {
     pub fn to_region_vid(&self, r: ty::Region<'tcx>) -> RegionVid {
         self.indices.to_region_vid(r)
     }
+
+    /// As part of the NLL unit tests, you can annotate a function with
+    /// `#[rustc_regions]`, and we will emit information about the region
+    /// inference context and -- in particular -- the external constraints
+    /// that this region imposes on others. The methods in this file
+    /// handle the part about dumping the inference context internal
+    /// state.
+    crate fn annotate(&self, tcx: TyCtxt<'_, '_, 'tcx>, err: &mut DiagnosticBuilder<'_>) {
+        match self.defining_ty {
+            DefiningTy::Closure(def_id, substs) => {
+                err.note(&format!(
+                    "defining type: {:?} with closure substs {:#?}",
+                    def_id,
+                    &substs.substs[..]
+                ));
+
+                // FIXME: It'd be nice to print the late-bound regions
+                // here, but unfortunately these wind up stored into
+                // tests, and the resulting print-outs include def-ids
+                // and other things that are not stable across tests!
+                // So we just include the region-vid. Annoying.
+                let closure_base_def_id = tcx.closure_base_def_id(def_id);
+                for_each_late_bound_region_defined_on(tcx, closure_base_def_id, |r| {
+                    err.note(&format!(
+                        "late-bound region is {:?}",
+                        self.to_region_vid(r),
+                    ));
+                });
+            }
+            DefiningTy::Generator(def_id, substs, _) => {
+                err.note(&format!(
+                    "defining type: {:?} with generator substs {:#?}",
+                    def_id,
+                    &substs.substs[..]
+                ));
+
+                // FIXME: As above, we'd like to print out the region
+                // `r` but doing so is not stable across architectures
+                // and so forth.
+                let closure_base_def_id = tcx.closure_base_def_id(def_id);
+                for_each_late_bound_region_defined_on(tcx, closure_base_def_id, |r| {
+                    err.note(&format!(
+                        "late-bound region is {:?}",
+                        self.to_region_vid(r),
+                    ));
+                });
+            }
+            DefiningTy::FnDef(def_id, substs) => {
+                err.note(&format!(
+                    "defining type: {:?} with substs {:#?}",
+                    def_id,
+                    &substs[..]
+                ));
+            }
+            DefiningTy::Const(def_id, substs) => {
+                err.note(&format!(
+                    "defining constant type: {:?} with substs {:#?}",
+                    def_id,
+                    &substs[..]
+                ));
+            }
+        }
+    }
 }
 
 struct UniversalRegionsBuilder<'cx, 'gcx: 'tcx, 'tcx: 'cx> {

src/librustc_mir/borrow_check/nll/universal_regions.rs

@@ -12,6 +12,9 @@ LL | |         },
                i16,
                for<'r, 's> extern "rust-call" fn((std::cell::Cell<&'_#1r &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) u32>, std::cell::Cell<&'_#2r &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) u32>, std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) &'_#3r u32>, std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) u32>, std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>))
            ]
+   = note: late-bound region is '_#4r
+   = note: late-bound region is '_#5r
+   = note: late-bound region is '_#6r
 
 error: unsatisfied lifetime constraints
   --> $DIR/propagate-approximated-fail-no-postdom.rs:56:13

src/test/ui/nll/closure-requirements/propagate-approximated-fail-no-postdom.stderr

@@ -14,6 +14,8 @@ LL | |     });
                i16,
                for<'r, 's, 't0, 't1, 't2, 't3> extern "rust-call" fn((&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) std::cell::Cell<&'_#1r &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't0)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't1)) &'_#2r u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't2)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't3)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't1)) u32>))
            ]
+   = note: late-bound region is '_#3r
+   = note: late-bound region is '_#4r
    = note: number of external vids: 5
    = note: where '_#1r: '_#2r
 

src/test/ui/nll/closure-requirements/propagate-approximated-ref.stderr

@@ -15,6 +15,8 @@ LL | |     });
                i16,
                for<'r, 's, 't0, 't1, 't2> extern "rust-call" fn((&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) std::cell::Cell<&'_#1r &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't0)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't1)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't2)) u32>))
            ]
+   = note: late-bound region is '_#2r
+   = note: late-bound region is '_#3r
    = note: number of external vids: 4
    = note: where '_#1r: '_#0r
 

src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-no-bound.stderr

@@ -14,6 +14,8 @@ LL | |     });
                i16,
                for<'r, 's, 't0, 't1, 't2, 't3> extern "rust-call" fn((&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) std::cell::Cell<&'_#1r &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't0)) std::cell::Cell<&'_#2r &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't1)) u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't2)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>, &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't3)) std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 't1)) u32>))
            ]
+   = note: late-bound region is '_#3r
+   = note: late-bound region is '_#4r
    = note: number of external vids: 5
    = note: where '_#1r: '_#0r
 

src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-wrong-bound.stderr

@@ -14,6 +14,8 @@ LL | |     });
                i16,
                for<'r, 's> extern "rust-call" fn((std::cell::Cell<&'_#1r &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) u32>, std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) &'_#2r u32>, std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) u32>, std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>))
            ]
+   = note: late-bound region is '_#3r
+   = note: late-bound region is '_#4r
    = note: number of external vids: 5
    = note: where '_#1r: '_#2r
 

src/test/ui/nll/closure-requirements/propagate-approximated-val.stderr

@@ -12,6 +12,7 @@ LL | |         },
                i16,
                for<'r, 's> extern "rust-call" fn((std::cell::Cell<&'_#1r &ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) u32>, std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) &'_#2r u32>, std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 'r)) u32>, std::cell::Cell<&ReLateBound(DebruijnIndex(0), BrNamed(crate0:DefIndex(0:0), 's)) u32>))
            ]
+   = note: late-bound region is '_#3r
    = note: number of external vids: 4
    = note: where '_#1r: '_#2r