internal: merge Trait and TraitAlias handling#20376
Conversation
rustbot
added
the
S-waiting-on-review
fee1-dead
changed the title
fee1-dead
force-pushed
the
traitalias
branch
2 times, most recently
from
900a128 to
e3cf7eb
Compare
| let ast_id_map = db.ast_id_map(ast_id.file_id); | ||
| let source = ast_id.with_value(ast_id_map.get(ast_id.value)).to_node(db); | ||
| if source.eq_token().is_some() { | ||
| // FIXME(trait-alias) probably needs special handling here |
There was a problem hiding this comment.
I guess this would be okay as this PR treats TraitAliass somewhat like new trait with bounds and no assoc items and rust-analyzer won't demand assoc items for such traits or (currently for) trait aliases?
There was a problem hiding this comment.
Yeah, I reconsidered and thought it might not need special handling halfway into implementing this PR. I originally was thinking maybe we should return the associated items of the trait that it aliases but we already lower them with bound handling - so the aliased trait will be treated as a super trait
ShoyuVanilla
left a comment
ShoyuVanilla
left a comment
There was a problem hiding this comment.
I think this is a solid improvement overall. It might end up marking trait aliases as actual traits, but it correctly resolves associated types and, more importantly, trait methods. That alone could potentially address our long-standing issues with trait aliases, such as #15566 .
My only hesitation is whether this could complicate the merging of #20329. I skimmed that PR and only noticed a few minor references to TraitAlias, so the impact may be limited.
That said, I'm unsure whether we should merge this before or after #20329. cc @rust-lang/rust-analyzer
|
#20329 was merged already and GitHub does not report any merge conflicts with this PR - so I think this is good to go? |
|
Surprising that there are no conflicts, but in that case yes let's merge this 👍 |
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
|
Rebased, only the newest commit has changed, please review :) |
rustbot
removed
the
S-waiting-on-review
lnicola
changed the title
4 participants
Much of the current infrastructure around traits are good and cool but trait aliases don't get to enjoy type inference because they are treated as distinct entities from traits.
Doesn't seem easy to change all the existing facilities to accept something like
Either<Trait, TraitAlias>.We can actually just merge them and most things will work. Second commit adds a simple test.
The core of this change is in
crates/syntax/rust.ungram.