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

add dump method for Ptr #26880

Merged
merged 4 commits into from
May 14, 2018
Merged

add dump method for Ptr #26880

merged 4 commits into from
May 14, 2018

Conversation

jekbradbury
Copy link
Contributor

Currently dump(x::Ptr) will print the type of the pointer twice, e.g.

Ptr{Nothing} Ptr{Nothing} @0x0000000120f1d2d0

This was because the catchall dump method first printed typeof(x), then x itself, and the print method for Ptr also prints the type first. This PR adds an overload so that the pointer type is only printed once.

Ptr{Nothing} @0x0000000120f1d2d0

I believe dump is not really exercised in tests right now but I could add some?

@ararslan ararslan added needs tests Unit tests are required for this change display and printing Aesthetics and correctness of printed representations of objects. labels Apr 22, 2018
@JeffBezanson
Copy link
Member

There are tests for dump in test/show.jl. They use the idiom sprint(dump, ...) so aren't always obvious to grep for.

Currently `dump(x::Ptr)` will print the type of the pointer twice, e.g.
```Ptr{Nothing} Ptr{Nothing} @0x0000000120f1d2d0```
This adds an overload so that it only prints once.
@StefanKarpinski StefanKarpinski added this to the 1.0.x milestone May 14, 2018
@jekbradbury
Copy link
Contributor Author

FreeBSD failure is the same as in #27075, which suggests that it may not be such a "low-probability" failure anymore (perhaps due to some other change)

@KristofferC KristofferC merged commit 098f40f into JuliaLang:master May 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
display and printing Aesthetics and correctness of printed representations of objects. needs tests Unit tests are required for this change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants