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

Make similar faster and safer #18107

Merged
merged 2 commits into from
Aug 19, 2016
Merged

Make similar faster and safer #18107

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
ac0367b
Make similar faster and safer
timholy Aug 18, 2016
c338485
Test that similar throws a MethodError for unsupported dims types
timholy Aug 18, 2016
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
15 changes: 8 additions & 7 deletions base/abstractarray.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ typealias RangeIndex Union{Int, Range{Int}, AbstractUnitRange{Int}, Colon}
typealias DimOrInd Union{Integer, AbstractUnitRange}
typealias IntOrInd Union{Int, AbstractUnitRange}
typealias DimsOrInds{N} NTuple{N,DimOrInd}
typealias NeedsShaping Union{Tuple{Integer,Vararg{Integer}}, Tuple{OneTo,Vararg{OneTo}}}

macro _inline_pure_meta()
Expr(:meta, :inline, :pure)
Expand Down Expand Up @@ -420,14 +421,14 @@ different element type it will create a regular `Array` instead:
2.18425e-314 2.18425e-314 2.18425e-314 2.18425e-314

"""
similar{T}(a::AbstractArray{T}) = similar(a, T)
similar( a::AbstractArray, T::Type) = similar(a, T, to_shape(indices(a)))
similar{T}(a::AbstractArray{T}, dims::Tuple) = similar(a, T, to_shape(dims))
similar{T}(a::AbstractArray{T}, dims::DimOrInd...) = similar(a, T, to_shape(dims))
similar( a::AbstractArray, T::Type, dims::DimOrInd...) = similar(a, T, to_shape(dims))
similar( a::AbstractArray, T::Type, dims) = similar(a, T, to_shape(dims))
similar{T}(a::AbstractArray{T}) = similar(a, T)
similar{T}(a::AbstractArray, ::Type{T}) = similar(a, T, to_shape(indices(a)))
similar{T}(a::AbstractArray{T}, dims::Tuple) = similar(a, T, to_shape(dims))
similar{T}(a::AbstractArray{T}, dims::DimOrInd...) = similar(a, T, to_shape(dims))
similar{T}(a::AbstractArray, ::Type{T}, dims::DimOrInd...) = similar(a, T, to_shape(dims))
similar{T}(a::AbstractArray, ::Type{T}, dims::NeedsShaping) = similar(a, T, to_shape(dims))
Copy link
Contributor

Choose a reason for hiding this comment

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

is this now a MethodError or handled elsewhere for dims not in this new Union?

Copy link
Member Author

@timholy timholy Aug 18, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Anything that's not ::Dims or ::NeedsShaping definitely has to be handled elsewhere. That's been the design all along (see CustomUnitRanges.jl), but it turns out that if you mess up and fail to "catch" the similar method then it resulted in a StackOverflowError. This can't do that anymore.

Copy link
Contributor

Choose a reason for hiding this comment

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

should this add a test_throws MethodError for such a case?

# similar creates an Array by default
similar{N}(a::AbstractArray, T::Type, dims::Dims{N}) = Array{T,N}(dims)
similar{T,N}(a::AbstractArray, ::Type{T}, dims::Dims{N}) = Array{T,N}(dims)

to_shape(::Tuple{}) = ()
to_shape(dims::Dims) = dims
Expand Down
17 changes: 17 additions & 0 deletions test/offsetarray.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,3 +369,20 @@ for s = -5:5
end

end # let

# Check that similar throws a MethodError rather than a
# StackOverflowError if no appropriate method has been defined
# (#18107)
module SimilarUR
using Base.Test
immutable MyURange <: AbstractUnitRange{Int}
start::Int
stop::Int
end
ur = MyURange(1,3)
a = Array{Int}(2)
@test_throws MethodError similar(a, ur)
@test_throws MethodError similar(a, Float64, ur)
@test_throws MethodError similar(a, Float64, (ur,))
@test_throws MethodError similar(a, (2.0,3.0))
end