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

Api change: Replace Ptr[Byte] with Ptr[_] where applicable. #3753

Conversation

WojciechMazur
Copy link
Contributor

Typically consumers of opaque inputs (void* in C) are good candidates for using Ptr[_]. This allows for better DX allowing to skip redundant .asInstanceOf[Ptr[Byte]] conversion.
Allocators malloc, calloc are still returniong Ptr[Byte] becouse it's literally what they do - these function return pointer to first byte of allocated area.

… consumers of opaque inputs (void* in C) are good candidates for using Ptr[_]. Allocators `malloc`, `calloc` are still returniong `Ptr[Byte]` becouse it's literally what they do - these function return pointer to first byte of allocated area.
@ekrich
Copy link
Member

ekrich commented Feb 14, 2024

Is this a wildcard type essentially because in C void* is a pointer to anything. So would that be Ptr[?] in Scala 3?

@WojciechMazur
Copy link
Contributor Author

Is this a wildcard type essentially because in C void* is a pointer to anything. So would that be Ptr[?] in Scala 3?

Exactly. For Scala 2 and Scala 3 (under old syntax) Ptr[_] would be equivalent of void* It seems to fit Scala typesystem much better becouse of two reasons:

  • easy consumption of opaque types: e.g. memcpy, memset
  • protection against read of opaque type - Ptr[Any], and Ptr[_] cannot be loaded/assigned - there is no Tag for them. On Scala 2 there seems to be a Tag for Ptr[Nothing which is strange actually

While the other two Ptr[Any], Ptr[Nothing] could be used to protect against read/write of opaque data, these two would not be enough to easily pass pointers around. However, I'm open to suggestion if this change should be merged or for alternative approach

@ekrich
Copy link
Member

ekrich commented Feb 14, 2024

No, this makes perfect sense. I just wondered if you could use Ptr[?] for the Scala 3 continuations site and if I was understanding correctly. Thanks for the explanation.

@LeeTibbert
Copy link
Contributor

LGTM. This is a good step.

I checked, at a high level, that mentions in the documentation happened.

Looked like a couple of good type change/corrections in the network code. netdb.scala and
NetworkInterface (I think) caught my eye, but there were several other places: changing a Ptr[Byte] to a,
say, Ptr[addrinfo].

To make it easier for end users, one could consider adding as a teaching comment, just above the CString line:

"// type CVoidPointer = // The idiom for C void * is Ptr[_]"

/** The C/C++ 'void *' type; by convention, not declaration.
   *  type CVoidPointer =  Ptr[_] 
   */

That is where I have and would go looking for a suitable void * replacement.
One does not want to clutter the code; that is what doc is for and people
should read it. However, sometimes a clue about a very common transition
idiom is worth the noise.

If that appeals I can make a short PR after this merges. If you and/or other
reviewers decline the suggestion, I understand.

…low for easier migration if Ptr[_] would not be allowed anymore under Scala 3 at some point
@WojciechMazur
Copy link
Contributor Author

I've added the CVoidPtr type and adapted code to use it

@WojciechMazur WojciechMazur merged commit 540dfbb into scala-native:main Feb 14, 2024
60 of 62 checks passed
@WojciechMazur WojciechMazur deleted the api/replace-Ptr-byte]-with-Ptr-_- branch February 14, 2024 13:38
@LeeTibbert
Copy link
Contributor

LeeTibbert commented Feb 14, 2024

re: CVoidPtr

I like what you did with the place. Nice change, especially in posixlib.

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.

None yet

3 participants