Skip to content

on the setindex! method #137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nsajko opened this issue Dec 30, 2024 · 1 comment
Open

on the setindex! method #137

nsajko opened this issue Dec 30, 2024 · 1 comment

Comments

@nsajko
Copy link
Contributor

nsajko commented Dec 30, 2024

I don't think you're allowed to do this:

# NOTE: for some reason, merely overloading this function causes allocations
# in Julia 1.9. This is not the case in 1.10, so presumably this is a bug.
# Therefore, allocation tests only are performed on >=1.10
function Base.setindex!(A::Array{T}, x::TempTPS, i1::Int) where {T<:TPS}
copy!(A[i1], x)
rel_temp!(x)
end

I think this only works "by accident", e.g., I suppose it could break with any Julia release, if Base decides to change its implementation regarding setindex! for Array.

It seems like type piracy, in some sense. Or perhaps it's function punning that might be confusing to users? Not sure TBH, but it definitely feels very bad.

There's another potential issue with the method signature, though an easily fixable one: it matches setindex!(::Array{Union{}}, ::TempTPS, ::Int), which is surely not intended. Fix: #136

@mattsignorelli
Copy link
Contributor

The intention here is to basically overload .= for setting an array of TPS equal to an array of TempTPS so that there are zero allocations of new TPS's - the TPSs originally in the array on the LHS are just changed in-place to be equal those by the temporaries.

This feature is useful for such mutable number types, but if you can think of a better way to handle this I would be happy to implement it.

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

No branches or pull requests

2 participants