Skip to content

Fix Array(T)#[]=(Int, Int, Array(T)) on shifted arrays#13275

Merged
straight-shoota merged 1 commit intocrystal-lang:masterfrom
HertzDevil:bug/array-replace-subrange
Apr 5, 2023
Merged

Fix Array(T)#[]=(Int, Int, Array(T)) on shifted arrays#13275
straight-shoota merged 1 commit intocrystal-lang:masterfrom
HertzDevil:bug/array-replace-subrange

Conversation

@HertzDevil
Copy link
Contributor

@HertzDevil HertzDevil commented Apr 4, 2023

This is like #13256, except for #[]= whenever the size of the array increases:

class Array(T)
  def inspect(io : IO) : Nil
    io << "["
    @capacity.times do |i|
      io << ", " if i > 0
      if 0 <= i - @offset_to_buffer < @size
        @buffer[i - @offset_to_buffer].inspect(io)
      else
        io << '_'
      end
    end
    io << ']'
  end
end

# element lost after reallocation
# also notice that the buffer unnecessarily shrinks
x = [0, 1, 2, 3, 4] # [0, 1, 2, 3, 4]
x << 5              # [0, 1, 2, 3, 4, 5, _, _, _, _]
x.shift             # [_, 1, 2, 3, 4, 5, _, _, _, _]
x[5, 0] = [6, 7, 8] # [_, 1, 2, 3, 4, 5, 6, 7]

# element lost without reallocation (this is not true, as
# the call to `Pointer#realloc` is unconditional)
x = [0, 1, 2, 3]       # [0, 1, 2, 3]
x << 4                 # [0, 1, 2, 3, 4, _, _, _]
x.shift                # [_, 1, 2, 3, 4, _, _, _]
x[4, 0] = [5, 6, 7, 8] # [_, 1, 2, 3, 4, 5, 6, 7]

After this PR:

x = [0, 1, 2, 3, 4]
x << 5
x.shift
x[5, 0] = [6, 7, 8] # [_, 1, 2, 3, 4, 5, 6, 7, 8, _]

x = [0, 1, 2, 3]
x << 4
x.shift
x[4, 0] = [5, 6, 7, 8] # [_, 1, 2, 3, 4, 5, 6, 7, 8, _, _, _, _, _, _, _]

This indirectly affects the Range overload as well, which forwards to this overload.

Also as a result of this fix, all resizing operations on Array now grow the buffer consistently using the same policy, unlike previously where some still use Math.pw2ceil.

@HertzDevil HertzDevil added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:collection labels Apr 4, 2023
@straight-shoota straight-shoota added this to the 1.8.0 milestone Apr 4, 2023
@straight-shoota straight-shoota merged commit 004c6ac into crystal-lang:master Apr 5, 2023
@HertzDevil HertzDevil deleted the bug/array-replace-subrange branch April 6, 2023 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:collection

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants