Skip to content

Add an explicit Self: Trait clause to trait assoc items #1541

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
wants to merge 2 commits into from

Conversation

Nadrieril
Copy link
Collaborator

Today, items defined within a trait decl (methods, types, consts and even closures inside default method bodies) can refer to the currently-implemented trait using ImplExpr::SelfId. This can cause issues where we can't know what the SelfId clause refers to without doing additional trait resolution, e.g. in a case like this:

trait Thing {
    type Item;
    fn foo(i: Self::Item) {
        // in the closure item we forgot what `Self as Trait` is
        (|_: Self::Item| ())(i)
    }
}

To make these items less special, this PR gives them an extra Self: Trait clause argument, which will be fulfilled by trait resolution whenever they are refered to. Now only the trait declaration itself (and its associated types, while GATs aren't finalized) can use SelfId, everything else can only refer to numbered local clauses.

Nadrieril added 2 commits July 3, 2025 14:24
This way, the special `ImplExpr::SelfImpl` is only valid within a trait
declaration.
@Nadrieril Nadrieril requested a review from a team as a code owner July 3, 2025 12:32
@Nadrieril Nadrieril requested a review from W95Psp July 3, 2025 12:32
@Nadrieril Nadrieril mentioned this pull request Jul 3, 2025
Copy link
Collaborator

@W95Psp W95Psp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that seems like a good idea to me.

However, this breaks how our engine deals with Self bounds.
So we need to schedule when we can fix it before merging.

@Nadrieril
Copy link
Collaborator Author

Oh, I indeed expected breakage but it seems that your CI passes, how come?

@Nadrieril
Copy link
Collaborator Author

Superceded by #1559.

@Nadrieril Nadrieril closed this Jul 22, 2025
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