rework so that we pick static as a last resort only

Author: nikomatsakis

compiler/rustc_borrowck/src/region_infer/mod.rs

@@ -733,44 +733,58 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             return false;
         }
 
-        // If there WERE no upper bounds (apart from static), and static is one of the options,
-        // then we can just pick that (c.f. #63033).
-        let choice = if !any_upper_bounds && choice_regions.contains(&fr_static) {
-            fr_static
-        } else {
-            // Otherwise, we need to find the minimum remaining choice, if
-            // any, and take that.
-            debug!("apply_member_constraint: choice_regions remaining are {:#?}", choice_regions);
-            let min = |r1: ty::RegionVid, r2: ty::RegionVid| -> Option<ty::RegionVid> {
-                let r1_outlives_r2 = self.universal_region_relations.outlives(r1, r2);
-                let r2_outlives_r1 = self.universal_region_relations.outlives(r2, r1);
-                match (r1_outlives_r2, r2_outlives_r1) {
-                    (true, true) => Some(r1.min(r2)),
-                    (true, false) => Some(r2),
-                    (false, true) => Some(r1),
-                    (false, false) => None,
-                }
-            };
-            let mut min_choice = choice_regions[0];
-            for &other_option in &choice_regions[1..] {
-                debug!(
-                    "apply_member_constraint: min_choice={:?} other_option={:?}",
-                    min_choice, other_option,
-                );
-                match min(min_choice, other_option) {
-                    Some(m) => min_choice = m,
-                    None => {
-                        debug!(
-                            "apply_member_constraint: {:?} and {:?} are incomparable; no min choice",
-                            min_choice, other_option,
+        // Otherwise, we need to find the minimum remaining choice, if
+        // any, and take that.
+        debug!("apply_member_constraint: choice_regions remaining are {:#?}", choice_regions);
+        let min = |r1: ty::RegionVid, r2: ty::RegionVid| -> Option<ty::RegionVid> {
+            let r1_outlives_r2 = self.universal_region_relations.outlives(r1, r2);
+            let r2_outlives_r1 = self.universal_region_relations.outlives(r2, r1);
+            match (r1_outlives_r2, r2_outlives_r1) {
+                (true, true) => Some(r1.min(r2)),
+                (true, false) => Some(r2),
+                (false, true) => Some(r1),
+                (false, false) => None,
+            }
+        };
+        let mut min_choice = choice_regions[0];
+        for &other_option in &choice_regions[1..] {
+            debug!(
+                "apply_member_constraint: min_choice={:?} other_option={:?}",
+                min_choice, other_option,
+            );
+            match min(min_choice, other_option) {
+                Some(m) => min_choice = m,
+                None => {
+                    debug!(
+                        "apply_member_constraint: {:?} and {:?} are incomparable; no min choice",
+                        min_choice, other_option,
+                    );
+
+                    // If there is no minimum choice, then we *may* wish to use `'static`
+                    // instead, but only if there are no upper bounds, and `'static` is a valid
+                    // choice.
+                    if !any_upper_bounds && choice_regions.contains(&fr_static) {
+                        return self.apply_member_constraint_choice(
+                            scc,
+                            member_constraint_index,
+                            fr_static,
                         );
-                        return false;
                     }
+
+                    // Otherwise, we give up.
+                    return false;
                 }
             }
-            min_choice
-        };
+        }
+        self.apply_member_constraint_choice(scc, member_constraint_index, min_choice)
+    }
 
+    fn apply_member_constraint_choice(
+        &mut self,
+        scc: ConstraintSccIndex,
+        member_constraint_index: NllMemberConstraintIndex,
+        choice: ty::RegionVid,
+    ) -> bool {
         let choice_scc = self.constraint_sccs.scc(choice);
         debug!("apply_member_constraint: min_choice={:?} best_choice_scc={:?}", choice, choice_scc,);
         if self.scc_values.add_region(scc, choice_scc) {

compiler/rustc_borrowck/src/universal_regions.rs

@@ -473,8 +473,9 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
         }
 
         let fr_fn_body = self.infcx.next_nll_region_var(FR).to_region_vid();
-        let num_universals = self.infcx.num_region_vars();
+        debug!("build: fr_fn_Body = {:?}", fr_fn_body);
 
+        let num_universals = self.infcx.num_region_vars();
         debug!("build: global regions = {}..{}", FIRST_GLOBAL_INDEX, first_extern_index);
         debug!("build: extern regions = {}..{}", first_extern_index, first_local_index);
         debug!("build: local regions  = {}..{}", first_local_index, num_universals);

compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs

@@ -303,16 +303,6 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
             self.collect_bounding_regions(graph, member_vid, OUTGOING, None);
         debug!("enforce_member_constraint: upper_bounds={:#?}", member_upper_bounds);
 
-        // If there are no upper bounds, and static is a choice (in practice, it always is),
-        // then we should just pick static.
-        if member_upper_bounds.is_empty()
-            && member_constraint.choice_regions.contains(&&ty::RegionKind::ReStatic)
-        {
-            debug!("enforce_member_constraint: selecting 'static since there are no upper bounds",);
-            *var_values.value_mut(member_vid) = VarValue::Value(self.tcx().lifetimes.re_static);
-            return true;
-        }
-
         // Get an iterator over the *available choice* -- that is,
         // each choice region `c` where `lb <= c` and `c <= ub` for all the
         // upper bounds `ub`.
@@ -326,19 +316,32 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
         // If there is more than one option, we only make a choice if
         // there is a single *least* choice -- i.e., some available
         // region that is `<=` all the others.
-        let mut least_choice: ty::Region<'tcx> = match options.next() {
+        let mut choice: ty::Region<'tcx> = match options.next() {
             Some(&r) => r,
             None => return false,
         };
-        debug!("enforce_member_constraint: least_choice={:?}", least_choice);
+        debug!("enforce_member_constraint: least_choice={:?}", choice);
         for &option in options {
             debug!("enforce_member_constraint: option={:?}", option);
-            if !self.sub_concrete_regions(least_choice, option) {
-                if self.sub_concrete_regions(option, least_choice) {
+            if !self.sub_concrete_regions(choice, option) {
+                if self.sub_concrete_regions(option, choice) {
                     debug!("enforce_member_constraint: new least choice");
-                    least_choice = option;
+                    choice = option;
                 } else {
                     debug!("enforce_member_constraint: no least choice");
+
+                    // Pick static if:
+                    // * There is no least choice (which we just found to be true)
+                    // * There are no upper bounds (so the region can grow to any size)
+                    // * Static is a choice (in practice, it always is)
+                    if member_upper_bounds.is_empty()
+                        && member_constraint.choice_regions.contains(&&ty::RegionKind::ReStatic)
+                    {
+                        choice = self.tcx().lifetimes.re_static;
+                        break;
+                    }
+
+                    // Otherwise give up
                     return false;
                 }
             }
@@ -353,13 +356,10 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {
         // choice is needed here so that we don't end up in a cycle of
         // `expansion` changing the region one way and the code here changing
         // it back.
-        let lub = self.lub_concrete_regions(least_choice, member_lower_bound);
-        debug!(
-            "enforce_member_constraint: final least choice = {:?}\nlub = {:?}",
-            least_choice, lub
-        );
+        let lub = self.lub_concrete_regions(choice, member_lower_bound);
+        debug!("enforce_member_constraint: final choice = {:?}\nlub = {:?}", choice, lub);
         if lub != member_lower_bound {
-            *var_values.value_mut(member_vid) = VarValue::Value(least_choice);
+            *var_values.value_mut(member_vid) = VarValue::Value(choice);
             true
         } else {
             false

src/test/ui/upper-bound-from-type-param.rs

@@ -0,0 +1,19 @@
+// Regression test found by crater run. The scenario here is that we have
+// a region variable `?0` from the `impl Future` with no upper bounds
+// and a member constraint of `?0 in ['a, 'static]`. If we pick `'static`,
+// however, we then fail the check that `F: ?0` (since we only know that
+// F: ?a). The problem here is that our upper bound detection
+// doesn't consider "type tests" like `F: 'x`. This was fixed by
+// preferring to pick a least choice and only using static as a last resort.
+//
+// edition:2018
+// check-pass
+
+trait Future {}
+impl Future for () {}
+
+fn sink_error1<'a, F: 'a>(f: F) -> impl Future + 'a {
+    sink_error2(f) // error: `F` may not live long enough
+}
+fn sink_error2<'a, F: 'a>(_: F) -> impl Future + 'a {}
+fn main() {}