Merge pull request #336 from JuliaArrays/cjf/fix-copying-constructor

Ensure static array constructors create copies

Author: c42f

src/MArray.jl

@@ -71,7 +71,7 @@ end
     end
 end
 
-@inline MArray(a::StaticArray) = MArray{size_tuple(typeof(a))}(Tuple(a))
+@inline MArray(a::StaticArray) = MArray{size_tuple(Size(a))}(Tuple(a))
 
 # Simplified show for the type
 show(io::IO, ::Type{MArray{S, T, N}}) where {S, T, N} = print(io, "MArray{$S,$T,$N}")

src/SArray.jl

@@ -50,7 +50,7 @@ end
     end
 end
 
-@inline SArray(a::StaticArray) = SArray{size_tuple(a)}(Tuple(a)) # TODO fixme
+@inline SArray(a::StaticArray) = SArray{size_tuple(Size(a))}(Tuple(a))
 
 # Simplified show for the type
 show(io::IO, ::Type{SArray{S, T, N}}) where {S, T, N} = print(io, "SArray{$S,$T,$N}")

src/convert.jl

@@ -1,7 +1,8 @@
 (::Type{SA})(x::Tuple{Tuple{Tuple{<:Tuple}}}) where {SA <: StaticArray} = error("No precise constructor for $SA found. Length of input was $(length(x[1][1][1])).")
 
 @inline (::Type{SA})(x...) where {SA <: StaticArray} = SA(x)
-@inline (::Type{SA})(a::AbstractArray) where {SA <: StaticArray} = convert(SA, a) # Is this a good idea?
+@inline (::Type{SA})(a::StaticArray) where {SA<:StaticArray} = SA(Tuple(a))
+@inline (::Type{SA})(a::AbstractArray) where {SA <: StaticArray} = convert(SA, a)
 
 # this covers most conversions and "statically-sized reshapes"
 @inline convert(::Type{SA}, sa::StaticArray) where {SA<:StaticArray} = SA(Tuple(sa))

src/traits.jl

@@ -86,6 +86,8 @@ Length(::Type{SA}) where {SA <: StaticArray} = Length(Size(SA))
 
 @pure @inline Base.sub2ind(::Size{S}, x::Int...) where {S} = sub2ind(S, x...)
 
+@pure size_tuple(::Size{S}) where {S} = Tuple{S...}
+
 # Some @pure convenience functions for `Length`
 @pure get(::Length{L}) where {L} = L
 

test/MArray.jl

@@ -27,6 +27,11 @@
         @test MArray{Tuple{2,2},Int}((1,2,3,4)).data === (1,2,3,4)
         @test MArray{Tuple{2,2}}((1,2,3,4)).data === (1,2,3,4)
 
+        @test MArray(SVector(1,2)) isa MArray{Tuple{2}}
+        # Constructors should create a copy (#335)
+        v = MArray{Tuple{2}}(1,2)
+        @test MArray(v) !== v && MArray(v) == v
+
         @test ((@MArray [1])::MArray{Tuple{1}}).data === (1,)
         @test ((@MArray [1,2])::MArray{Tuple{2}}).data === (1,2)
         @test ((@MArray Float64[1,2,3])::MArray{Tuple{3}}).data === (1.0, 2.0, 3.0)

test/MVector.jl

@@ -16,6 +16,10 @@
         @test MVector((1,)).data === (1,)
         @test MVector((1.0,)).data === (1.0,)
 
+        # Constructors should create a copy (#335)
+        v = MVector(1,2)
+        @test MVector(v) !== v && MVector(v) == v
+
         @test ((@MVector [1.0])::MVector{1}).data === (1.0,)
         @test ((@MVector [1, 2, 3])::MVector{3}).data === (1, 2, 3)
         @test ((@MVector Float64[1,2,3])::MVector{3}).data === (1.0, 2.0, 3.0)

test/SArray.jl

@@ -25,6 +25,8 @@
         @test SArray{Tuple{2,2},Int}((1,2,3,4)).data === (1,2,3,4)
         @test SArray{Tuple{2,2}}((1,2,3,4)).data === (1,2,3,4)
 
+        @test SArray(SArray{Tuple{2}}(1,2)) === SArray{Tuple{2}}(1,2)
+
         @test ((@SArray [1])::SArray{Tuple{1}}).data === (1,)
         @test ((@SArray [1,2])::SArray{Tuple{2}}).data === (1,2)
         @test ((@SArray Float64[1,2,3])::SArray{Tuple{3}}).data === (1.0, 2.0, 3.0)

test/core.jl

@@ -105,6 +105,10 @@
         @test @inferred(convert(SArray{Tuple{1},Float64}, ma_int)) === sa_float
         @test @inferred(convert(SArray{Tuple{1},Float64,1}, ma_int)) === sa_float
         @test @inferred(convert(SArray{Tuple{1},Float64,1,1}, ma_int)) === sa_float
+
+        # Self-conversion returns the matrix itself
+        @test convert(MArray, ma_int) === ma_int
+        @test convert(MArray, ma_float) === ma_float
     end
 
     @testset "AbstractArray conversion" begin