Skip to content

Conversation

rossviljoen
Copy link
Contributor

No description provided.

Comment on lines 27 to 41
function CircularBuffer{T}(iter, capacity::Integer) where {T}
vec = copyto!(Vector{T}(undef,capacity), iter)
CircularBuffer{T}(1, length(iter),vec)
vec = copyto!(Vector{T}(undef, capacity), iter)
CircularBuffer{T}(1, length(iter), vec)
end

CircularBuffer(capacity::Integer) = CircularBuffer{Any}(capacity)

CircularBuffer{T}(capacity::Integer) where {T} = CircularBuffer{T}(T[],capacity)
CircularBuffer{T}(capacity::Integer) where {T} = CircularBuffer{T}(T[], capacity)

CircularBuffer(iter,capacity::Integer) = CircularBuffer{eltype(iter)}(iter,capacity)
CircularBuffer(iter, capacity::Integer) = CircularBuffer{eltype(iter)}(iter, capacity)

function CircularBuffer{T}(iter) where {T}
vec = reshape(collect(T,iter),:)
vec = reshape(collect(T, iter), :)
CircularBuffer{T}(1, length(vec), vec)
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are just formatting fixes

f <= length(buf) || throw(ArgumentError("Value of 'first' must be inbounds of buffer"))
len <= length(buf) || throw(ArgumentError("Value of 'length' must be <= length of buffer"))
return new{T}(length(buf), f, len, buf)
end

# Convert any `Integer` to whatever `Int` is on the relevant machine
CircularBuffer{T}(f::Integer, len::Integer, buf::Integer) where {T} = CircularBuffer{T}(Int(f), Int(len), Int(buf))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This constructor could be removed entirely, as it was just never called in the previous version. However it seems that #893 decided to keep it

@rossviljoen rossviljoen force-pushed the rv/circularbuffer-fixes branch from 1feac5b to b6058cf Compare August 26, 2025 07:50
@oxinabox
Copy link
Member

What is this fixing?
Can it have a test added that was failing before and now passes?

@rossviljoen
Copy link
Contributor Author

Nothing was actually broken before. The issue is that there is an inner constructor:
CircularBuffer{T}(f::Integer, len::Integer, buf::Integer) where {T} = CircularBuffer{T}(Int(f), Int(len), Int(buf))
which is incorrect, since buf is never an Integer/Int.

This constructor was already superfluous because of #893 (comment) , so there's no change in behaviour, but this PR just implements the original intention in that linked PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants