wf: discard nested obligations with placeholders

compiler/rustc_trait_selection/src/traits/wf.rs

@@ -168,6 +168,23 @@ pub fn predicate_obligations<'tcx>(
     wf.normalize(infcx)
 }
 
+/// Whether we should emit nested obligations containing `value`.
+/// This is a hack to deal with higher ranked types as we require implications
+/// in binders for a more correct implementation.
+///
+/// `WF(for<'a> fn(&'a T))` would otherwise emit a `T: 'a` bound for `&'a T`
+/// which would require `T: 'static`. What we actually want here is the type
+/// `for<'a> where { T: 'a } fn(&'a T)`, at which point the `T: 'a` bound is
+/// trivially implied from the `param_env`.
+///
+/// This means that we WF bounds aren't always sufficiently checked for now.
+fn emit_obligations_for<'tcx, T: TypeVisitable<'tcx>>(value: T) -> bool {
+    // We also don't emit obligations for placeholders. This will be incorrect
+    // if we ever decide to also replace `ty::Param` with placeholders during
+    // canonicalization or whatever. Hopefully we will have implications by then.
+    !value.has_escaping_bound_vars() && !value.has_placeholders()
+}
+
 struct WfPredicates<'tcx> {
     tcx: TyCtxt<'tcx>,
     param_env: ty::ParamEnv<'tcx>,
@@ -280,7 +297,7 @@ impl<'tcx> WfPredicates<'tcx> {
         let param_env = self.param_env;
         let mut obligations = Vec::with_capacity(self.out.len());
         for mut obligation in self.out {
-            assert!(!obligation.has_escaping_bound_vars());
+            assert!(emit_obligations_for(obligation.predicate));
             let mut selcx = traits::SelectionContext::new(infcx);
             // Don't normalize the whole obligation, the param env is either
             // already normalized, or we're currently normalizing the
@@ -373,7 +390,7 @@ impl<'tcx> WfPredicates<'tcx> {
                 .filter(|(_, arg)| {
                     matches!(arg.unpack(), GenericArgKind::Type(..) | GenericArgKind::Const(..))
                 })
-                .filter(|(_, arg)| !arg.has_escaping_bound_vars())
+                .filter(|&(_, arg)| emit_obligations_for(arg))
                 .map(|(i, arg)| {
                     let mut cause = traits::ObligationCause::misc(self.span, self.body_id);
                     // The first subst is the self ty - use the correct span for it.
@@ -433,7 +450,7 @@ impl<'tcx> WfPredicates<'tcx> {
                 .filter(|arg| {
                     matches!(arg.unpack(), GenericArgKind::Type(..) | GenericArgKind::Const(..))
                 })
-                .filter(|arg| !arg.has_escaping_bound_vars())
+                .filter(|&arg| emit_obligations_for(arg))
                 .map(|arg| {
                     traits::Obligation::with_depth(
                         cause.clone(),
@@ -446,7 +463,7 @@ impl<'tcx> WfPredicates<'tcx> {
     }
 
     fn require_sized(&mut self, subty: Ty<'tcx>, cause: traits::ObligationCauseCode<'tcx>) {
-        if !subty.has_escaping_bound_vars() {
+        if emit_obligations_for(subty) {
             let cause = self.cause(cause);
             let trait_ref = ty::TraitRef {
                 def_id: self.tcx.require_lang_item(LangItem::Sized, None),
@@ -582,7 +599,7 @@ impl<'tcx> WfPredicates<'tcx> {
 
                 ty::Ref(r, rty, _) => {
                     // WfReference
-                    if !r.has_escaping_bound_vars() && !rty.has_escaping_bound_vars() {
+                    if emit_obligations_for(r) && emit_obligations_for(rty) {
                         let cause = self.cause(traits::ReferenceOutlivesReferent(ty));
                         self.out.push(traits::Obligation::with_depth(
                             cause,
@@ -755,7 +772,7 @@ impl<'tcx> WfPredicates<'tcx> {
                 }
                 traits::Obligation::with_depth(cause, self.recursion_depth, self.param_env, pred)
             })
-            .filter(|pred| !pred.has_escaping_bound_vars())
+            .filter(|obl| emit_obligations_for(obl.predicate))
             .collect()
     }
 
@@ -812,7 +829,7 @@ impl<'tcx> WfPredicates<'tcx> {
         //
         // Note: in fact we only permit builtin traits, not `Bar<'d>`, I
         // am looking forward to the future here.
-        if !data.has_escaping_bound_vars() && !region.has_escaping_bound_vars() {
+        if emit_obligations_for(data) && emit_obligations_for(region) {
             let implicit_bounds = object_region_bounds(self.tcx, data);
 
             let explicit_bound = region;
@@ -876,12 +893,12 @@ pub fn object_region_bounds<'tcx>(
 /// Requires that trait definitions have been processed so that we can
 /// elaborate predicates and walk supertraits.
 #[instrument(skip(tcx, predicates), level = "debug", ret)]
-pub(crate) fn required_region_bounds<'tcx>(
+fn required_region_bounds<'tcx>(
     tcx: TyCtxt<'tcx>,
     erased_self_ty: Ty<'tcx>,
     predicates: impl Iterator<Item = ty::Predicate<'tcx>>,
 ) -> Vec<ty::Region<'tcx>> {
-    assert!(!erased_self_ty.has_escaping_bound_vars());
+    assert!(emit_obligations_for(erased_self_ty));
 
     traits::elaborate_predicates(tcx, predicates)
         .filter_map(|obligation| {
@@ -898,7 +915,7 @@ pub(crate) fn required_region_bounds<'tcx>(
                 | ty::PredicateKind::ConstEvaluatable(..)
                 | ty::PredicateKind::ConstEquate(..)
                 | ty::PredicateKind::TypeWellFormedFromEnv(..) => None,
-                ty::PredicateKind::TypeOutlives(ty::OutlivesPredicate(ref t, ref r)) => {
+                ty::PredicateKind::TypeOutlives(ty::OutlivesPredicate(t, r)) => {
                     // Search for a bound of the form `erased_self_ty
                     // : 'a`, but be wary of something like `for<'a>
                     // erased_self_ty : 'a` (we interpret a
@@ -908,11 +925,7 @@ pub(crate) fn required_region_bounds<'tcx>(
                     // it's kind of a moot point since you could never
                     // construct such an object, but this seems
                     // correct even if that code changes).
-                    if t == &erased_self_ty && !r.has_escaping_bound_vars() {
-                        Some(*r)
-                    } else {
-                        None
-                    }
+                    if t == erased_self_ty && emit_obligations_for(r) { Some(r) } else { None }
                 }
             }
         })

src/test/ui/wf/higher-ranked/ignored-trait-bound.rs

@@ -0,0 +1,19 @@
+// check-pass
+struct NeedsCopy<T: Copy>(T);
+
+// Skips WF because of an escaping bound region.
+struct HasWfHigherRanked
+where
+    (for<'a> fn(NeedsCopy<&'a mut u32>)):,
+{}
+
+// Skips WF because of a placeholder region.
+struct HasWfPlaceholder
+where
+    for<'a> NeedsCopy<&'a mut u32>:,
+{}
+
+fn main() {
+    let _: HasWfHigherRanked;
+    let _: HasWfPlaceholder;
+}

src/test/ui/wf/higher-ranked/on-impl-not-implied.rs

@@ -0,0 +1,17 @@
+// Checks that manual WF bounds are not use for implied bounds.
+//
+// With the current implementation for WF check, this can easily cause
+// unsoundness, as wf does not emit any obligations containing
+// placeholders or bound variables.
+struct MyStruct<T, U>(T, U);
+
+trait Foo<'a, 'b> {}
+
+// IF THIS TEST STOPS EMITTING ERRORS, PLEASE NOTIFY T-TYPES TO CHECK WHETHER THE CHANGE IS SOUND.
+impl<'a, 'b> Foo<'a, 'b> for () //~ ERROR cannot infer an appropriate lifetime
+where
+    &'a &'b ():,
+    //~^ ERROR in type `&'a &'b ()`, reference has a longer lifetime than the data it references
+{}
+
+fn main() {}

src/test/ui/wf/higher-ranked/on-impl-not-implied.stderr

@@ -0,0 +1,45 @@
+error[E0495]: cannot infer an appropriate lifetime for lifetime parameter `'b` due to conflicting requirements
+  --> $DIR/on-impl-not-implied.rs:11:14
+   |
+LL | impl<'a, 'b> Foo<'a, 'b> for ()
+   |              ^^^^^^^^^^^
+   |
+note: first, the lifetime cannot outlive the lifetime `'b` as defined here...
+  --> $DIR/on-impl-not-implied.rs:11:10
+   |
+LL | impl<'a, 'b> Foo<'a, 'b> for ()
+   |          ^^
+note: ...but the lifetime must also be valid for the lifetime `'a` as defined here...
+  --> $DIR/on-impl-not-implied.rs:11:6
+   |
+LL | impl<'a, 'b> Foo<'a, 'b> for ()
+   |      ^^
+note: ...so that the types are compatible
+  --> $DIR/on-impl-not-implied.rs:11:14
+   |
+LL | impl<'a, 'b> Foo<'a, 'b> for ()
+   |              ^^^^^^^^^^^
+   = note: expected `Foo<'a, 'b>`
+              found `Foo<'_, '_>`
+
+error[E0491]: in type `&'a &'b ()`, reference has a longer lifetime than the data it references
+  --> $DIR/on-impl-not-implied.rs:13:5
+   |
+LL |     &'a &'b ():,
+   |     ^^^^^^^^^^
+   |
+note: the pointer is valid for the lifetime `'a` as defined here
+  --> $DIR/on-impl-not-implied.rs:11:6
+   |
+LL | impl<'a, 'b> Foo<'a, 'b> for ()
+   |      ^^
+note: but the referenced data is only valid for the lifetime `'b` as defined here
+  --> $DIR/on-impl-not-implied.rs:11:10
+   |
+LL | impl<'a, 'b> Foo<'a, 'b> for ()
+   |          ^^
+
+error: aborting due to 2 previous errors
+
+Some errors have detailed explanations: E0491, E0495.
+For more information about an error, try `rustc --explain E0491`.

src/test/ui/wf/higher-ranked/on-impl.rs

@@ -0,0 +1,21 @@
+// check-pass
+trait Foo<A> {}
+impl<'a, T: Foo<A>, A> Foo<A> for &'a mut T
+where
+    // Needed to use `(A,)` because `A:` by itself doesn't emit a WF bound
+    // as of writing this comment.
+    //
+    // This happens in `fn explicit_predicates_of`.
+    (A,):,
+{}
+
+fn tragic<T, F: for<'a> Foo<&'a T>>(_: F) {}
+fn oh_no<T, F: for<'a> Foo<&'a T>>(mut f: F) {
+    // This results in a `for<'a> WF(&'a T)` bound where `'a` is replaced
+    // with a placeholder before we compute the wf requirements.
+    //
+    // This bound would otherwise result in a `T: 'static` bound.
+    tragic::<T, _>(&mut f);
+}
+
+fn main() {}