Skip to content

Commit

Permalink
Auto merge of #112842 - lcnr:non-defining-use, r=oli-obk
Browse files Browse the repository at this point in the history
check for non-defining uses of RPIT

This PR requires defining uses of RPIT and the async functions return type to use unique generic parameters as type and const arguments, (mostly) fixing #111935. This changes the following snippet to an error (it compiled since 1.62):
```rust
fn foo<T>() -> impl Sized {
    let _: () = foo::<u8>(); //~ ERROR non-defining use of `impl Sized`
}
```
Since 1.62 we only checked that the generic arguments of opaque types are unique parameters for TAIT and ignored RPITs, so this PR changes the behavior here to be consistent.

For defining uses which do not have unique params as arguments it is unclear how the hidden type should map to the generic params of the opaque. In the following snippet, should the hidden type of `foo<T>::opaque` be `T` or `u32`.
```rust
fn foo<T>() -> impl Sized {
    let _: u32 = foo::<u32>();
    foo::<T>()
}
```
There are no crater regressions caused by this change.

---

The same issue exists for lifetime arguments which is not fixed by this PR, currently resulting in an ICE in mir borrowck (I wasn't able to get an example which didn't ICE, it might be possible):
```rust
fn foo<'a: 'a>() -> impl Sized {
    let _: &'static () = foo::<'static>();
    //~^ ICE opaque type with non-universal region substs
    foo::<'a>()
}
```
Fixing this for lifetimes as well is blocked on #113916. Due to this issue, functions returning an RPIT with lifetime parameters equal in the region constraint graph would always result in an error, resulting in breakage found via crater: #112842 (comment)
```rust
trait Trait<'a, 'b> {}
impl Trait<'_, '_> for () {}

struct Type<'a>(&'a ());
impl<'a> Type<'a> {
    // `'b == 'a`
    fn do_stuff<'b: 'a>(&'b self) -> impl Trait<'a, 'b> {
        // This fails as long there is something in the body
        // which adds the outlives constraints to the constraint graph.
        //
        // This is the case for nested closures.
        (|| ())()

    }
}
```
  • Loading branch information
bors committed Aug 14, 2023
2 parents 3071e0a + e04f582 commit e845910
Show file tree
Hide file tree
Showing 8 changed files with 109 additions and 37 deletions.
41 changes: 14 additions & 27 deletions compiler/rustc_borrowck/src/region_infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,40 +371,27 @@ fn check_opaque_type_parameter_valid(
span: Span,
) -> Result<(), ErrorGuaranteed> {
let opaque_ty_hir = tcx.hir().expect_item(opaque_type_key.def_id);
match opaque_ty_hir.expect_opaque_ty().origin {
// No need to check return position impl trait (RPIT)
// because for type and const parameters they are correct
// by construction: we convert
//
// fn foo<P0..Pn>() -> impl Trait
//
// into
//
// type Foo<P0...Pn>
// fn foo<P0..Pn>() -> Foo<P0...Pn>.
//
// For lifetime parameters we convert
//
// fn foo<'l0..'ln>() -> impl Trait<'l0..'lm>
//
// into
//
// type foo::<'p0..'pn>::Foo<'q0..'qm>
// fn foo<l0..'ln>() -> foo::<'static..'static>::Foo<'l0..'lm>.
//
// which would error here on all of the `'static` args.
OpaqueTyOrigin::FnReturn(..) | OpaqueTyOrigin::AsyncFn(..) => return Ok(()),
// Check these
OpaqueTyOrigin::TyAlias { .. } => {}
}
let is_ty_alias = match opaque_ty_hir.expect_opaque_ty().origin {
OpaqueTyOrigin::TyAlias { .. } => true,
OpaqueTyOrigin::AsyncFn(..) | OpaqueTyOrigin::FnReturn(..) => false,
};

let opaque_generics = tcx.generics_of(opaque_type_key.def_id);
let mut seen_params: FxIndexMap<_, Vec<_>> = FxIndexMap::default();
for (i, arg) in opaque_type_key.args.iter().enumerate() {
if let Err(guar) = arg.error_reported() {
return Err(guar);
}

let arg_is_param = match arg.unpack() {
GenericArgKind::Type(ty) => matches!(ty.kind(), ty::Param(_)),
GenericArgKind::Lifetime(lt) => {
GenericArgKind::Lifetime(lt) if is_ty_alias => {
matches!(*lt, ty::ReEarlyBound(_) | ty::ReFree(_))
}
// FIXME(#113916): we can't currently check for unique lifetime params,
// see that issue for more. We will also have to ignore unused lifetime
// params for RPIT, but that's comparatively trivial ✨
GenericArgKind::Lifetime(_) => continue,
GenericArgKind::Const(ct) => matches!(ct.kind(), ty::ConstKind::Param(_)),
};

Expand Down
1 change: 1 addition & 0 deletions tests/ui/impl-trait/issue-99073-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ fn test<T: Display>(t: T, recurse: bool) -> impl Display {
let f = || {
let i: u32 = test::<i32>(-1, false);
//~^ ERROR concrete type differs from previous defining opaque type use
//~| ERROR expected generic type parameter, found `i32`
println!("{i}");
};
if recurse {
Expand Down
18 changes: 14 additions & 4 deletions tests/ui/impl-trait/issue-99073-2.stderr
Original file line number Diff line number Diff line change
@@ -1,14 +1,24 @@
error[E0792]: expected generic type parameter, found `i32`
--> $DIR/issue-99073-2.rs:9:22
|
LL | fn test<T: Display>(t: T, recurse: bool) -> impl Display {
| - this generic parameter must be used with a generic type parameter
LL | let f = || {
LL | let i: u32 = test::<i32>(-1, false);
| ^^^^^^^^^^^^^^^^^^^^^^

error: concrete type differs from previous defining opaque type use
--> $DIR/issue-99073-2.rs:9:22
|
LL | let i: u32 = test::<i32>(-1, false);
| ^^^^^^^^^^^^^^^^^^^^^^ expected `T`, got `u32`
|
note: previous use here
--> $DIR/issue-99073-2.rs:16:5
--> $DIR/issue-99073-2.rs:7:45
|
LL | t
| ^
LL | fn test<T: Display>(t: T, recurse: bool) -> impl Display {
| ^^^^^^^^^^^^

error: aborting due to previous error
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0792`.
1 change: 1 addition & 0 deletions tests/ui/impl-trait/issue-99073.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ fn main() {
fn fix<F: Fn(G), G: Fn()>(f: F) -> impl Fn() {
move || f(fix(&f))
//~^ ERROR concrete type differs from previous defining opaque type use
//~| ERROR expected generic type parameter, found `&F`
}
21 changes: 15 additions & 6 deletions tests/ui/impl-trait/issue-99073.stderr
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
error: concrete type differs from previous defining opaque type use
error[E0792]: expected generic type parameter, found `&F`
--> $DIR/issue-99073.rs:6:11
|
LL | fn fix<F: Fn(G), G: Fn()>(f: F) -> impl Fn() {
| - this generic parameter must be used with a generic type parameter
LL | move || f(fix(&f))
| ^^^^^^^^^^

error: concrete type differs from previous defining opaque type use
--> $DIR/issue-99073.rs:6:13
|
LL | move || f(fix(&f))
| ^^^^^^^^^^ expected `[closure@$DIR/issue-99073.rs:6:3: 6:10]`, got `G`
| ^^^^^^^ expected `[closure@$DIR/issue-99073.rs:6:3: 6:10]`, got `G`
|
note: previous use here
--> $DIR/issue-99073.rs:6:3
--> $DIR/issue-99073.rs:5:36
|
LL | move || f(fix(&f))
| ^^^^^^^^^^^^^^^^^^
LL | fn fix<F: Fn(G), G: Fn()>(f: F) -> impl Fn() {
| ^^^^^^^^^

error: aborting due to previous error
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0792`.
19 changes: 19 additions & 0 deletions tests/ui/impl-trait/rpit/equal-lifetime-params-ok.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// check-pass

// related to #113916, check that using RPITs in functions with lifetime params
// which are constrained to be equal compiles.

trait Trait<'a, 'b> {}
impl Trait<'_, '_> for () {}
fn pass<'a: 'b, 'b: 'a>() -> impl Trait<'a, 'b> {
(|| {})()
}

struct Foo<'a>(&'a ());
impl<'a> Foo<'a> {
fn bar<'b: 'a>(&'b self) -> impl Trait<'a, 'b> {
let _: &'a &'b &'a ();
}
}

fn main() {}
14 changes: 14 additions & 0 deletions tests/ui/impl-trait/rpit/non-defining-use.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Regression test for #111935 that non-defining uses of RPIT result in errors
#![allow(unconditional_recursion)]
fn foo<T>() -> impl Sized {
let _: () = foo::<u8>(); //~ ERROR expected generic type parameter, found `u8`
}

fn bar<T>(val: T) -> impl Sized {
let _: u8 = bar(0u8);
//~^ ERROR concrete type differs from previous defining opaque type use
//~| ERROR expected generic type parameter, found `u8`
val
}

fn main() {}
31 changes: 31 additions & 0 deletions tests/ui/impl-trait/rpit/non-defining-use.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
error[E0792]: expected generic type parameter, found `u8`
--> $DIR/non-defining-use.rs:4:12
|
LL | fn foo<T>() -> impl Sized {
| - this generic parameter must be used with a generic type parameter
LL | let _: () = foo::<u8>();
| ^^

error[E0792]: expected generic type parameter, found `u8`
--> $DIR/non-defining-use.rs:8:12
|
LL | fn bar<T>(val: T) -> impl Sized {
| - this generic parameter must be used with a generic type parameter
LL | let _: u8 = bar(0u8);
| ^^

error: concrete type differs from previous defining opaque type use
--> $DIR/non-defining-use.rs:8:17
|
LL | let _: u8 = bar(0u8);
| ^^^^^^^^ expected `T`, got `u8`
|
note: previous use here
--> $DIR/non-defining-use.rs:7:22
|
LL | fn bar<T>(val: T) -> impl Sized {
| ^^^^^^^^^^

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0792`.

0 comments on commit e845910

Please sign in to comment.