julia
julia copied to clipboard
reimplement `_setindex` using recursion
FTR, this PR is part of a series of changes which (re)implement many of the operations on tuples using a new recursive technique. The ultimate goal is to hopefully increase the value of the loop-vs-recurse cutoff (Any32, sometimes hardcoded 32) for tuple operations.
Updates #54462
Closes #54463
Example type inference improvement:
julia> Core.Compiler.return_type(Base.setindex, Tuple{Tuple{Vararg{Int}},Int,Int})
Tuple{Vararg{Int64}}
It is difficult to understand the relationship to the issue and prior PR. (typo?)
from https://github.com/JuliaLang/julia/pull/54463#issuecomment-2111227953
(I think it is also misleading that the function
_setindexexists inBase. Can't we use_setindex_tupleor something like that?)
I think there are two problems with the current (master) _setindex that have nothing to do with the inference.
- Although the
Base.setindexis also implemented byNamedTuple, etc., the signature does not indicate that it is forTuple(due to the splatting). - The order of arguments is different from
setindexdue to theVarargtechnique.
If we don't use Vararg, I don't see the need to change the order of the arguments from setindex.
I think the true NTuple cases (x::NTuple{N, T}, v::T, i::Integer) where {N, T} are worth specializing. It is easier to understand for humans as well as the compiler.
Regarding the naming of the functions, that's way off-topic IMO (to this PR).
I think the true
NTuplecases...are worth specializing
Why? As shown above, the generic implementation seems to handle this just fine.
Regarding the naming of the functions, that's way off-topic IMO (to this PR).
Agreed. However, we would probably lose the opportunity to change it.
I think the true NTuple cases ... are worth specializing
Why?
As noted above, it is because it is easy for humans to understand. TBH, I still don't understand this PR. Also, I believe that not only inference but also runtime performance is important. Some benchmarks would be helpful.
If we were to try to support Base.setindex((1, 2, 3), 0, 2:3), would this PR technique work well? (Edit: cf. #54508)
Of course that has nothing to do with this PR, but I don't think having a simple implementation is a bad thing.