Skip to content

Enforce predicates which link bivariant opaque lifetimes #122311

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions compiler/rustc_infer/src/infer/opaque_types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,15 @@ impl<'tcx> InferCtxt<'tcx> {
ty::ClauseKind::WellFormed(hidden_ty.into()),
));

// This enforces the region obligations that link the bivariant
// lifetimes of an opaque to their invariant copies.
obligations.push(traits::Obligation::new(
tcx,
cause.clone(),
param_env,
ty::ClauseKind::WellFormed(Ty::new_opaque(self.tcx, def_id, args).into()),
));
Comment on lines +615 to +620
Copy link
Contributor

@lcnr lcnr Mar 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this necessary? I'd expect that we require the opaque to be well-formed in the location which introduces it. We have to check the hidden type, as the structural recursion in wf.rs does not check it, so there are types in "relevant positions" which are not wf.

This comment was marked as outdated.

This comment was marked as outdated.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is confusing for me 🤔 I think I now get what's going on:

  • calling test::<'a>() returns Opaque<'a, 'a>, we then assign this to a local of type () which uses subtyping.
  • generalizing Opaque<'a, 'a> results in Opaque<'whatever, 'a> because the first lifetime param is bivariant (would be the same if it were co/contravariant)
  • we then never check that Opaque<'whatever, 'a> is wf, leaving 'whatever unconstrained

That's also not quite right, as even changing Generalizer::regions to always set self.has_unconstrained_ty_var when creating a new region var does not fix this 🤔


let item_bounds = tcx.explicit_item_bounds(def_id);
for (predicate, _) in item_bounds.iter_instantiated_copied(tcx, args) {
let predicate = predicate.fold_with(&mut BottomUpFolder {
Expand Down
9 changes: 2 additions & 7 deletions compiler/rustc_trait_selection/src/traits/wf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -792,13 +792,8 @@ impl<'a, 'tcx> TypeVisitor<TyCtxt<'tcx>> for WfPredicates<'a, 'tcx> {
}

ty::Alias(ty::Opaque, ty::AliasTy { def_id, args, .. }) => {
// All of the requirements on type parameters
// have already been checked for `impl Trait` in
// return position. We do need to check type-alias-impl-trait though.
if self.tcx().is_type_alias_impl_trait(def_id) {
let obligations = self.nominal_obligations(def_id, args);
self.out.extend(obligations);
}
let obligations = self.nominal_obligations(def_id, args);
self.out.extend(obligations);
Comment on lines +795 to +796
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very much in favor of this 😁 wanted to actually open a PR for that myself soon.

Can you merge the behavior of all ty::Alias here?

}

ty::Alias(ty::Weak, ty::AliasTy { def_id, args, .. }) => {
Expand Down
9 changes: 9 additions & 0 deletions tests/ui/impl-trait/bivariant-duplicated-lifetime.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//@ check-pass

#![allow(unconditional_recursion)]

fn test<'a>() -> impl Sized + 'a {
let _: () = test::<'a>();
}

fn main() {}
1 change: 1 addition & 0 deletions tests/ui/impl-trait/in-trait/return-dont-satisfy-bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ impl Foo<char> for Bar {
//~^ ERROR: the trait bound `impl Foo<u8>: Foo<char>` is not satisfied [E0277]
//~| ERROR: the trait bound `Bar: Foo<u8>` is not satisfied [E0277]
//~| ERROR: impl has stricter requirements than trait
//~| ERROR: the trait bound `F2: Foo<u8>` is not satisfied
self
}
}
Expand Down
18 changes: 17 additions & 1 deletion tests/ui/impl-trait/in-trait/return-dont-satisfy-bounds.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,22 @@ note: required by a bound in `Foo::{synthetic#0}`
LL | fn foo<F2>(self) -> impl Foo<T>;
| ^^^^^^ required by this bound in `Foo::{synthetic#0}`

error[E0277]: the trait bound `F2: Foo<u8>` is not satisfied
--> $DIR/return-dont-satisfy-bounds.rs:8:34
|
LL | fn foo<F2: Foo<u8>>(self) -> impl Foo<u8> {
| ^^^^^^^^^^^^ the trait `Foo<u8>` is not implemented for `F2`
|
note: required by a bound in `<Bar as Foo<char>>::foo`
--> $DIR/return-dont-satisfy-bounds.rs:8:16
|
LL | fn foo<F2: Foo<u8>>(self) -> impl Foo<u8> {
| ^^^^^^^ required by this bound in `<Bar as Foo<char>>::foo`
help: consider further restricting this bound
|
LL | fn foo<F2: Foo<u8> + Foo<u8>>(self) -> impl Foo<u8> {
| +++++++++

error[E0276]: impl has stricter requirements than trait
--> $DIR/return-dont-satisfy-bounds.rs:8:16
|
Expand All @@ -32,7 +48,7 @@ LL | self
= help: the trait `Foo<char>` is implemented for `Bar`
= help: for that trait implementation, expected `char`, found `u8`

error: aborting due to 3 previous errors
error: aborting due to 4 previous errors

Some errors have detailed explanations: E0276, E0277.
For more information about an error, try `rustc --explain E0276`.
1 change: 1 addition & 0 deletions tests/ui/lifetimes/issue-76168-hr-outlives-3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ async fn wrapper<F>(f: F)
//~^ ERROR: expected a `FnOnce(&'a mut i32)` closure, found `i32`
//~| ERROR: expected a `FnOnce(&'a mut i32)` closure, found `i32`
//~| ERROR: expected a `FnOnce(&'a mut i32)` closure, found `i32`
//~| ERROR: expected a `FnOnce(&'a mut i32)` closure, found `i32`
where
F:,
for<'a> <i32 as FnOnce<(&'a mut i32,)>>::Output: Future<Output = ()> + 'a,
Expand Down
23 changes: 19 additions & 4 deletions tests/ui/lifetimes/issue-76168-hr-outlives-3.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ LL | / async fn wrapper<F>(f: F)
LL | |
LL | |
LL | |
LL | | where
... |
LL | | F:,
LL | | for<'a> <i32 as FnOnce<(&'a mut i32,)>>::Output: Future<Output = ()> + 'a,
| |__________________________________________________________________________^ expected an `FnOnce(&'a mut i32)` closure, found `i32`
Expand All @@ -27,15 +27,30 @@ LL | / async fn wrapper<F>(f: F)
LL | |
LL | |
LL | |
LL | | where
... |
LL | | F:,
LL | | for<'a> <i32 as FnOnce<(&'a mut i32,)>>::Output: Future<Output = ()> + 'a,
| |__________________________________________________________________________^ expected an `FnOnce(&'a mut i32)` closure, found `i32`
|
= help: the trait `for<'a> FnOnce<(&'a mut i32,)>` is not implemented for `i32`

error[E0277]: expected a `FnOnce(&'a mut i32)` closure, found `i32`
--> $DIR/issue-76168-hr-outlives-3.rs:13:1
--> $DIR/issue-76168-hr-outlives-3.rs:6:1
|
LL | / async fn wrapper<F>(f: F)
LL | |
LL | |
LL | |
... |
LL | | F:,
LL | | for<'a> <i32 as FnOnce<(&'a mut i32,)>>::Output: Future<Output = ()> + 'a,
| |__________________________________________________________________________^ expected an `FnOnce(&'a mut i32)` closure, found `i32`
|
= help: the trait `for<'a> FnOnce<(&'a mut i32,)>` is not implemented for `i32`
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error[E0277]: expected a `FnOnce(&'a mut i32)` closure, found `i32`
--> $DIR/issue-76168-hr-outlives-3.rs:14:1
|
LL | / {
LL | |
Expand All @@ -46,6 +61,6 @@ LL | | }
|
= help: the trait `for<'a> FnOnce<(&'a mut i32,)>` is not implemented for `i32`

error: aborting due to 4 previous errors
error: aborting due to 5 previous errors

For more information about this error, try `rustc --explain E0277`.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,27 @@ LL | #![feature(non_lifetime_binders)]
= note: see issue #108185 <https://github.com/rust-lang/rust/issues/108185> for more information
= note: `#[warn(incomplete_features)]` on by default

error[E0309]: the placeholder type `!1_"F"` may not live long enough
--> $DIR/type-match-with-late-bound.rs:8:1
|
LL | async fn walk2<'a, T: 'a>(_: T)
| ^ -- the placeholder type `!1_"F"` must be valid for the lifetime `'a` as defined here...
| _|
| |
LL | | where
LL | | for<F> F: 'a,
| |_________________^ ...so that the type `F` will meet its required lifetime bounds...
|
note: ...that is required by this bound
--> $DIR/type-match-with-late-bound.rs:10:15
|
LL | for<F> F: 'a,
| ^^
help: consider adding an explicit lifetime bound
|
LL | for<F> F: 'a, !1_"F": 'a
| ~~~~~~~~~~~~

error[E0309]: the placeholder type `!1_"F"` may not live long enough
--> $DIR/type-match-with-late-bound.rs:11:1
|
Expand Down Expand Up @@ -35,6 +56,20 @@ help: consider adding an explicit lifetime bound
LL | for<F> F: 'a, !2_"F": 'a
| ~~~~~~~~~~~~

error: aborting due to 2 previous errors; 1 warning emitted
error[E0309]: the placeholder type `!3_"F"` may not live long enough
--> $DIR/type-match-with-late-bound.rs:11:1
|
LL | async fn walk2<'a, T: 'a>(_: T)
| -- the placeholder type `!3_"F"` must be valid for the lifetime `'a` as defined here...
...
LL | {}
| ^^ ...so that the type `F` will meet its required lifetime bounds
|
help: consider adding an explicit lifetime bound
|
LL | for<F> F: 'a, !3_"F": 'a
| ~~~~~~~~~~~~

error: aborting due to 4 previous errors; 1 warning emitted

For more information about this error, try `rustc --explain E0309`.
12 changes: 12 additions & 0 deletions tests/ui/type-alias-impl-trait/bivariant-duplicated-lifetime.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//@ check-pass

#![feature(type_alias_impl_trait)]
#![allow(unconditional_recursion)]

type Opaque<'a> = impl Sized + 'a;

fn test<'a>() -> Opaque<'a> {
let _: () = test::<'a>();
}

fn main() {}