Skip to content

Add support for negative start index in Slice#[start, count]#14778

Merged
straight-shoota merged 3 commits intocrystal-lang:masterfrom
ysbaddaden:fix/subslice-with-negative-start
Aug 7, 2024
Merged

Add support for negative start index in Slice#[start, count]#14778
straight-shoota merged 3 commits intocrystal-lang:masterfrom
ysbaddaden:fix/subslice-with-negative-start

Conversation

@ysbaddaden
Copy link
Collaborator

@ysbaddaden ysbaddaden commented Jul 4, 2024

Allows slice[-3, 3] to return the last 3 elements of the slice, for example, similar to how Array#[-start, count] behaves, with the difference that Slice returns exactly count elements, while Array returns up to count elements.

Introduces a couple changes:

  • negative start now returns from the end of the slice instead of returning nil/raising IndexError
  • negative count now raises ArgumentError instead of returning nil/raising IndexError

I believe the current behavior is buggy (unexpected result, underspecified documentation), but this can be considered a breaking change.

refs #14775

@ysbaddaden ysbaddaden self-assigned this Jul 4, 2024
@straight-shoota straight-shoota added kind:feature breaking-change May have breaking effect in some edge cases. Mostly negligible, but still noteworthy. labels Jul 4, 2024
Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Describe this additional behaviour in the API docs.

slice1.should_not be_nil
slice1 = slice1.not_nil!
slice1.size.should eq(2)
slice1.to_unsafe.should eq(slice.to_unsafe + 1)
Copy link
Collaborator Author

@ysbaddaden ysbaddaden Jul 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I added tests to verify that the subslice is pointing to the same memory as the original slice, and not returning a copy. This is the expected behavior, but we didn't check it.

@straight-shoota straight-shoota added this to the 1.14.0 milestone Jul 4, 2024
@straight-shoota straight-shoota changed the title Slice#[start, count]: support negative start Add support for negative start index in Slice#[start, count] Jul 4, 2024
@straight-shoota straight-shoota merged commit d738ac9 into crystal-lang:master Aug 7, 2024
@ysbaddaden ysbaddaden deleted the fix/subslice-with-negative-start branch August 8, 2024 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change May have breaking effect in some edge cases. Mostly negligible, but still noteworthy. kind:feature topic:stdlib:collection

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants