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

[BUG] Unify the constructor signature for pointer-based initialization across multiple types #3704

Open
soraros opened this issue Oct 22, 2024 · 2 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@soraros
Copy link
Contributor

soraros commented Oct 22, 2024

That said, the unsafe constructors from pointers are really a mess, and I find myself having to look up the keyword names every time. Could we adopt a convention like fn __init__(inout self, *, ptr: UnsafePointer, len: Int)?

  • String: fn __init__(inout self, ptr: UnsafePointer[UInt8], len: Int):
  • StringRef: fn __init__(inout self, ptr: UnsafePointer[c_char], len: Int):
  • StringSlice: fn __init__(inout self, *, unsafe_from_utf8_ptr: UnsafePointer[UInt8], len: Int):
  • Span: fn __init__(inout self, *, unsafe_ptr: UnsafePointer[T], len: Int):
  • List: fn __init__(inout self, *, unsafe_pointer: UnsafePointer[T], size: Int, capacity: Int):

Agreed, they're wildly inconsistent. +1 to your proposed convention - feel free to open a PR. I'm also +1 for making them all (including String kw-only for the ptr + len constructor until we fix implicit constructor issue you linked).

Originally posted by @JoeLoser in #3698 (comment)

@JoeLoser JoeLoser added good first issue Good for newcomers help wanted Extra attention is needed labels Oct 22, 2024 — with Linear
@martinvuyk
Copy link
Contributor

I agree that it's necessary, but I'll be a bit nit-picky and very subjective: since we are defining a new standard, I wanted to say that I strongly dislike using len as argument since it is the name of a builtin function. I personally prefer going for length.

@JoeLoser
Copy link
Collaborator

I agree that it's necessary, but I'll be a bit nit-picky and very subjective: since we are defining a new standard, I wanted to say that I strongly dislike using len as argument since it is the name of a builtin function. I personally prefer going for length.

Agreed given that argument. I'd be fine with length.

modularbot added a commit that referenced this issue Nov 8, 2024
…uctor args (#50485)

[External] [stdlib] [NFC] Rename `StringSlice` `UnsafePointer`
constructor args

Rename `StringSlice` `UnsafePointer` constructor args. Part of
#3704.

ORIGINAL_AUTHOR=martinvuyk
<[email protected]>
PUBLIC_PR_LINK=#3740

Co-authored-by: martinvuyk <[email protected]>
Closes #3740
MODULAR_ORIG_COMMIT_REV_ID: 66fcc1d96c46dcd4b8d1b872b4da38843b014995
modularbot added a commit to modularml/max that referenced this issue Nov 9, 2024
… args (#50486)

[External] [stdlib] [NFC] Rename `String` `UnsafePointer` constructor
args

Rename `String` `UnsafePointer` constructor args. Part of
modularml/mojo#3704.

ORIGINAL_AUTHOR=martinvuyk
<[email protected]>
PUBLIC_PR_LINK=modularml/mojo#3741

Co-authored-by: martinvuyk <[email protected]>
Closes modularml/mojo#3741
MODULAR_ORIG_COMMIT_REV_ID: 7c043cc298711e9d981a61a80693ca4b7278f4c0
modularbot added a commit that referenced this issue Nov 9, 2024
… args (#50486)

[External] [stdlib] [NFC] Rename `String` `UnsafePointer` constructor
args

Rename `String` `UnsafePointer` constructor args. Part of
#3704.

ORIGINAL_AUTHOR=martinvuyk
<[email protected]>
PUBLIC_PR_LINK=#3741

Co-authored-by: martinvuyk <[email protected]>
Closes #3741
MODULAR_ORIG_COMMIT_REV_ID: 7c043cc298711e9d981a61a80693ca4b7278f4c0
modularbot added a commit that referenced this issue Nov 9, 2024
…rgs (#50488)

[External] [stdlib] [NFC] Rename `Span` `UnsafePointer` constructor args

Rename `Span` `UnsafePointer` constructor args. Part of
#3704.

ORIGINAL_AUTHOR=martinvuyk
<[email protected]>
PUBLIC_PR_LINK=#3743

Co-authored-by: martinvuyk <[email protected]>
Closes #3743
MODULAR_ORIG_COMMIT_REV_ID: 4845b40b59eaa55827b4ee42240b7f9361d4a608
modularbot added a commit that referenced this issue Nov 9, 2024
…rgs (#50487)

[External] [stdlib] [NFC] Rename `List` `UnsafePointer` constructor args

Rename `List` `UnsafePointer` constructor args. Part of
#3704.

ORIGINAL_AUTHOR=martinvuyk
<[email protected]>
PUBLIC_PR_LINK=#3742

Co-authored-by: martinvuyk <[email protected]>
Closes #3742
MODULAR_ORIG_COMMIT_REV_ID: f4e85d1d2a78c7cc81860b9757897bd5cf230f06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants