diff --git a/compiler/rustc_trait_selection/src/traits/wf.rs b/compiler/rustc_trait_selection/src/traits/wf.rs index 635cdde0e8eec..2348d996bcfea 100644 --- a/compiler/rustc_trait_selection/src/traits/wf.rs +++ b/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>, ) -> Vec> { - 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 } } } }) diff --git a/src/test/ui/wf/higher-ranked/ignored-trait-bound.rs b/src/test/ui/wf/higher-ranked/ignored-trait-bound.rs new file mode 100644 index 0000000000000..efb4e20dd3149 --- /dev/null +++ b/src/test/ui/wf/higher-ranked/ignored-trait-bound.rs @@ -0,0 +1,19 @@ +// check-pass +struct NeedsCopy(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; +} diff --git a/src/test/ui/wf/higher-ranked/on-impl-not-implied.rs b/src/test/ui/wf/higher-ranked/on-impl-not-implied.rs new file mode 100644 index 0000000000000..bff11c4c977a4 --- /dev/null +++ b/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); + +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() {} diff --git a/src/test/ui/wf/higher-ranked/on-impl-not-implied.stderr b/src/test/ui/wf/higher-ranked/on-impl-not-implied.stderr new file mode 100644 index 0000000000000..d08e5b4e344a6 --- /dev/null +++ b/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`. diff --git a/src/test/ui/wf/higher-ranked/on-impl.rs b/src/test/ui/wf/higher-ranked/on-impl.rs new file mode 100644 index 0000000000000..6468e4da2e71c --- /dev/null +++ b/src/test/ui/wf/higher-ranked/on-impl.rs @@ -0,0 +1,21 @@ +// check-pass +trait Foo {} +impl<'a, T: Foo, A> Foo 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 Foo<&'a T>>(_: F) {} +fn oh_no 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::(&mut f); +} + +fn main() {}