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 UntypedPointer as a superclass of Pointer #10749

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

HertzDevil
Copy link
Contributor

This is a proof-of-concept implementation for #10657 (comment).

UntypedPointer has no support for pointer arithmetic at all and cannot be dereferenced; other than that, it has the same semantics as Pointer(Void), and the two types should be interchangeable. Pointer is unique in that the union of a Pointer(T) instance with another Pointer(U) or UntypedPointer does not merge into UntypedPointer.

Code that uses Pointer(Void) shall continue to work for the rest of 1.x; this PR does not apply UntypedPointer to the standard library yet, because it is a primitive type and the 1.0 compiler has no knowledge of its existence.

@straight-shoota
Copy link
Member

straight-shoota commented May 27, 2021

This looks great. Limiting the interface of a void pointer is a good idea. This is a step towards removing Void as proposed in #10657. Although not technically necessary for that, I think it's far better approach than artificially limiting the use of Void to generic type arguments.

As for the time table, I assume we can merge this (as soon as it's ironed out), then deprecate Pointer(Void) and migrate stdlib, and finally implement #10657 by completely removing Void from the language outside lib definitions (or then probably entirely?).

As a tiny detail, I'd prefer a namespaced name: Pointer::Untyped, or maybe Pointer::Void?

@asterite
Copy link
Member

Thanks for this!

A couple of comments:

  • I personally don't understand what's the advantage of introducing a new pointer type instead of using Pointer(Void), which is only used in C bindings and so unfrequent. People will have to learn a new concept.
  • I'm not sure how it currently works, but if struct Pointer(T) < UntypedPointer then by how the language works UntypedPointer must be an abstract struct. You can't inherit a non-abstract struct. So UntypedPointer is actually a union of UntypedPointer and every Pointer(T) which doesn't feel right. So this instead should be: struct AbstractPointer, Pointer(T) < AbstractPointer, UntypedPointer < AbstractPointer. Though I'm still not convinced we need a new pointer type.

@asterite
Copy link
Member

I'd prefer a namespaced name: Pointer::Untyped

Please, let's not continue names that are semantically wrong. Pointer::Untyped looks like it's an untyped thing inside the Pointer namespace, which doesn't read good. Similar to IO::Memory, which is not a memory (it's an IO). Though I guess at this pointer we should just continue the (wrong) tradition.

@straight-shoota
Copy link
Member

@asterite The advantage is that it allows us to get rid of Void, which is really a weird concept in Crystal's type system. As a stand-alone value it's practically equivalent to Nil in most situations. There's no reason for having Void for anything except Pointer(Void), hence the idea is to make Pointer(Void) a discrete type.
IMO the additional cognitive load is negligible. And it only affects working with C interop, that's a rather small surface area.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants