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

[stdlib] Refactor uses of StaticTuple to use InlineArray #2514

Closed
JoeLoser opened this issue May 4, 2024 · 4 comments
Closed

[stdlib] Refactor uses of StaticTuple to use InlineArray #2514

JoeLoser opened this issue May 4, 2024 · 4 comments
Labels
good first issue Good for newcomers mojo-repo Tag all issues with this label mojo-stdlib Tag for issues related to standard library refactoring Refactoring/clean up of code

Comments

@JoeLoser
Copy link
Collaborator

JoeLoser commented May 4, 2024

We recently got a generic array type (InlineArray) (see here) that works on memory-only types rather than AnyRegType like StaticTuple works on. So, conceptually, InlineArray can replace all of these uses.

This issue tracks doing the work of replacing uses of StaticTuple with InlineArray so we can eventually remove StaticTuple and then re-home the InlineArray into the collections module where it belong.

@JoeLoser JoeLoser added good first issue Good for newcomers mojo-stdlib Tag for issues related to standard library refactoring Refactoring/clean up of code labels May 4, 2024
@vidur2
Copy link

vidur2 commented May 5, 2024

Do you have any idea how to run the testing suite? Is the 'Test Examples' workflow comprehensive?

@artemiogr97
Copy link
Contributor

artemiogr97 commented May 6, 2024

@JoeLoser there are some differences between StaticTuple and InlineArray so I could not replace it in most of the places:

  • In os._linux_aarch64, os._linux_x86, and os._macos StaticTuple is used for the last unused member of _c_stat which is register_passable and so all members must be register passable, but InlineArray is not register passable (not sure if we can add the decorator)
  • in collections.vector, utils.inlined_string StaticTuple is instantiated with uninitialized values which is forbidden for InlinedArray, maybe a staticmethod undefined_init could be created for those cases? I think keeping the init that causes a compile time error makes sense, should we allow to create an uninitialized array somehow something like __init__(*, uninitialized: Bool) such that it can not easily be created by accident
  • the other uses are either in StaticIntTuple or in the tests, so to be removed at the end once they are no longer used

@ematejska ematejska added mojo-repo Tag all issues with this label and removed mojo-stdlib Tag for issues related to standard library labels May 6, 2024
@ematejska ematejska added the mojo-stdlib Tag for issues related to standard library label May 7, 2024 — with Linear
@ConnorGray
Copy link
Collaborator

Hi @artemiogr97, re. your second question:

in collections.vector, utils.inlined_string StaticTuple is instantiated with uninitialized values which is forbidden for InlinedArray

Yes, it would be good for us to preserve a way to create uninitialized InlinedArray values.

We may want to go a different direction down the line, but I think your suggestion for a __init__(*, uninitialized: Bool) seems like a reasonable solution to try for the time being🙂

JoeLoser pushed a commit to JoeLoser/mojo that referenced this issue May 8, 2024
[External] [stdlib] remove uses of StaticTuple where possible

Fix for modularml#2514

ORIGINAL_AUTHOR=artemiogr97
<57588855+artemiogr97@users.noreply.github.com>
PUBLIC_PR_LINK=modularml#2583

---------

Co-authored-by: artemiogr97 <57588855+artemiogr97@users.noreply.github.com>
Closes modularml#2583
MODULAR_ORIG_COMMIT_REV_ID: 690cbe5148e583e1f86638f251fab93a1c72a948
JoeLoser pushed a commit that referenced this issue May 8, 2024
[External] [stdlib] remove uses of StaticTuple where possible

Fix for #2514

ORIGINAL_AUTHOR=artemiogr97
<57588855+artemiogr97@users.noreply.github.com>
PUBLIC_PR_LINK=#2583

---------

Co-authored-by: artemiogr97 <57588855+artemiogr97@users.noreply.github.com>
Closes #2583
MODULAR_ORIG_COMMIT_REV_ID: 690cbe5148e583e1f86638f251fab93a1c72a948
@JoeLoser
Copy link
Collaborator Author

JoeLoser commented May 9, 2024

Fixed with 6af1fa7. Thank you!

@JoeLoser JoeLoser closed this as completed May 9, 2024
lsh pushed a commit to lsh/mojo that referenced this issue May 17, 2024
[External] [stdlib] remove uses of StaticTuple where possible

Fix for modularml#2514

ORIGINAL_AUTHOR=artemiogr97
<57588855+artemiogr97@users.noreply.github.com>
PUBLIC_PR_LINK=modularml#2583

---------

Co-authored-by: artemiogr97 <57588855+artemiogr97@users.noreply.github.com>
Closes modularml#2583
MODULAR_ORIG_COMMIT_REV_ID: 690cbe5148e583e1f86638f251fab93a1c72a948

Signed-off-by: Lukas Hermann <lukashermann28@gmail.com>
modularbot added a commit that referenced this issue Jun 7, 2024
[External] [stdlib] remove uses of StaticTuple where possible

Fix for #2514

ORIGINAL_AUTHOR=artemiogr97
<57588855+artemiogr97@users.noreply.github.com>
PUBLIC_PR_LINK=#2583

---------

Co-authored-by: artemiogr97 <57588855+artemiogr97@users.noreply.github.com>
Closes #2583
MODULAR_ORIG_COMMIT_REV_ID: 690cbe5148e583e1f86638f251fab93a1c72a948
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 mojo-repo Tag all issues with this label mojo-stdlib Tag for issues related to standard library refactoring Refactoring/clean up of code
Projects
None yet
Development

No branches or pull requests

5 participants