Skip to content
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

Add associated_ty_from_impl to Chalk db to avoid computing associated types eagerly #826

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Mar 19, 2025

Right now we compute all associated types' values in push_program_clauses_for_associated_type_values_in_impls_of. This results in a cycle in rust-analyzer's implementation of RPITITs, since we need to be able to compute some associated type values before others. For example in:

trait Trait {
    type Assoc;
    fn foo() -> impl Sized;
}
impl Trait for () {
    type Assoc = ();
    fn foo() -> Self::Assoc;
}

When solving for the hidden type of fn fooI() -> impl Sized, we need to be able to normalize Self::Assoc before we know the value of the RPITIT in fn foo() -> impl Sized, which we are currently solving for :D

This PR adds associated_ty_from_impl, which will compute that the associated type that is actually the one we're assembling clauses for, rather than iterating through all associated types in the impl.

This should break the cycle and should be reasonably easy to implement in chalk, though I'd like to know if this method would be difficult to implement: cc @ChayimFriedman2

@ChayimFriedman2
Copy link

I tested this and this fixes the cycle in r-a.

@compiler-errors
Copy link
Member Author

Updated the strategy: #t-compiler/help > Cycle on RPITITs computation @ 💬

@compiler-errors compiler-errors changed the title Add associated_ty_matches to Chalk db to avoid computing RPITIT associated types eagerly Add associated_ty_from_impl to Chalk db to avoid computing associated types eagerly Mar 20, 2025
@ChayimFriedman2
Copy link

Can we get this merged and released? Acknowledged, some RPITITs still cause cycles, but this fixes at least some of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants