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

Properly re-print trait impls. #1956

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

Properly re-print trait impls. #1956

wants to merge 2 commits into from

Conversation

chriseth
Copy link
Member

@chriseth chriseth commented Oct 25, 2024

Before this PR, the list of trait implementations was not present in Analyzed, but we need them for re-printing the PIL (and other things).

This PR also contains a simplification in that it only keeps a list of all trait impls, not sorted into which trait they implement. This information is only relevant during trait impl solving, where we re-build that data structure. Keeping the pre-sorted layout would make re-printing in source order much more difficult.

And now it also formats paths to the current namespace properly.

}
let Some(trait_impls) = self.trait_impls.get(trait_decl_name) else {
return Err(format!(
"Could not find an implementation for the trait function {reference}"
Copy link
Collaborator

@gzanitti gzanitti Oct 25, 2024

Choose a reason for hiding this comment

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

maybe

Suggested change
"Could not find an implementation for the trait function {reference}"
"Could not find an implementation for the trait declaration {trait_decl_name}"

to differentiate it from the following error

Copy link
Collaborator

@gzanitti gzanitti left a comment

Choose a reason for hiding this comment

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

LGTM

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