Skip to content

Strange type inference for recursive RPIT #139406

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

Open
theemathas opened this issue Apr 5, 2025 · 1 comment
Open

Strange type inference for recursive RPIT #139406

theemathas opened this issue Apr 5, 2025 · 1 comment
Labels
A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. C-bug Category: This is a bug. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@theemathas
Copy link
Contributor

I tried this code:

#![allow(unconditional_recursion)]

fn what1<T>(x: T) -> impl Sized {
    what1(x)
}

fn what2<T>(x: T) -> impl Sized {
    what2(what2(x))
}

fn print_return_type<T, U>(_: impl Fn(T) -> U) {
    println!("{}", std::any::type_name::<U>())
}

fn main() {
    print_return_type(what1::<i32>);
    print_return_type(what2::<i32>);
}

I compiled the above code in edition 2024, and got the output:

()
i32

Also, the following code gives a compile error:

#![allow(unconditional_recursion)]

fn what3<T>(x: T) -> impl Sized {
    what3(what3(what3(x)))
}
error[E0792]: expected generic type parameter, found `impl Sized`
 --> src/lib.rs:4:5
  |
3 | fn what3<T>(x: T) -> impl Sized {
  |          - this generic parameter must be used with a generic type parameter
4 |     what3(what3(what3(x)))
  |     ^^^^^^^^^^^^^^^^^^^^^^

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

(See also #139350)

I think that both what1 and what2 should not compile, but I'm not sure. And if it does compiles, they should both return !. However, they return () and i32, respectively. (This happens despite the edition 2024 never type fallback change.) The current behavior doesn't make any sense to me.

Discovered while experimenting with #139402.

@rustbot labels +A-impl-trait

Meta

Reproducible on the playground with nightly rust 1.88.0-nightly (2025-04-04 17ffbc81a30c09419383)

@theemathas theemathas added the C-bug Category: This is a bug. label Apr 5, 2025
@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. labels Apr 5, 2025
@lcnr
Copy link
Contributor

lcnr commented Apr 7, 2025

Let me explain what's going on. I would like us to definitely break what2 for now. Longterm it may also result in ()/!

what1

we add RPIT<T> = RPIT<T> to the opaque type storage, this constraint gets ignored and not added to the final TypeckResults

if !self.fcx.next_trait_solver() {
if let ty::Alias(ty::Opaque, alias_ty) = hidden_type.ty.kind()
&& alias_ty.def_id == opaque_type_key.def_id.to_def_id()
&& alias_ty.args == opaque_type_key.args
{
continue;
}
}

If there are no constraints for an RPIT, we explicitly fall back to Ty::new_diverging_default(tcx)

DefiningScopeKind::HirTypeck => {
let tables = tcx.typeck(owner_def_id);
if let Some(guar) = tables.tainted_by_errors {
Ty::new_error(tcx, guar)
} else if let Some(hidden_ty) = tables.concrete_opaque_types.get(&def_id) {
hidden_ty.ty
} else {
// FIXME(-Znext-solver): This should not be necessary and we should
// instead rely on inference variable fallback inside of typeck itself.
// We failed to resolve the opaque type or it
// resolves to itself. We interpret this as the
// no values of the hidden type ever being constructed,
// so we can just make the hidden type be `!`.
// For backwards compatibility reasons, we fall back to
// `()` until we the diverging default is changed.
Ty::new_diverging_default(tcx)
}
}

what2

HIR typeck eagerly replaces the return type with an infer var, ending up with RPIT<T> = RPIT<RPIT<T>> in the storage. While we return this in the TypeckResults, it's never actually used anywhere.

MIR building then results in the following statement

let _0: impl RPIT<T> /* the return place */ = build<RPIT<T>>(_some_local);

Unlike HIR typeck MIR typeck now directly equates RPIT<T> with RPIT<RPIT<T>>. This does not try to define RPIT but instead relates its generic arguments

(
&ty::Alias(ty::Opaque, ty::AliasTy { def_id: a_def_id, .. }),
&ty::Alias(ty::Opaque, ty::AliasTy { def_id: b_def_id, .. }),
) if a_def_id == b_def_id => {
super_combine_tys(infcx, self, a, b)?;
}

This means we relate T with RPIT<T>, which results in a defining use RPIT<T> = T

what3

same as what2 except that we relate RPIT<T> with RPIT<RPIT<RPIT<T>>> which means we get a non-universal defining use RPIT<RPIT<T>> = T.

@jieyouxu jieyouxu added T-types Relevant to the types team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Apr 30, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue May 20, 2025
…r=lcnr

Error on recursive opaque ty in HIR typeck

"Non-trivially recursive" opaques are opaques whose hidden types are inferred to be equal to something other than themselves. For example, if we have a TAIT like `type TAIT = impl Sized`, if we infer the hidden type to be `TAIT := (TAIT,)`, that would be a non-trivial recursive definition. We don't want to support opaques that are non-trivially recursive, since they will (almost!! -- see caveat below) always result in borrowck errors, and are generally a pain to deal with.

On the contrary, trivially recursive opaques may occur today because the old solver overagerly uses `replace_opaque_types_with_inference_vars`. This infer var can then later be constrained to be equal to the opaque itself. These cases will not necessarily result in borrow-checker errors, since other uses of the opaque may properly constrain the opaque. If there are no other uses we may instead fall back to `()` today.

The only weird case that we have to unfortunately deal with was discovered in rust-lang#139406:

```rust
#![allow(unconditional_recursion)]

fn what1<T>(x: T) -> impl Sized {
    what1(x)
}

fn what2<T>(x: T) -> impl Sized {
    what2(what2(x))
}

fn print_return_type<T, U>(_: impl Fn(T) -> U) {
    println!("{}", std::any::type_name::<U>())
}

fn main() {
    print_return_type(what1::<i32>); // ()
    print_return_type(what2::<i32>); // i32
}
```

> HIR typeck eagerly replaces the return type with an infer var, ending up with `RPIT<T> = RPIT<RPIT<T>>` in the storage. While we return this in the `TypeckResults`, it's never actually used anywhere.
>
> MIR building then results in the following statement
> ```rust
> let _0: impl RPIT<T> /* the return place */ = build<RPIT<T>>(_some_local);
> ```
> Unlike HIR typeck MIR typeck now directly equates `RPIT<T>` with `RPIT<RPIT<T>>`. This does not try to define `RPIT` but instead relates its generic arguments https://github.com/rust-lang/rust/blob/b9856b6e400709392dd14599265b6fd52fc19f3e/compiler/rustc_infer/src/infer/relate/type_relating.rs#L185-L190
>
> This means we relate `T` with `RPIT<T>`, which results in a defining use `RPIT<T> = T`

I think it's pretty obvious that this is not desirable behavior, and according to the crater run there were no regressions, so let's break this so that we don't have any inference hazards in the new solver.

In the future `what2` may end up compiling again by also falling back to `()`. However, that is not yet guaranteed and the transition to this state is made significantly harder by not temporarily breaking it on the way. It is also concerning to change the inferred hidden type like this without any notification to the user, even if likely not an issue in this concrete case.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue May 20, 2025
Rollup merge of rust-lang#139419 - compiler-errors:recursive-opaque, r=lcnr

Error on recursive opaque ty in HIR typeck

"Non-trivially recursive" opaques are opaques whose hidden types are inferred to be equal to something other than themselves. For example, if we have a TAIT like `type TAIT = impl Sized`, if we infer the hidden type to be `TAIT := (TAIT,)`, that would be a non-trivial recursive definition. We don't want to support opaques that are non-trivially recursive, since they will (almost!! -- see caveat below) always result in borrowck errors, and are generally a pain to deal with.

On the contrary, trivially recursive opaques may occur today because the old solver overagerly uses `replace_opaque_types_with_inference_vars`. This infer var can then later be constrained to be equal to the opaque itself. These cases will not necessarily result in borrow-checker errors, since other uses of the opaque may properly constrain the opaque. If there are no other uses we may instead fall back to `()` today.

The only weird case that we have to unfortunately deal with was discovered in rust-lang#139406:

```rust
#![allow(unconditional_recursion)]

fn what1<T>(x: T) -> impl Sized {
    what1(x)
}

fn what2<T>(x: T) -> impl Sized {
    what2(what2(x))
}

fn print_return_type<T, U>(_: impl Fn(T) -> U) {
    println!("{}", std::any::type_name::<U>())
}

fn main() {
    print_return_type(what1::<i32>); // ()
    print_return_type(what2::<i32>); // i32
}
```

> HIR typeck eagerly replaces the return type with an infer var, ending up with `RPIT<T> = RPIT<RPIT<T>>` in the storage. While we return this in the `TypeckResults`, it's never actually used anywhere.
>
> MIR building then results in the following statement
> ```rust
> let _0: impl RPIT<T> /* the return place */ = build<RPIT<T>>(_some_local);
> ```
> Unlike HIR typeck MIR typeck now directly equates `RPIT<T>` with `RPIT<RPIT<T>>`. This does not try to define `RPIT` but instead relates its generic arguments https://github.com/rust-lang/rust/blob/b9856b6e400709392dd14599265b6fd52fc19f3e/compiler/rustc_infer/src/infer/relate/type_relating.rs#L185-L190
>
> This means we relate `T` with `RPIT<T>`, which results in a defining use `RPIT<T> = T`

I think it's pretty obvious that this is not desirable behavior, and according to the crater run there were no regressions, so let's break this so that we don't have any inference hazards in the new solver.

In the future `what2` may end up compiling again by also falling back to `()`. However, that is not yet guaranteed and the transition to this state is made significantly harder by not temporarily breaking it on the way. It is also concerning to change the inferred hidden type like this without any notification to the user, even if likely not an issue in this concrete case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. C-bug Category: This is a bug. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants