Skip to content

Resolve associated types #8283

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Apr 1, 2021
Merged

Resolve associated types #8283

merged 2 commits into from
Apr 1, 2021

Conversation

Veykril
Copy link
Member

@Veykril Veykril commented Apr 1, 2021

Prior we were only resolving paths until the first type was found, then discarding the result if the path wasn't fully consumed. That of course causes associated types to not resolve. Fixes #5003

},
_ => return None,
};
// FIXME supertraits
Copy link
Member Author

Choose a reason for hiding this comment

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

We got a function for this in hir_ty, I guess it would make sense to move it to hir_def?(I think it's implementation solely relies on hir_def). I have no clue in which module that would belong in that case though, unless a new utils module like it is in hir_ty
https://github.com/rust-analyzer/rust-analyzer/blob/25201b2dad7b4b0d41494e238ebf643ad7ad8cd6/crates/hir_ty/src/utils.rs#L81

Copy link
Member

Choose a reason for hiding this comment

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

I probably want to get rid of all_super_traits, but it might be nice to turn all_super_trait_refs into a query.

@flodiebold
Copy link
Member

flodiebold commented Apr 1, 2021

I'm not a fan of the duplication of logic. Actually assoc_type_shorthand_candidates is already exposed on the semantics level, so can't that be reused?

Also, I'm not sure whether this handles <T as Trait>::Type and <T>::Type?

@Veykril
Copy link
Member Author

Veykril commented Apr 1, 2021

Ye the duplication isn't too nice, I'll see what I can do about that.

It doesn't handle <T as Trait>::Type yet, I already looked into that and it'll require changing a lot of ModPath params to Paths so that we don't discard the generics. Didn't feel like doing that here to keep the PR smaller.

<T>::Type isn't handled here either I believe, should be fixed if I reduce the duplication though I imagine. now handled properly

@Veykril
Copy link
Member Author

Veykril commented Apr 1, 2021

Oh, I think I didn't even see assoc_type_shorthand_candiates, that should be easily reuseable.

@flodiebold
Copy link
Member

LGTM, r=me

@Veykril
Copy link
Member Author

Veykril commented Apr 1, 2021

bors r=flodiebold

@bors
Copy link
Contributor

bors bot commented Apr 1, 2021

@bors bors bot merged commit 5ef0c7a into rust-lang:master Apr 1, 2021
@Veykril Veykril deleted the namers-assoc branch April 1, 2021 21:28
@Veykril
Copy link
Member Author

Veykril commented Apr 2, 2021

changelog fix: Resolve associated types at the IDE layer

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

Successfully merging this pull request may close these issues.

Associated types are not resolved in the IDE layer
4 participants