Skip to content

rocket regression: rigid alias don't require trait to hold #177

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
compiler-errors opened this issue Apr 11, 2025 · 4 comments · Fixed by rust-lang/rust#139828
Closed

rocket regression: rigid alias don't require trait to hold #177

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

Comments

@compiler-errors
Copy link
Member

compiler-errors commented Apr 11, 2025

https://crater-reports.s3.amazonaws.com/pr-133502-1/try%23fa8e241660363f48d64b66b05eea58c93ab828fb/reg/rocket-0.5.1/log.txt

Minimized:

use std::{future::Future, pin::Pin};

pub trait FromRequest<'r> {
    type Assoc;
    fn from_request() -> Pin<Box<dyn Future<Output = Self::Assoc> + Send>>;
}

fn from_request<'r, T: FromRequest<'r>>() -> Pin<Box<dyn Future<Output = ()> + Send>> {
    Box::pin(async move {
        T::from_request().await;
    })
}

@compiler-errors

This comment has been minimized.

@compiler-errors
Copy link
Member Author

compiler-errors commented Apr 11, 2025

Has to do with the leak check and higher ranked trait bounds b/c of coroutine witness erasure.

I think we're somewhat unnecessarily requiring that dyn Future<Output = Self::Assoc> + Send implements all of its bounds in order for it to implement Send.

Proving the coroutine is Send requires proving (dyn Future<Output = <T as FromRequest<'!0>>::Assoc> + Send): Send. Note that '!0 is some higher ranked lifetime due to coroutine lifetime erasure.

Checking this goal goes through assemble_object_bound_candidates -> probe_and_consider_object_bound_candidate, which equates the self type with itself. That emits an alias-relate goal <T as FromRequest<'!0>>::Assoc == <T as FromRequest<'!0>>::Assoc, which ends up normalizing <T as FromRequest<'!0>>::Assoc, which is rigid, which ends up checking T: FromRequest<'!0> for alias wf.

Since all we know is that T: FromRequest<'r> from the param-env, we end up failing (I believe due to the leak check here).

@compiler-errors
Copy link
Member Author

The reason this doesn't fail in the old solver is b/c we don't check that rigid projections implement their trait goal, lol.

@compiler-errors
Copy link
Member Author

I'm curious to know exactly what the fallout of this is in the old solver: rust-lang/rust#139763

@lcnr lcnr added the from-crater A regression found via a crater run, not part of our test suite label Apr 14, 2025
@compiler-errors compiler-errors self-assigned this Apr 14, 2025
@lcnr lcnr moved this from unknown to in progress in -Znext-solver=globally Apr 16, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 16, 2025
Don't require rigid alias's trait to hold

See test for write-up. TL;DR is that we don't need the trait bound to hold, since we enforce it during WF.

I think this is preferable to introducing (if we even could do so) a more specific hack around coroutine interiors, higher ranked types, etc, since this is just a manifestation of more pervasive issues w/ lifetime erasure in coroutines. This just doesn't manifest in the old solver b/c it doesn't try to prove `T: Trait` holds when rigidly projecting `<T as Trait>::Assoc`.

It's pretty clear that this affects quite a few traits (rust-lang#139763), so I think this needs fixing.

r? lcnr

Fixes rust-lang/trait-system-refactor-initiative#177
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Apr 17, 2025
Rollup merge of rust-lang#139828 - compiler-errors:rigid-trait, r=lcnr

Don't require rigid alias's trait to hold

See test for write-up. TL;DR is that we don't need the trait bound to hold, since we enforce it during WF.

I think this is preferable to introducing (if we even could do so) a more specific hack around coroutine interiors, higher ranked types, etc, since this is just a manifestation of more pervasive issues w/ lifetime erasure in coroutines. This just doesn't manifest in the old solver b/c it doesn't try to prove `T: Trait` holds when rigidly projecting `<T as Trait>::Assoc`.

It's pretty clear that this affects quite a few traits (rust-lang#139763), so I think this needs fixing.

r? lcnr

Fixes rust-lang/trait-system-refactor-initiative#177
@lcnr lcnr moved this from in progress to done in -Znext-solver=globally Apr 17, 2025
@lcnr lcnr closed this as completed by moving to done in -Znext-solver=globally Apr 17, 2025
@lcnr lcnr changed the title rocket regression rocket regression: rigid alias don't require trait to hold Apr 17, 2025
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this issue Apr 19, 2025
Don't require rigid alias's trait to hold

See test for write-up. TL;DR is that we don't need the trait bound to hold, since we enforce it during WF.

I think this is preferable to introducing (if we even could do so) a more specific hack around coroutine interiors, higher ranked types, etc, since this is just a manifestation of more pervasive issues w/ lifetime erasure in coroutines. This just doesn't manifest in the old solver b/c it doesn't try to prove `T: Trait` holds when rigidly projecting `<T as Trait>::Assoc`.

It's pretty clear that this affects quite a few traits (rust-lang/rust#139763), so I think this needs fixing.

r? lcnr

Fixes rust-lang/trait-system-refactor-initiative#177
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
from-crater A regression found via a crater run, not part of our test suite
Projects
Development

Successfully merging a pull request may close this issue.

2 participants