Auto merge of #97406 - aliemjay:verify-bounds-fix-master, r=oli-obk

Make outlives::{components,verify} agree

fixes #97405

cc `@oli-obk` this is should fix #95474 (comment)

r? `@oli-obk`

Author: bors

compiler/rustc_infer/src/infer/outlives/components.rs

@@ -190,7 +190,11 @@ fn compute_components<'tcx>(
         }
 }
 
-fn compute_components_recursive<'tcx>(
+/// Collect [Component]s for *all* the substs of `parent`.
+///
+/// This should not be used to get the components of `parent` itself.
+/// Use [push_outlives_components] instead.
+pub(super) fn compute_components_recursive<'tcx>(
     tcx: TyCtxt<'tcx>,
     parent: GenericArg<'tcx>,
     out: &mut SmallVec<[Component<'tcx>; 4]>,

compiler/rustc_infer/src/infer/outlives/verify.rs

@@ -1,12 +1,15 @@
+use crate::infer::outlives::components::{compute_components_recursive, Component};
 use crate::infer::outlives::env::RegionBoundPairs;
 use crate::infer::region_constraints::VerifyIfEq;
 use crate::infer::{GenericKind, VerifyBound};
 use rustc_data_structures::captures::Captures;
 use rustc_data_structures::sso::SsoHashSet;
 use rustc_hir::def_id::DefId;
-use rustc_middle::ty::subst::{GenericArg, GenericArgKind, Subst};
+use rustc_middle::ty::subst::{GenericArg, Subst};
 use rustc_middle::ty::{self, EarlyBinder, Ty, TyCtxt};
 
+use smallvec::smallvec;
+
 /// The `TypeOutlives` struct has the job of "lowering" a `T: 'a`
 /// obligation into a series of `'a: 'b` constraints and "verifys", as
 /// described on the module comment. The final constraints are emitted
@@ -47,43 +50,6 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {
         }
     }
 
-    fn type_bound(
-        &self,
-        ty: Ty<'tcx>,
-        visited: &mut SsoHashSet<GenericArg<'tcx>>,
-    ) -> VerifyBound<'tcx> {
-        match *ty.kind() {
-            ty::Param(p) => self.param_bound(p),
-            ty::Projection(data) => self.projection_bound(data, visited),
-            ty::FnDef(_, substs) => {
-                // HACK(eddyb) ignore lifetimes found shallowly in `substs`.
-                // This is inconsistent with `ty::Adt` (including all substs),
-                // but consistent with previous (accidental) behavior.
-                // See https://github.com/rust-lang/rust/issues/70917
-                // for further background and discussion.
-                let mut bounds = substs
-                    .iter()
-                    .filter_map(|child| match child.unpack() {
-                        GenericArgKind::Type(ty) => Some(self.type_bound(ty, visited)),
-                        GenericArgKind::Lifetime(_) => None,
-                        GenericArgKind::Const(_) => Some(self.recursive_bound(child, visited)),
-                    })
-                    .filter(|bound| {
-                        // Remove bounds that must hold, since they are not interesting.
-                        !bound.must_hold()
-                    });
-
-                match (bounds.next(), bounds.next()) {
-                    (Some(first), None) => first,
-                    (first, second) => VerifyBound::AllBounds(
-                        first.into_iter().chain(second).chain(bounds).collect(),
-                    ),
-                }
-            }
-            _ => self.recursive_bound(ty.into(), visited),
-        }
-    }
-
     fn param_bound(&self, param_ty: ty::ParamTy) -> VerifyBound<'tcx> {
         debug!("param_bound(param_ty={:?})", param_ty);
 
@@ -188,27 +154,24 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {
             .map(|r| VerifyBound::OutlivedBy(r));
 
         // see the extensive comment in projection_must_outlive
-        let ty = self.tcx.mk_projection(projection_ty.item_def_id, projection_ty.substs);
-        let recursive_bound = self.recursive_bound(ty.into(), visited);
+        let recursive_bound = {
+            let mut components = smallvec![];
+            let ty = self.tcx.mk_projection(projection_ty.item_def_id, projection_ty.substs);
+            compute_components_recursive(self.tcx, ty.into(), &mut components, visited);
+            self.bound_from_components(&components, visited)
+        };
 
         VerifyBound::AnyBound(env_bounds.chain(trait_bounds).collect()).or(recursive_bound)
     }
 
-    fn recursive_bound(
+    fn bound_from_components(
         &self,
-        parent: GenericArg<'tcx>,
+        components: &[Component<'tcx>],
         visited: &mut SsoHashSet<GenericArg<'tcx>>,
     ) -> VerifyBound<'tcx> {
-        let mut bounds = parent
-            .walk_shallow(visited)
-            .filter_map(|child| match child.unpack() {
-                GenericArgKind::Type(ty) => Some(self.type_bound(ty, visited)),
-                GenericArgKind::Lifetime(lt) => {
-                    // Ignore late-bound regions.
-                    if !lt.is_late_bound() { Some(VerifyBound::OutlivedBy(lt)) } else { None }
-                }
-                GenericArgKind::Const(_) => Some(self.recursive_bound(child, visited)),
-            })
+        let mut bounds = components
+            .iter()
+            .map(|component| self.bound_from_single_component(component, visited))
             .filter(|bound| {
                 // Remove bounds that must hold, since they are not interesting.
                 !bound.must_hold()
@@ -222,6 +185,32 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {
         }
     }
 
+    fn bound_from_single_component(
+        &self,
+        component: &Component<'tcx>,
+        visited: &mut SsoHashSet<GenericArg<'tcx>>,
+    ) -> VerifyBound<'tcx> {
+        match *component {
+            Component::Region(lt) => VerifyBound::OutlivedBy(lt),
+            Component::Param(param_ty) => self.param_bound(param_ty),
+            Component::Projection(projection_ty) => self.projection_bound(projection_ty, visited),
+            Component::EscapingProjection(ref components) => {
+                self.bound_from_components(components, visited)
+            }
+            Component::UnresolvedInferenceVariable(v) => {
+                // ignore this, we presume it will yield an error
+                // later, since if a type variable is not resolved by
+                // this point it never will be
+                self.tcx.sess.delay_span_bug(
+                    rustc_span::DUMMY_SP,
+                    &format!("unresolved inference variable in outlives: {:?}", v),
+                );
+                // add a bound that never holds
+                VerifyBound::AnyBound(vec![])
+            }
+        }
+    }
+
     /// Searches the environment for where-clauses like `G: 'a` where
     /// `G` is either some type parameter `T` or a projection like
     /// `T::Item`. Returns a vector of the `'a` bounds it can find.

src/test/ui/regions/closure-in-projection-issue-97405.rs

@@ -0,0 +1,32 @@
+// Regression test for #97405.
+// In `good_generic_fn` the param `T` ends up in the substs of closures/generators,
+// but we should be able to prove `<Gen<T> as Iterator>::Item: 'static` without
+// requiring `T: 'static`
+
+// edition:2018
+// check-fail
+
+fn opaque<F>(_: F) -> impl Iterator { b"".iter() }
+
+fn assert_static<T: 'static>(_: T) {}
+
+fn good_generic_fn<T>() {
+    // Previously, proving `<OpaqueTy<type_of(async {})> as Iterator>::Item: 'static`
+    // used to require `T: 'static`.
+    assert_static(opaque(async {}).next());
+    assert_static(opaque(|| {}).next());
+    assert_static(opaque(opaque(async {}).next()).next());
+}
+
+
+// This should fail because `T` ends up in the upvars of the closure.
+fn bad_generic_fn<T: Copy>(t: T) {
+    assert_static(opaque(async move { t; }).next());
+    //~^ ERROR the associated type `<impl Iterator as Iterator>::Item` may not live long enough
+    assert_static(opaque(move || { t; }).next());
+    //~^ ERROR the associated type `<impl Iterator as Iterator>::Item` may not live long enough
+    assert_static(opaque(opaque(async move { t; }).next()).next());
+    //~^ ERROR the associated type `<impl Iterator as Iterator>::Item` may not live long enough
+}
+
+fn main() {}

src/test/ui/regions/closure-in-projection-issue-97405.stderr

@@ -0,0 +1,30 @@
+error[E0310]: the associated type `<impl Iterator as Iterator>::Item` may not live long enough
+  --> $DIR/closure-in-projection-issue-97405.rs:24:5
+   |
+LL |     assert_static(opaque(async move { t; }).next());
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider adding an explicit lifetime bound `<impl Iterator as Iterator>::Item: 'static`...
+   = note: ...so that the type `<impl Iterator as Iterator>::Item` will meet its required lifetime bounds
+
+error[E0310]: the associated type `<impl Iterator as Iterator>::Item` may not live long enough
+  --> $DIR/closure-in-projection-issue-97405.rs:26:5
+   |
+LL |     assert_static(opaque(move || { t; }).next());
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider adding an explicit lifetime bound `<impl Iterator as Iterator>::Item: 'static`...
+   = note: ...so that the type `<impl Iterator as Iterator>::Item` will meet its required lifetime bounds
+
+error[E0310]: the associated type `<impl Iterator as Iterator>::Item` may not live long enough
+  --> $DIR/closure-in-projection-issue-97405.rs:28:5
+   |
+LL |     assert_static(opaque(opaque(async move { t; }).next()).next());
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider adding an explicit lifetime bound `<impl Iterator as Iterator>::Item: 'static`...
+   = note: ...so that the type `<impl Iterator as Iterator>::Item` will meet its required lifetime bounds
+
+error: aborting due to 3 previous errors
+
+For more information about this error, try `rustc --explain E0310`.