view should throw BoundsError

[bjarthur]

am i thinking about this wrong, or is the fact that indexing the 4th column of a buffer of length 3 does not throw an error a bug?

julia> using CircularArrayBuffers

julia> b = CircularArrayBuffer([1 2 3; 4 5 6])
CircularArrayBuffer(::Matrix{Int64}) with eltype Int64:
 1  2  3
 4  5  6

julia> @view b[:,4]          ### !!! shouldn't this throw a BoundsError ???
2-element view(::Matrix{Int64}, :, 1) with eltype Int64:
 1
 4

julia> @view b[:,7]      ### it finally does here once the length of the buffer has been exceeded
ERROR: BoundsError: attempt to access 2×3 Matrix{Int64} at index [1:2, 4]
Stacktrace:
 [1] throw_boundserror(A::Matrix{Int64}, I::Tuple{Base.Slice{Base.OneTo{Int64}}, Int64})
   @ Base ./abstractarray.jl:737
 [2] checkbounds
   @ ./abstractarray.jl:702 [inlined]
 [3] view
   @ ./subarray.jl:184 [inlined]
 [4] view(::CircularArrayBuffer{Int64, 2, Matrix{Int64}}, ::Function, ::Int64)
   @ CircularArrayBuffers ~/.julia/dev/CircularArrayBuffers/src/CircularArrayBuffers.jl:73
 [5] top-level scope
   @ REPL[6]:1

(jl_gK1hOx) pkg> st
Status `/tmp/jl_gK1hOx/Project.toml`
  [9de3a189] CircularArrayBuffers v0.1.12 `~/.julia/dev/CircularArrayBuffers`

julia> versioninfo()
Julia Version 1.10.2
Commit bd47eca2c8a (2024-03-01 10:14 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 64 × Intel(R) Xeon(R) CPU E5-2683 v4 @ 2.10GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, broadwell)
Threads: 1 default, 0 interactive, 1 GC (on 64 virtual cores)
Environment:
  LD_LIBRARY_PATH = /misc/lsf/10.1/linux3.10-glibc2.17-x86_64/lib
  JULIA_PROJECT = @.
  JULIA_DEPOT_PATH = /groups/scicompsoft/home/arthurb/.julia:

[jeremiahpslewis]

@CasBex this looks like a bug, do you know why we use _buffer_frame in the getindex functions?

[CasBex]

I don't recall working on this particular function but as far as I'm aware. What I can see from looking at the code

• _buffer_frame is used as a getindex method because the first index of the CircularArray is mutable (for fast rotation). It wraps the index in the lookup to get the result from the internal buffer of the CircularArray.
• _buffer_frame does not check for out-of bounds but uses either start+i or start+i-length on the internal array CircularArray.buffer. i.e. it only throws out of bounds when start+i-length is out of bounds of the internal array.

Quick solution is to just put a check in _buffer_frame I guess.

[findmyway]

I think this is a bug, we should add boundscheck here.

[bjarthur]

thinking on this further, the _buffer_frame code looks like it was intended to circularize the indexing by wrapping around to the beginning of the buffer if the index was greater than the capacity. but as written, it only does this once. seems to me it would be better to not subtract the buffer length if the index exceeds it, but rather use mod(idx,n). then it would truly be a circular buffer, for arbitrarily large indices, as the name of this package implies.

[findmyway]

thinking on this further, the _buffer_frame code looks like it was intended to circularize the indexing by wrapping around to the beginning of the buffer if the index was greater than the capacity. but as written, it only does this once. seems to me it would be better to not subtract the buffer length if the index exceeds it, but rather use mod(idx,n). then it would truly be a circular buffer, for arbitrarily large indices, as the name of this package implies.

I barely remember that was intentional.

1. mod(idx,n) is slow (compared to subtraction)
2. By design, the input to _buffer_frame should never be larger than the capacity of underlying buffer.

Although supporting arbitrarily large indices will make it more circular. I prefer keeping existing implementation:

1. In that case, how would we define the size of the buffer?
2. It is error-prone to access unexpected elements

[jeremiahpslewis]

Maybe this helps to make it explicit? #22

[jeremiahpslewis]

@findmyway If you had a moment to write a docstring for the CircularArrayBuffer struct elements, I'd be grateful, find it hard to think through what the different elements are doing.