Skip to content

salvo_core regression: RPITIT type_of cycle #185

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.

Sign up for GitHub

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

Jump to bottom
Closed
compiler-errors opened this issue Apr 24, 2025 · 2 comments · Fixed by rust-lang/rust#140276
Closed

salvo_core regression: RPITIT type_of cycle #185

compiler-errors opened this issue Apr 24, 2025 · 2 comments · Fixed by rust-lang/rust#140276
Assignees
@compiler-errors
Labels
from-crater A regression found via a crater run, not part of our test suite

Comments

@compiler-errors
Copy link
Member

https://crater-reports.s3.amazonaws.com/pr-133502-4/try%230c7fed2b7584d310ca8221267fa1b3bec4df427f/reg/salvo_core-0.78.0/log.txt

use std::future::Future;

trait Handler {}

struct W<T>(T);

trait SendTarget {
    fn call(self) -> impl Future<Output = ()> + Send;
}

impl<T> SendTarget for W<T>
where
    T: Handler + Send,
{
    async fn call(self) {
        todo!()
    }
}

impl<T> SendTarget for T
where
    T: Handler + Send,
{
    async fn call(self) {
        W(self).call().await
    }
}

fn main() {}
@compiler-errors
Copy link
Member Author

Smaller:

trait Foo {
    fn call(self) -> impl Send;
}

trait Nested {}
impl<T> Foo for T
where
    T: Nested,
{
    fn call(self) -> impl Sized {
        W.call()
    }
}

struct NotSatisfied;
impl Foo for NotSatisfied {
    fn call(self) -> impl Sized {
        todo!()
    }
}

@compiler-errors compiler-errors mentioned this issue Apr 25, 2025
Merged
@lcnr lcnr changed the title salvo_core regression salvo_core regression: RPITIT type_of cycle Apr 25, 2025
@lcnr lcnr moved this from unknown to in progress in -Znext-solver=globally Apr 25, 2025
@lcnr lcnr added the from-crater A regression found via a crater run, not part of our test suite label Apr 25, 2025
@compiler-errors compiler-errors self-assigned this Apr 25, 2025
ChrisDenton added a commit to ChrisDenton/rust that referenced this issue Apr 28, 2025
@ChrisDenton
Rollup merge of rust-lang#140276 - compiler-errors:typeof-less-eagerl…
508c98f 
…y, r=lcnr

Do not compute type_of for impl item if impl where clauses are unsatisfied

Consider the following code:

```rust
trait Foo {
    fn call(self) -> impl Send;
}

trait Nested {}
impl<T> Foo for T
where
    T: Nested,
{
    fn call(self) -> impl Sized {
        NotSatisfied.call()
    }
}

struct NotSatisfied;
impl Foo for NotSatisfied {
    fn call(self) -> impl Sized {
        todo!()
    }
}
```

In `impl Foo for NotSatisfied`, we need to prove that the RPITIT is well formed. This requires proving the item bound `<NotSatisfied as Foo>::RPITIT: Send`. Normalizing `<NotSatisfied as Foo>::RPITIT: Send` assembles two impl candidates, via the `NotSatisfied` impl and the blanket `T` impl. We end up computing the `type_of` for the blanket impl even if `NotSatisfied: Nested` where clause does not hold.

This type_of query ends up needing to prove that its own `impl Sized` RPIT satisfies `Send`, which ends up needing to compute the hidden type of the RPIT, which is equal to the return type  of `NotSatisfied.call()`. That ends up in a query cycle, since we subsequently try normalizing that return type via the blanket impl again!

In the old solver, we don't end up computing the `type_of` an impl candidate if its where clauses don't hold, since this select call would fail before confirming the projection candidate:

https://github.com/rust-lang/rust/blob/d7ea436a02d5de4033fcf7fd4eb8ed965d0f574c/compiler/rustc_trait_selection/src/traits/project.rs#L882

This PR makes the new solver more consistent with the old solver by adding a call to `try_evaluate_added_goals` after regstering the impl predicates, which causes us to bail before computing the `type_of` for impls if the impl definitely doesn't apply.

r? lcnr

Fixes rust-lang/trait-system-refactor-initiative#185
ChrisDenton added a commit to ChrisDenton/rust that referenced this issue Apr 28, 2025
@ChrisDenton
Rollup merge of rust-lang#140276 - compiler-errors:typeof-less-eagerl…
1d63e53 
…y, r=lcnr

Do not compute type_of for impl item if impl where clauses are unsatisfied

Consider the following code:

```rust
trait Foo {
    fn call(self) -> impl Send;
}

trait Nested {}
impl<T> Foo for T
where
    T: Nested,
{
    fn call(self) -> impl Sized {
        NotSatisfied.call()
    }
}

struct NotSatisfied;
impl Foo for NotSatisfied {
    fn call(self) -> impl Sized {
        todo!()
    }
}
```

In `impl Foo for NotSatisfied`, we need to prove that the RPITIT is well formed. This requires proving the item bound `<NotSatisfied as Foo>::RPITIT: Send`. Normalizing `<NotSatisfied as Foo>::RPITIT: Send` assembles two impl candidates, via the `NotSatisfied` impl and the blanket `T` impl. We end up computing the `type_of` for the blanket impl even if `NotSatisfied: Nested` where clause does not hold.

This type_of query ends up needing to prove that its own `impl Sized` RPIT satisfies `Send`, which ends up needing to compute the hidden type of the RPIT, which is equal to the return type  of `NotSatisfied.call()`. That ends up in a query cycle, since we subsequently try normalizing that return type via the blanket impl again!

In the old solver, we don't end up computing the `type_of` an impl candidate if its where clauses don't hold, since this select call would fail before confirming the projection candidate:

https://github.com/rust-lang/rust/blob/d7ea436a02d5de4033fcf7fd4eb8ed965d0f574c/compiler/rustc_trait_selection/src/traits/project.rs#L882

This PR makes the new solver more consistent with the old solver by adding a call to `try_evaluate_added_goals` after regstering the impl predicates, which causes us to bail before computing the `type_of` for impls if the impl definitely doesn't apply.

r? lcnr

Fixes rust-lang/trait-system-refactor-initiative#185
ChrisDenton added a commit to ChrisDenton/rust that referenced this issue Apr 28, 2025
@ChrisDenton
Rollup merge of rust-lang#140276 - compiler-errors:typeof-less-eagerl…
0bd531a 
…y, r=lcnr

Do not compute type_of for impl item if impl where clauses are unsatisfied

Consider the following code:

```rust
trait Foo {
    fn call(self) -> impl Send;
}

trait Nested {}
impl<T> Foo for T
where
    T: Nested,
{
    fn call(self) -> impl Sized {
        NotSatisfied.call()
    }
}

struct NotSatisfied;
impl Foo for NotSatisfied {
    fn call(self) -> impl Sized {
        todo!()
    }
}
```

In `impl Foo for NotSatisfied`, we need to prove that the RPITIT is well formed. This requires proving the item bound `<NotSatisfied as Foo>::RPITIT: Send`. Normalizing `<NotSatisfied as Foo>::RPITIT: Send` assembles two impl candidates, via the `NotSatisfied` impl and the blanket `T` impl. We end up computing the `type_of` for the blanket impl even if `NotSatisfied: Nested` where clause does not hold.

This type_of query ends up needing to prove that its own `impl Sized` RPIT satisfies `Send`, which ends up needing to compute the hidden type of the RPIT, which is equal to the return type  of `NotSatisfied.call()`. That ends up in a query cycle, since we subsequently try normalizing that return type via the blanket impl again!

In the old solver, we don't end up computing the `type_of` an impl candidate if its where clauses don't hold, since this select call would fail before confirming the projection candidate:

https://github.com/rust-lang/rust/blob/d7ea436a02d5de4033fcf7fd4eb8ed965d0f574c/compiler/rustc_trait_selection/src/traits/project.rs#L882

This PR makes the new solver more consistent with the old solver by adding a call to `try_evaluate_added_goals` after regstering the impl predicates, which causes us to bail before computing the `type_of` for impls if the impl definitely doesn't apply.

r? lcnr

Fixes rust-lang/trait-system-refactor-initiative#185
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Apr 29, 2025
@rust-timer
Unrolled build for rust-lang#140276
a728176 
Rollup merge of rust-lang#140276 - compiler-errors:typeof-less-eagerly, r=lcnr

Do not compute type_of for impl item if impl where clauses are unsatisfied

Consider the following code:

```rust
trait Foo {
    fn call(self) -> impl Send;
}

trait Nested {}
impl<T> Foo for T
where
    T: Nested,
{
    fn call(self) -> impl Sized {
        NotSatisfied.call()
    }
}

struct NotSatisfied;
impl Foo for NotSatisfied {
    fn call(self) -> impl Sized {
        todo!()
    }
}
```

In `impl Foo for NotSatisfied`, we need to prove that the RPITIT is well formed. This requires proving the item bound `<NotSatisfied as Foo>::RPITIT: Send`. Normalizing `<NotSatisfied as Foo>::RPITIT: Send` assembles two impl candidates, via the `NotSatisfied` impl and the blanket `T` impl. We end up computing the `type_of` for the blanket impl even if `NotSatisfied: Nested` where clause does not hold.

This type_of query ends up needing to prove that its own `impl Sized` RPIT satisfies `Send`, which ends up needing to compute the hidden type of the RPIT, which is equal to the return type  of `NotSatisfied.call()`. That ends up in a query cycle, since we subsequently try normalizing that return type via the blanket impl again!

In the old solver, we don't end up computing the `type_of` an impl candidate if its where clauses don't hold, since this select call would fail before confirming the projection candidate:

https://github.com/rust-lang/rust/blob/d7ea436a02d5de4033fcf7fd4eb8ed965d0f574c/compiler/rustc_trait_selection/src/traits/project.rs#L882

This PR makes the new solver more consistent with the old solver by adding a call to `try_evaluate_added_goals` after regstering the impl predicates, which causes us to bail before computing the `type_of` for impls if the impl definitely doesn't apply.

r? lcnr

Fixes rust-lang/trait-system-refactor-initiative#185
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Apr 29, 2025
@ChrisDenton
Rollup merge of #140276 - compiler-errors:typeof-less-eagerly, r=lcnr
12b611c 
Do not compute type_of for impl item if impl where clauses are unsatisfied

Consider the following code:

```rust
trait Foo {
    fn call(self) -> impl Send;
}

trait Nested {}
impl<T> Foo for T
where
    T: Nested,
{
    fn call(self) -> impl Sized {
        NotSatisfied.call()
    }
}

struct NotSatisfied;
impl Foo for NotSatisfied {
    fn call(self) -> impl Sized {
        todo!()
    }
}
```

In `impl Foo for NotSatisfied`, we need to prove that the RPITIT is well formed. This requires proving the item bound `<NotSatisfied as Foo>::RPITIT: Send`. Normalizing `<NotSatisfied as Foo>::RPITIT: Send` assembles two impl candidates, via the `NotSatisfied` impl and the blanket `T` impl. We end up computing the `type_of` for the blanket impl even if `NotSatisfied: Nested` where clause does not hold.

This type_of query ends up needing to prove that its own `impl Sized` RPIT satisfies `Send`, which ends up needing to compute the hidden type of the RPIT, which is equal to the return type  of `NotSatisfied.call()`. That ends up in a query cycle, since we subsequently try normalizing that return type via the blanket impl again!

In the old solver, we don't end up computing the `type_of` an impl candidate if its where clauses don't hold, since this select call would fail before confirming the projection candidate:

https://github.com/rust-lang/rust/blob/d7ea436a02d5de4033fcf7fd4eb8ed965d0f574c/compiler/rustc_trait_selection/src/traits/project.rs#L882

This PR makes the new solver more consistent with the old solver by adding a call to `try_evaluate_added_goals` after regstering the impl predicates, which causes us to bail before computing the `type_of` for impls if the impl definitely doesn't apply.

r? lcnr

Fixes rust-lang/trait-system-refactor-initiative#185
@lcnr
Copy link
Contributor

lcnr commented Apr 29, 2025

fixed by rust-lang/rust#140276

@lcnr lcnr moved this from in progress to done in -Znext-solver=globally Apr 29, 2025
@lcnr lcnr closed this as completed by moving to done in -Znext-solver=globally Apr 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@compiler-errors compiler-errors

Labels
from-crater A regression found via a crater run, not part of our test suite
Projects
-Znext-solver=globally
Status: done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Do not compute type_of for impl item if impl where clauses are unsatisfied compiler-errors/rust
2 participants
@compiler-errors @lcnr