Implemented mSlice on the VM allowing toOpenArray to work at compile time.
#20586
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
zerbina
reviewed
Oct 18, 2022
There was a problem hiding this comment.
We already discussed some of the changes via real-time chat, so I thought I'd do a second round of review here.
You also need to handle the case of creating slices from slices (e.g. toOpenArray(toOpenArray(x, 1, 4), 1, 2). My recommendation would be to prevent nested slices by unwrapping them as part of opcSlice.
The tyOpenArray handling in ccgexprs.genBracedInit also needs to be adjusted - it currently expects an nkBracket.
|
Irrelevant CI failure, merging. |
|
Thanks for your hard work on this PR! Hint: mm: orc; opt: speed; options: -d:release |
This was referenced Oct 21, 2022
narimiran
pushed a commit
that referenced
this pull request
Oct 24, 2022
…ile time. (#20586) * Implemented opcSlice to make 'toOpenArray' work on the VM * Added nkOpenArray for VM to reduce bodgeness * Fixed range issues and erraneous comments * Range check correctly for openArrays in opcLdArr * Inverted logic for ldArr checking * vm now supports slicing strings * Added string tests * Removed usage of 'nkOpenArray' and redundant operations * Refactored vmSlice implementation, removing redundant and incorrect code * Made tuples go throw opcWrObj for field assignment * All strkinds should be considered for openarrays (cherry picked from commit 4aa67ad)
capocasa
pushed a commit
to capocasa/Nim
that referenced
this pull request
Mar 31, 2023
…ile time. (nim-lang#20586) * Implemented opcSlice to make 'toOpenArray' work on the VM * Added nkOpenArray for VM to reduce bodgeness * Fixed range issues and erraneous comments * Range check correctly for openArrays in opcLdArr * Inverted logic for ldArr checking * vm now supports slicing strings * Added string tests * Removed usage of 'nkOpenArray' and redundant operations * Refactored vmSlice implementation, removing redundant and incorrect code * Made tuples go throw opcWrObj for field assignment * All strkinds should be considered for openarrays
bung87
pushed a commit
to bung87/Nim
that referenced
this pull request
Jul 29, 2023
…ile time. (nim-lang#20586) * Implemented opcSlice to make 'toOpenArray' work on the VM * Added nkOpenArray for VM to reduce bodgeness * Fixed range issues and erraneous comments * Range check correctly for openArrays in opcLdArr * Inverted logic for ldArr checking * vm now supports slicing strings * Added string tests * Removed usage of 'nkOpenArray' and redundant operations * Refactored vmSlice implementation, removing redundant and incorrect code * Made tuples go throw opcWrObj for field assignment * All strkinds should be considered for openarrays
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.
This is requisite for changes to the stdlib to use
openArray[char]more. There is an issue withstatic openArray[T], butoa[a..b]does work so can be used in place.