Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Delete unused arguments in subarray code #30789

Merged
merged 2 commits into from
Jan 23, 2019
Merged

Delete unused arguments in subarray code #30789

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a1ea610
Add nospecialize to unused arguments in subarray code
timholy Jan 21, 2019
00930a0
Delete unused arguments in subarray code
timholy Jan 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions base/deprecated.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,10 @@ function promote_eltype_op end

# @deprecate one(i::CartesianIndex) oneunit(i)
# @deprecate one(::Type{I}) where I<:CartesianIndex oneunit(I)

@deprecate reindex(V, idxs, subidxs) reindex(idxs, subidxs)
Copy link
Member

Choose a reason for hiding this comment

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

I think these should be commented out like the lines above it, since we can't introduce new deprecations until 2.0, if I'm not mistaken.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

These are internal functions, so they don't need deprecations at all unless we're aware that people are using them and we want to be nice.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

I'm not aware of any users, but this seemed harmless and better than throwing a MethodError.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

BTW _indices_sub is not deprecatable, so this doesn't try.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Friendly reminder to use a trailing false in cases like this to prevent unexported things from being exported. :)

#31201

@deprecate substrides(parent::AbstractArray, strds::Tuple, I::Tuple) substrides(strds, I)

# TODO: deprecate these
one(::CartesianIndex{N}) where {N} = one(CartesianIndex{N})
one(::Type{CartesianIndex{N}}) where {N} = CartesianIndex(ntuple(x -> 1, Val(N)))
Expand Down
53 changes: 26 additions & 27 deletions base/subarray.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ _maybe_reindex(V, I, A::Tuple{AbstractArray{<:AbstractCartesianIndex{1}}, Vararg
_maybe_reindex(V, I, A::Tuple{Any, Vararg{Any}}) = (@_inline_meta; _maybe_reindex(V, I, tail(A)))
function _maybe_reindex(V, I, ::Tuple{})
@_inline_meta
@inbounds idxs = to_indices(V.parent, reindex(V, V.indices, I))
@inbounds idxs = to_indices(V.parent, reindex(V.indices, I))
SubArray(V.parent, idxs)
end

Expand All @@ -191,32 +191,32 @@ end

AbstractZeroDimArray{T} = AbstractArray{T, 0}

reindex(V, ::Tuple{}, ::Tuple{}) = ()
reindex(::Tuple{}, ::Tuple{}) = ()

# Skip dropped scalars, so simply peel them off the parent indices and continue
reindex(V, idxs::Tuple{ScalarIndex, Vararg{Any}}, subidxs::Tuple{Vararg{Any}}) =
(@_propagate_inbounds_meta; (idxs[1], reindex(V, tail(idxs), subidxs)...))
reindex(idxs::Tuple{ScalarIndex, Vararg{Any}}, subidxs::Tuple{Vararg{Any}}) =
(@_propagate_inbounds_meta; (idxs[1], reindex(tail(idxs), subidxs)...))

# Slices simply pass their subindices straight through
reindex(V, idxs::Tuple{Slice, Vararg{Any}}, subidxs::Tuple{Any, Vararg{Any}}) =
(@_propagate_inbounds_meta; (subidxs[1], reindex(V, tail(idxs), tail(subidxs))...))
reindex(idxs::Tuple{Slice, Vararg{Any}}, subidxs::Tuple{Any, Vararg{Any}}) =
(@_propagate_inbounds_meta; (subidxs[1], reindex(tail(idxs), tail(subidxs))...))

# Re-index into parent vectors with one subindex
reindex(V, idxs::Tuple{AbstractVector, Vararg{Any}}, subidxs::Tuple{Any, Vararg{Any}}) =
(@_propagate_inbounds_meta; (idxs[1][subidxs[1]], reindex(V, tail(idxs), tail(subidxs))...))
reindex(idxs::Tuple{AbstractVector, Vararg{Any}}, subidxs::Tuple{Any, Vararg{Any}}) =
(@_propagate_inbounds_meta; (idxs[1][subidxs[1]], reindex(tail(idxs), tail(subidxs))...))

# Parent matrices are re-indexed with two sub-indices
reindex(V, idxs::Tuple{AbstractMatrix, Vararg{Any}}, subidxs::Tuple{Any, Any, Vararg{Any}}) =
(@_propagate_inbounds_meta; (idxs[1][subidxs[1], subidxs[2]], reindex(V, tail(idxs), tail(tail(subidxs)))...))
reindex(idxs::Tuple{AbstractMatrix, Vararg{Any}}, subidxs::Tuple{Any, Any, Vararg{Any}}) =
(@_propagate_inbounds_meta; (idxs[1][subidxs[1], subidxs[2]], reindex(tail(idxs), tail(tail(subidxs)))...))

# In general, we index N-dimensional parent arrays with N indices
@generated function reindex(V, idxs::Tuple{AbstractArray{T,N}, Vararg{Any}}, subidxs::Tuple{Vararg{Any}}) where {T,N}
@generated function reindex(idxs::Tuple{AbstractArray{T,N}, Vararg{Any}}, subidxs::Tuple{Vararg{Any}}) where {T,N}
if length(subidxs.parameters) >= N
subs = [:(subidxs[$d]) for d in 1:N]
tail = [:(subidxs[$d]) for d in N+1:length(subidxs.parameters)]
:(@_propagate_inbounds_meta; (idxs[1][$(subs...)], reindex(V, tail(idxs), ($(tail...),))...))
:(@_propagate_inbounds_meta; (idxs[1][$(subs...)], reindex(tail(idxs), ($(tail...),))...))
else
:(throw(ArgumentError("cannot re-index $(ndims(V)) dimensional SubArray with fewer than $(ndims(V)) indices\nThis should not occur; please submit a bug report.")))
:(throw(ArgumentError("cannot re-index SubArray with fewer indices than dimensions\nThis should not occur; please submit a bug report.")))
end
end

Expand All @@ -225,7 +225,7 @@ SlowSubArray{T,N,P,I} = SubArray{T,N,P,I,false}
function getindex(V::SubArray{T,N}, I::Vararg{Int,N}) where {T,N}
@_inline_meta
@boundscheck checkbounds(V, I...)
@inbounds r = V.parent[reindex(V, V.indices, I)...]
@inbounds r = V.parent[reindex(V.indices, I)...]
r
end

Expand Down Expand Up @@ -266,7 +266,7 @@ end
function setindex!(V::SubArray{T,N}, x, I::Vararg{Int,N}) where {T,N}
@_inline_meta
@boundscheck checkbounds(V, I...)
@inbounds V.parent[reindex(V, V.indices, I)...] = x
@inbounds V.parent[reindex(V.indices, I)...] = x
V
end
function setindex!(V::FastSubArray, x, i::Int)
Expand Down Expand Up @@ -299,14 +299,13 @@ IndexStyle(::Type{<:SubArray}) = IndexCartesian()

# Strides are the distance in memory between adjacent elements in a given dimension
# which we determine from the strides of the parent
strides(V::SubArray) = substrides(V.parent, V.indices)
strides(V::SubArray) = substrides(strides(V.parent), V.indices)

substrides(parent, I::Tuple) = substrides(parent, strides(parent), I)
substrides(parent, strds::Tuple{}, ::Tuple{}) = ()
substrides(parent, strds::NTuple{N,Int}, I::Tuple{ScalarIndex, Vararg{Any}}) where N = (substrides(parent, tail(strds), tail(I))...,)
substrides(parent, strds::NTuple{N,Int}, I::Tuple{Slice, Vararg{Any}}) where N = (first(strds), substrides(parent, tail(strds), tail(I))...)
substrides(parent, strds::NTuple{N,Int}, I::Tuple{AbstractRange, Vararg{Any}}) where N = (first(strds)*step(I[1]), substrides(parent, tail(strds), tail(I))...)
substrides(parent, strds, I::Tuple{Any, Vararg{Any}}) = throw(ArgumentError("strides is invalid for SubArrays with indices of type $(typeof(I[1]))"))
substrides(strds::Tuple{}, ::Tuple{}) = ()
substrides(strds::NTuple{N,Int}, I::Tuple{ScalarIndex, Vararg{Any}}) where N = (substrides(tail(strds), tail(I))...,)
substrides(strds::NTuple{N,Int}, I::Tuple{Slice, Vararg{Any}}) where N = (first(strds), substrides(tail(strds), tail(I))...)
substrides(strds::NTuple{N,Int}, I::Tuple{AbstractRange, Vararg{Any}}) where N = (first(strds)*step(I[1]), substrides(tail(strds), tail(I))...)
substrides(strds, I::Tuple{Any, Vararg{Any}}) = throw(ArgumentError("strides is invalid for SubArrays with indices of type $(typeof(I[1]))"))

stride(V::SubArray, d::Integer) = d <= ndims(V) ? strides(V)[d] : strides(V)[end] * size(V)[end]

Expand Down Expand Up @@ -394,12 +393,12 @@ end
# indices are taken from the range/vector
# Since bounds-checking is performance-critical and uses
# indices, it's worth optimizing these implementations thoroughly
axes(S::SubArray) = (@_inline_meta; _indices_sub(S, S.indices...))
_indices_sub(S::SubArray) = ()
_indices_sub(S::SubArray, ::Real, I...) = (@_inline_meta; _indices_sub(S, I...))
function _indices_sub(S::SubArray, i1::AbstractArray, I...)
axes(S::SubArray) = (@_inline_meta; _indices_sub(S.indices...))
_indices_sub(::Real, I...) = (@_inline_meta; _indices_sub(I...))
_indices_sub() = ()
function _indices_sub(i1::AbstractArray, I...)
@_inline_meta
(unsafe_indices(i1)..., _indices_sub(S, I...)...)
(unsafe_indices(i1)..., _indices_sub(I...)...)
end

## Compatibility
Expand Down