provide non-aliasing constructors for sparse arrays (#34647)

* provide non-aliasing convert methods for sparse arrays * provide non-aliasing constructors for sparse arrays * make all sparse constructors copy * remove unnecessary type assetion
JuliaLang · Apr 11, 2020 · c9965ac · c9965ac
1 parent e7219c5
commit c9965ac
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 12 deletions.
diff --git a/stdlib/SparseArrays/src/sparsematrix.jl b/stdlib/SparseArrays/src/sparsematrix.jl
@@ -397,11 +397,12 @@ SparseMatrixCSC{Tv}(S::AbstractSparseMatrixCSC{Tv}) where {Tv} = copy(S)
 SparseMatrixCSC{Tv}(S::AbstractSparseMatrixCSC) where {Tv} = SparseMatrixCSC{Tv,eltype(getcolptr(S))}(S)
 SparseMatrixCSC{Tv,Ti}(S::AbstractSparseMatrixCSC{Tv,Ti}) where {Tv,Ti} = copy(S)
 function SparseMatrixCSC{Tv,Ti}(S::AbstractSparseMatrixCSC) where {Tv,Ti}
- eltypeTicolptr = convert(Vector{Ti}, getcolptr(S))
- eltypeTirowval = convert(Vector{Ti}, rowvals(S))
- eltypeTvnzval = convert(Vector{Tv}, nonzeros(S))
+ eltypeTicolptr = Vector{Ti}(getcolptr(S))
+ eltypeTirowval = Vector{Ti}(rowvals(S))
+ eltypeTvnzval = Vector{Tv}(nonzeros(S))
  return SparseMatrixCSC(size(S, 1), size(S, 2), eltypeTicolptr, eltypeTirowval, eltypeTvnzval)
 end
+
 # converting from other matrix types to SparseMatrixCSC (also see sparse())
 SparseMatrixCSC(M::Matrix) = sparse(M)
 function SparseMatrixCSC(T::Tridiagonal{Tv}) where Tv

diff --git a/stdlib/SparseArrays/src/sparsevector.jl b/stdlib/SparseArrays/src/sparsevector.jl
@@ -421,18 +421,17 @@ SparseVector{Tv}(s::AbstractVector{Tv}) where {Tv} = SparseVector{Tv,Int}(s)
 
 SparseVector(s::AbstractVector{Tv}) where {Tv} = SparseVector{Tv,Int}(s)
 
+# copy-constructors
+SparseVector(s::SparseVector{Tv,Ti}) where {Tv,Ti} = SparseVector{Tv,Ti}(s)
+SparseVector{Tv}(s::SparseVector{<:Any,Ti}) where {Tv,Ti} = SparseVector{Tv,Ti}(s)
+function SparseVector{Tv,Ti}(s::SparseVector) where {Tv,Ti}
+ copyind = Vector{Ti}(nonzeroinds(s))
+ copynz = Vector{Tv}(nonzeros(s))
+ SparseVector{Tv,Ti}(length(s), copyind, copynz)
+end
 
 # convert between different types of SparseVector
-SparseVector{Tv}(s::SparseVector{Tv}) where {Tv} = s
-SparseVector{Tv,Ti}(s::SparseVector{Tv,Ti}) where {Tv,Ti} = s
-SparseVector{Tv,Ti}(s::SparseVector) where {Tv,Ti} =
- SparseVector{Tv,Ti}(length(s::SparseVector), convert(Vector{Ti}, nonzeroinds(s)), convert(Vector{Tv}, nonzeros(s)))
-
-SparseVector{Tv}(s::SparseVector{<:Any,Ti}) where {Tv,Ti} =
- SparseVector{Tv,Ti}(length(s::SparseVector), nonzeroinds(s), convert(Vector{Tv}, nonzeros(s)))
-
 convert(T::Type{<:SparseVector}, m::AbstractVector) = m isa T ? m : T(m)
-
 convert(T::Type{<:SparseVector}, m::AbstractSparseMatrixCSC) = T(m)
 convert(T::Type{<:AbstractSparseMatrixCSC}, v::SparseVector) = T(v)
 

diff --git a/stdlib/SparseArrays/test/sparse.jl b/stdlib/SparseArrays/test/sparse.jl
@@ -2834,4 +2834,17 @@ end
  @test reshape(crA, 20, 10) == A
 end
 
+@testset "avoid aliasing of fields during constructing $T (issue #34630)" for T in
+ (SparseMatrixCSC, SparseMatrixCSC{Float64}, SparseMatrixCSC{Float64,Int16})
+
+ A = sparse([1 1; 1 0])
+ B = T(A)
+ @test A == B
+ A[2,2] = 1
+ @test A != B
+ @test getcolptr(A) !== getcolptr(B)
+ @test rowvals(A) !== rowvals(B)
+ @test nonzeros(A) !== nonzeros(B)
+end
+
 end # module
diff --git a/stdlib/SparseArrays/test/sparsevector.jl b/stdlib/SparseArrays/test/sparsevector.jl
@@ -1408,4 +1408,16 @@ end
  @test nnz(scv) == nnz(A[:, 1])
 end
 
+@testset "avoid aliasing of fields during constructing $T (issue #34630)" for T in
+ (SparseVector, SparseVector{Float64}, SparseVector{Float64,Int16})
+
+ A = sparse([1; 0])
+ B = T(A)
+ @test A == B
+ A[2] = 1
+ @test A != B
+ @test nonzeroinds(A) !== nonzeroinds(B)
+ @test nonzeros(A) !== nonzeros(B)
+end
+
 end # module