Skip to content

Commit f69b057

Browse files
mbaumanKristofferC
authored andcommitted
error instead of widening index types in sparse (#33083)
Followup to https://github.com/JuliaLang/julia/pull/31724/files#r317686891; instead of widening the index type dynamically based upon the index vector length, just throw an error in the case where the index type cannot hold all the indices in a CSC format. This previously was an OOB access (and likely segfault) in 1.2, so throwing an error here is not a breaking change -- and throwing an error restores type stability, addressing the performance regression flagged in #32985. (cherry picked from commit 9725fb4)
1 parent 9f39733 commit f69b057

File tree

2 files changed

+7
-8
lines changed

2 files changed

+7
-8
lines changed

stdlib/SparseArrays/src/sparsematrix.jl

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -630,9 +630,8 @@ function sparse(I::AbstractVector{Ti}, J::AbstractVector{Ti}, V::AbstractVector{
630630
"length(I) (=$(length(I))) == length(J) (= $(length(J))) == length(V) (= ",
631631
"$(length(V)))")))
632632
end
633-
Tj = Ti
634-
while isbitstype(Tj) && coolen >= typemax(Tj)
635-
Tj = widen(Tj)
633+
if Base.hastypemax(Ti) && coolen >= typemax(Ti)
634+
throw(ArgumentError("the index type $Ti cannot hold $coolen elements; use a larger index type"))
636635
end
637636
if m == 0 || n == 0 || coolen == 0
638637
if coolen != 0
@@ -645,13 +644,13 @@ function sparse(I::AbstractVector{Ti}, J::AbstractVector{Ti}, V::AbstractVector{
645644
SparseMatrixCSC(m, n, fill(one(Ti), n+1), Vector{Ti}(), Vector{Tv}())
646645
else
647646
# Allocate storage for CSR form
648-
csrrowptr = Vector{Tj}(undef, m+1)
647+
csrrowptr = Vector{Ti}(undef, m+1)
649648
csrcolval = Vector{Ti}(undef, coolen)
650649
csrnzval = Vector{Tv}(undef, coolen)
651650

652651
# Allocate storage for the CSC form's column pointers and a necessary workspace
653652
csccolptr = Vector{Ti}(undef, n+1)
654-
klasttouch = Vector{Tj}(undef, n)
653+
klasttouch = Vector{Ti}(undef, n)
655654

656655
# Allocate empty arrays for the CSC form's row and nonzero value arrays
657656
# The parent method called below automagically resizes these arrays

stdlib/SparseArrays/test/sparse.jl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2662,14 +2662,14 @@ end
26622662
J1 = Int8.(rand(1:10, 500))
26632663
V1 = ones(500)
26642664
# m * n < typemax(Ti) and length(I) >= typemax(Ti) - combining values
2665-
@test sparse(I1, J1, V1, 10, 10) !== nothing
2665+
@test_throws ArgumentError sparse(I1, J1, V1, 10, 10)
26662666
# m * n >= typemax(Ti) and length(I) >= typemax(Ti)
2667-
@test sparse(I1, J1, V1, 12, 13) !== nothing
2667+
@test_throws ArgumentError sparse(I1, J1, V1, 12, 13)
26682668
I1 = Int8.(rand(1:10, 126))
26692669
J1 = Int8.(rand(1:10, 126))
26702670
V1 = ones(126)
26712671
# m * n >= typemax(Ti) and length(I) < typemax(Ti)
2672-
@test sparse(I1, J1, V1, 100, 100) !== nothing
2672+
@test size(sparse(I1, J1, V1, 100, 100)) == (100,100)
26732673
end
26742674

26752675
@testset "Typecheck too strict #31435" begin

0 commit comments

Comments
 (0)