[stdlib] use __refitem__ in List
#2544
Closed
+29
−65
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Michael Kowalski <1331470+mikowals@users.noreply.github.com>
Signed-off-by: Michael Kowalski <1331470+mikowals@users.noreply.github.com>
JoeLoser
approved these changes
May 10, 2024
| @@ -727,7 +727,7 @@ fn _get_runtime_dtype_size(type: DType) -> Int: | |||
| statically known dtypes. Instead, we have to perform a dispatch to | |||
| determine the size of the dtype. | |||
| """ | |||
| alias type_list = List[DType]( | |||
| alias type_list = StaticTuple[DType, 16]( | |||
There was a problem hiding this comment.
Suggestion Use InlineArray instead. We're about to mark StaticTuple as deprecated and soon remove it.
There was a problem hiding this comment.
InlineArray gets the error: "cannot use a dynamic value in alias initializer". From the line alias concrete_type = type_list[idx]. This is the same error List also gets with the nightly 2024.5.1002. I assume because InlineArray and List make very similar use of __refitem__.
|
No longer planned as the functionality of |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
attempts #2432. Encountered a couple of issues:
__refitem__fails to compile with "value doesn't have a memory type" #2543 when using List in an alias. Worked around by using a StaticTuple in place of List. Was required in only one place.__getitem__(self, Slice)with__refitem__gives callee expects 2 parameters, but 0 were specified #2540 when usingvec[::1]or any other Slice. Can be worked around by__getitem__(slice(...))in many cases but I don't think we want to do that. And thatslicesyntax requires the endpoints so it isn't a workaround in all cases.For special attention during review, I changed explicit use of
__refitem__in other modules likeDictto just use vanillafoo[index]. I think those calls existed to avoid copies which should be default behaviour with__refitem__wiring. Sovar ev = self._entries.__get_ref(index)[]becameself.entries[index]. Where specific mutability and lifetimes were being passed I kept the longform use of__refitem__.