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

Normalize associated types in paths in expressions #14436

Merged
merged 2 commits into from Apr 5, 2023

Conversation

lowr
Copy link
Contributor

@lowr lowr commented Mar 29, 2023

Part of #14393

When we resolve paths in expressions (either path expressions or paths in struct expressions), there's a need of projection normalization, which TyLoweringContext cannot do on its own. We've been properly applying normalization for paths in struct expressions without type anchor, but not for others:

enum E {
    S { v: i32 }
    Empty,
}

impl Foo for Bar {
    type Assoc = E;
    fn foo() {
        let _ = Self::Assoc::S { v: 42 };   // path in struct expr without type anchor; we already support this
        let _ = <Self>::Assoc::S { v: 42 }; // path in struct expr with type anchor; resolves with this PR
        let _ = Self::Assoc::Empty;         // path expr; resolves with this PR
    }
}

With this PR we correctly resolve the whole path, but we need some more tweaks in HIR and/or IDE layers to properly resolve a qualifier (prefix) of such paths and provide IDE features that are pointed out in #14393 to be currently broken.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 29, 2023
@lowr lowr changed the title Normalize associated types in path expressions Normalize associated types in paths in expressions Mar 31, 2023
@lowr lowr marked this pull request as ready for review March 31, 2023 12:37
Comment on lines +885 to +888
// to a projection, which `TyLoweringContext` cannot handle on its own.
while !remaining_segments.is_empty() {
let resolved_segment = path.segments().get(remaining_idx - 1).unwrap();
let current_segment = remaining_segments.take(1);
Copy link
Member

Choose a reason for hiding this comment

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

There are a bunch of unwrap in this function, which I think it is possible to remove by defining some windows like thing for PathSegments (or by using Itertools::tuple_windows) and a function which goes from a &PathSegment to a &PathSegments (similar to slice::from_ref).

Copy link
Member

Choose a reason for hiding this comment

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

we've got these path segment unwraps in more places (our API for paths isn't really that great currently)

Copy link
Member

Choose a reason for hiding this comment

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

Okay, so we can do it in a follow up.

@HKalbasi
Copy link
Member

HKalbasi commented Apr 5, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 5, 2023

📌 Commit 6447d48 has been approved by HKalbasi

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Apr 5, 2023

⌛ Testing commit 6447d48 with merge af30656...

@bors
Copy link
Collaborator

bors commented Apr 5, 2023

☀️ Test successful - checks-actions
Approved by: HKalbasi
Pushing af30656 to master...

@bors bors merged commit af30656 into rust-lang:master Apr 5, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants