Make `repr(::Ptr)` parsable Julia code.

[LilithHafner]

I simply changed the default display which also adjusts the "text/plain" representation. I did this because I don't think the current text/plain representation is any more canonical / human readable than this PR's display, so I don't think it warrants added display diversity. The on the wikipedia page, the @ sign only appears in reference to BASIC syntax.

julia> pointer([])
Ptr{Any} @0x0000ffff2b045140

julia> @eval Base show(io::IO, p::Ptr) = print(io, typeof(p), "(0x", string(UInt(p), base = 16, pad = Sys.WORD_SIZE>>2), ")")
show (generic function with 319 methods)

julia> pointer([])
Ptr{Any}(0x0000ffff2b045140)

julia> Ptr{Any}(0x0000ffff2b045140) === ans
true

[tecosaur]

Out of curiosity, how does Ptr display if its show method is removed entirely?

[LilithHafner]

Good question

julia> pointer(Int[])
Ptr{Int64} @0x0000fffed19aa550

julia> Base.delete_method(@which show(stdout, pointer([])))

julia> pointer(Int[])
Ptr{Int64}(0x0000fffed19aa550)

[tecosaur]

I like this, but given the historical legacy the current behaviour has, I feel like somebody else should approve this PR.

For what it's worth though, this LGTM.

[Seelengrab]

I'm torn on this - on one hand, the current printing annoyed me in the past when inspecting generated LLVM IR of a live session, on the other hand I don't really feel comfortable with having a copy-pasteable Ptr representation..

[tecosaur]

Two comments:

• Why is having a copy-pasteable Ptr representation actively bad?
• Could changing the 2-arg show into a 3-arg show be a good alternative if needed?

[Seelengrab]

Why is having a copy-pasteable Ptr representation actively bad?

It gives the illusion that the value behind that pointer is alive, when that's not necessarily the case. Case in point - the cases where I tried to inspect generated LLVM IR of a live session more often than not led to me crashing my Julia session because the underlying object wasn't actually alive :sweat_smile: Since Julia is generally more high level than raw pointer wrangling, I think it's better to print raw pointers with some special representation.

Could changing the 2-arg show into a 3-arg show be a good alternative if needed?

Possibly? I don't know OTOH of all cases where the 3-arg method would be chosen over the 2-arg method.

---

As an aside, I tried looking into the printing of this through the commit history. The current printing has been this way since 2010, the commit that introduced this is 18f456233b280f2b5649c033c1977133c73bfd88, which also introduced the Ptr{T} type in the first place. Just about 99% of all commits ever made to this repo have been past that point. Considering the history here, I think we'd need a VERY good reason to change the printing.

[tecosaur]

The reasoning for this is to make repr valid / 2-arg show abide by this part of the show docstring:

The representation used by show generally includes Julia-specific formatting and type information, and should be parseable Julia code when possible. [emphasis mine]

The problem isn't that Ptr is shown specially, but that the "julia interpretable" show method is unparsable.

What you're talking about "we should show humans something special" seems much more appropriate as the 3-arg show:

For a more verbose human-readable text output for objects of type T, define show(io::IO, ::MIME"text/plain", ::T) in addition

[Seelengrab]

The reasoning for this is to make repr valid / 2-arg show abide by this part of the show docstring:

What you're talking about "we should show humans something special" seems much more appropriate as the 3-arg show:

Yes, I'm aware of that. My point is that it's sometimes really difficult to know when 2-arg vs. 3-arg actually happens, and reproducing/recreating pointers in particular like that is seldom a good idea in either case. Note that the docstring also states:

See also print, which writes un-decorated representations.

Implying that even 2-arg show is intended for decorated representations. It's a big ol' mess :shrug:

I don't mean to block this PR; as mentioned above, I'm torn on this and gave my reasoning for why so that a future person that does merge it has some additional input to consider.

[tecosaur]

Yea, it is a bit of a mess. Particularly in usage, it seems that the delineation between parsable and pretty methods isn't clear. Multiple dispatch is great and all, but if we could time travel I almost wonder if it would have been better to have repr(::IO, x) and prettyprint / show so that the different use cases actually have different names not just different numbers of arguments.

This said, I do think parsibility is worth prioritising, for cases like when a test fails and you want to copy-paste the value into the REPL to inspect the value (e.g. taking WhyNotEqual for a spin when the test failure comes from deep within a struct). As things are now, any non-parsable 2-arg shows taint all output that involves them, and that's what I really don't like.

[Seelengrab]

This said, I do think parsibility is worth prioritising, for cases like when a test fails and you want to copy-paste the value into the REPL to inspect the value (e.g. taking WhyNotEqual for a spin when the test failure comes from deep within a struct). As things are now, any non-parsable 2-arg shows taint all output that involves them, and that's what I really don't like.

Right, for regular values I 100% unequivocally agree - but not for pointers, because those are invalid as soon as the object being pointed to is dead (which is almost assuredly the case when you inspect a test case involving pointers). Recreating the semantic that the Ptr has is not easily possible from repr alone, because just recreating the pointer doesn't mean that the value it originally pointed at will be at the same location in the recreated object.

It's the same reason why the Serialization stdlib doesn't preserve pointers either, preferring to instead write null-pointers. It could be argued that undecorated show is a form of serialization too.

[tecosaur]

Mmmm, a pointer may or may not still be "alive" depending on the particular situation, and so I see the motivation for doing something based on that. I don't think making the representation of anything with a pointer involved un-parsable is the right solution though.

[KristofferC]

Yeah, at that point you might as well just hide the pointer value and show it as Ptr{Int64}(🖕)

[vtjnash]

There is nothing "unsafe" about the bits of a pointer value. They do not have "liveness". If someone is doing calls to the "unsafe" functions, then those are what is unsafe and that is what should be avoided anyways.

[Seelengrab]

Right, the issue is that evaluating a string-representation from repr of an object containing a Ptr (obtained from e.g. a @test) is very unlikely to give you an actually usable Ptr into the new object. The Ptr would refer to the (possibly dead) old object, which is confusing at best and crashing at worst.

[LilithHafner]

It seems to me that Timothy and I are in agreement that this is a good change, with Timothy's reason for wanting to hold of on merging before hearing more people's thoughts that someone added this method explicitly in the past. Sukera is hesitant because pointers can be used unsafely and Sukera thinks a good way to communicate that to users is to make their display representation unparsable, but doesn't want to block this PR. Jameson, Kristoffer, Timothy, and I all think that unparsability is not a good way to dissuade users form performing unsafe operations in this case, and Jameson further (correctly) noted that repring a shown Ptr is not an unsafe operation.

I see this as consensus to merge.

[Edited based @Seelengrab's response]

[Seelengrab]

I object to the framing that I object - I have explicitly said that I don't mean to block this PR:

I don't mean to block this PR; as mentioned above, I'm torn on this and gave my reasoning for why so that a future person that does merge it has some additional input to consider.

There's no need to frame this as a me-against-you kind of thing.

Similarly, I don't claim that repring a Ptr is unsafe, just that there's hardly anything safe at all you can do with that reprd value once you re-eval it, so the printing as a non-parseable string serves as a reminder to be careful (in my view improving the user experience). That's all there is to my position :shrug: