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

fix debug print of pointer types #608

Merged
merged 1 commit into from
Sep 14, 2020
Merged

Conversation

basil-cow
Copy link
Contributor

@basil-cow basil-cow commented Sep 12, 2020

Fixes confusing debug print of pointer types, previously &'a T it was printed as Not<!0_0, T> and *const T was printed as Not<T>, which was not ideal.

@basil-cow
Copy link
Contributor Author

While we are at it, is there a specific reason for application types to be printed as ugly as they are now, or I can go ahead and change them to be rendered "properly"?

Copy link
Member

@detrumi detrumi left a comment

Choose a reason for hiding this comment

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

I think we should add a impl Debug for Mutability instead, to avoid doing this for each place where it can occur.

While we are at it, is there a specific reason for application types to be printed as ugly as they are now, or I can go ahead and change them to be rendered "properly"?

I'm assuming it's there because implementing Debug directly doesn't work, but if we don't need it then feel free to simplify it.

@basil-cow
Copy link
Contributor Author

basil-cow commented Sep 12, 2020

I think we should add a impl Debug for Mutability

I am not sure how to do that, it renders to different things when in different places, i.e. Mutability::Not in Ref renders to nothing in {{&}} and to const in {{*const}} in RawPtr.

By ugly I didn't mean the implementation, I meant that printed types are in {type name}{substitution} format, instead of the natural way to render it, i.e. {{array}}<u32, 3> instead of [u32; 3].

@detrumi
Copy link
Member

detrumi commented Sep 14, 2020

Ah right, did not notice that.
And yeah, improving the application ty formatting would be great

@jackh726
Copy link
Member

So, yeah, debug printing of Application tys could be improved (see the changes I made in the rustc integration). The only problem is this can't be done generically, since it relies on unpacking the substitution without having access to an interner instance. But we can easily do this for the chalk-integration Interner impl.

@jackh726
Copy link
Member

@Areredify feel free to followup with this, if you want. Or just file an issue for someone else to followup.

In the meantime: @bors r=jackh726,detrumi

@bors
Copy link
Collaborator

bors commented Sep 14, 2020

📌 Commit 3434f66 has been approved by jackh726,detrumi

@bors
Copy link
Collaborator

bors commented Sep 14, 2020

⌛ Testing commit 3434f66 with merge 3ea88b7...

@bors
Copy link
Collaborator

bors commented Sep 14, 2020

☀️ Test successful - checks-actions
Approved by: jackh726,detrumi
Pushing 3ea88b7 to master...

@bors bors merged commit 3ea88b7 into rust-lang:master Sep 14, 2020
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.

4 participants