Skip to content

Conversation

frewsxcv
Copy link
Contributor

Fixes #46755.

@rust-highfive
Copy link
Contributor

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

@frewsxcv frewsxcv added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Dec 17, 2017
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

I don't think wrapping these in Unique(...) and Shared(...) adds value, but adds a lot of noise if you have Vec<Shared<T>> for example. I would prefer to print them as just the pointer value, Debug::fmt(&self.as_ptr(), f). @bluss flagged this as well in #32054 (expand the outdated comments on src/libcore/ptr.rs).

@SimonSapin
Copy link
Contributor

Printing just the pointer sounds good to me.

@sfackler
Copy link
Member

👍 to just printing the pointer.

@frewsxcv
Copy link
Contributor Author

you all have spoken
the implementation will
now print the pointer

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 18, 2017
@bors
Copy link
Collaborator

bors commented Dec 19, 2017

☔ The latest upstream changes (presumably #46749) made this pull request unmergeable. Please resolve the merge conflicts.

@frewsxcv
Copy link
Contributor Author

merge conflicts addressed

@kennytm
Copy link
Member

kennytm commented Dec 27, 2017

This is going to conflict with #46952 which removed Unique and renamed Shared to NonNull.

@SimonSapin
Copy link
Contributor

I can merge this in #46952, if that helps.

@frewsxcv
Copy link
Contributor Author

closing in favor of #46952

@frewsxcv frewsxcv closed this Dec 28, 2017
@frewsxcv frewsxcv deleted the frewsxcv-debug branch December 28, 2017 16:01
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. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants