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] Make ListLiteral subscriptable, and remove get[i, T](). #3991

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jjvraw
Copy link
Contributor

@jjvraw jjvraw commented Feb 8, 2025

Prevent crashes similar to that seen with Tuple.get (#3935). Also addressing #319, introducing __getitem__ by forwarding Tuple.__getitem__.

stdlib/src/builtin/object.mojo Outdated Show resolved Hide resolved
stdlib/src/builtin/object.mojo Outdated Show resolved Hide resolved
Signed-off-by: Joshua James Venter <[email protected]>
Signed-off-by: Joshua James Venter <[email protected]>
Signed-off-by: Joshua James Venter <[email protected]>
@jjvraw jjvraw changed the title [stdlib] Mark ListLiteral.get as unsafe and introduce __getitem__. [stdlib] Make ListLiteral subscriptable, and remove get[i, T](). Feb 13, 2025
@rd4com
Copy link
Contributor

rd4com commented Feb 14, 2025

Hello, that one worked if need to refine this one: #3835 👍

If needed, just re-use any part of the implementation and the other PR can be closed 👍

@jjvraw
Copy link
Contributor Author

jjvraw commented Feb 16, 2025

Hi @rd4com,

Apologies for duplicating your PR! If you'd prefer, you could update python_object.mojo & object.mojo to use ListLiteral.__getitem__ directly. I could then either wait for that PR to be pulled through, or merge with yours remotely such that this PR is focused on removing get.

@rd4com
Copy link
Contributor

rd4com commented Feb 16, 2025

No need to apologize this is awesome i love it 👍
just check how return type uses __type_of so that the inner tuple can do it all
The other pr i created can be closed if needed, just there for you if needed ❤️‍🔥

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.

3 participants