Revert "add unsetindex support to more copyto methods (#51760)" (#54332)

KristofferC · web-flow · commit 931f6de4ca73 · 2024-05-13T20:27:44.000+02:00
This reverts commit f0a28e9. This introduced in general a try catch inside the inner loop for `copyto!` and it also has performance regression in other cases #53430. Since this was added without any tests and "is not-quite-public API" it seems easiest to just revert it. This was added for Memory-to-Array and vice versa but dedicated methods could be added for that if it is desirable Fixes #53430, #52070
diff --git a/base/abstractarray.jl b/base/abstractarray.jl
@@ -1083,45 +1083,28 @@ function copyto_unaliased!(deststyle::IndexStyle, dest::AbstractArray, srcstyle:
         if srcstyle isa IndexLinear
             # Single-index implementation
             @inbounds for i in srcinds
-                if isassigned(src, i)
-                    dest[i + Δi] = src[i]
-                else
-                    _unsetindex!(dest, i + Δi)
-                end
+                dest[i + Δi] = src[i]
             end
         else
             # Dual-index implementation
             i = idf - 1
-            @inbounds for a in eachindex(src)
-                i += 1
-                if isassigned(src, a)
-                    dest[i] = src[a]
-                else
-                    _unsetindex!(dest, i)
-                end
+            @inbounds for a in src
+                dest[i+=1] = a
             end
         end
     else
         iterdest, itersrc = eachindex(dest), eachindex(src)
         if iterdest == itersrc
             # Shared-iterator implementation
-            @inbounds for I in iterdest
-                if isassigned(src, I)
-                    dest[I] = src[I]
-                else
-                    _unsetindex!(dest, I)
-                end
+            for I in iterdest
+                @inbounds dest[I] = src[I]
             end
         else
             # Dual-iterator implementation
             ret = iterate(iterdest)
-            @inbounds for a in itersrc
+            @inbounds for a in src
                 idx, state = ret::NTuple{2,Any}
-                if isassigned(src, a)
-                    dest[idx] = src[a]
-                else
-                    _unsetindex!(dest, idx)
-                end
+                dest[idx] = a
                 ret = iterate(iterdest, state)
             end
         end
@@ -1150,11 +1133,7 @@ function copyto!(dest::AbstractArray, dstart::Integer,
     (checkbounds(Bool, srcinds, sstart)  && checkbounds(Bool, srcinds, sstart+n-1))  || throw(BoundsError(src,  sstart:sstart+n-1))
     src′ = unalias(dest, src)
     @inbounds for i = 0:n-1
-        if isassigned(src′, sstart+i)
-            dest[dstart+i] = src′[sstart+i]
-        else
-            _unsetindex!(dest, dstart+i)
-        end
+        dest[dstart+i] = src′[sstart+i]
     end
     return dest
 end
@@ -1165,7 +1144,7 @@ function copy(a::AbstractArray)
 end
 
 function copyto!(B::AbstractVecOrMat{R}, ir_dest::AbstractRange{Int}, jr_dest::AbstractRange{Int},
-                 A::AbstractVecOrMat{S}, ir_src::AbstractRange{Int}, jr_src::AbstractRange{Int}) where {R,S}
+               A::AbstractVecOrMat{S}, ir_src::AbstractRange{Int}, jr_src::AbstractRange{Int}) where {R,S}
     if length(ir_dest) != length(ir_src)
         throw(ArgumentError(LazyString("source and destination must have same size (got ",
             length(ir_src)," and ",length(ir_dest),")")))
@@ -1495,20 +1474,7 @@ function _setindex!(::IndexCartesian, A::AbstractArray, v, I::Vararg{Int,M}) whe
     r
 end
 
-function _unsetindex!(A::AbstractArray, i::Integer...)
-    @_propagate_inbounds_meta
-    _unsetindex!(A, map(to_index, i)...)
-end
-
-function _unsetindex!(A::AbstractArray{T}, i::Int...) where T
-    # this provides a fallback method which is a no-op if the element is already unassigned
-    # such that copying into an uninitialized object generally always will work,
-    # even if the specific custom array type has not implemented `_unsetindex!`
-    @inline
-    @boundscheck checkbounds(A, i...)
-    allocatedinline(T) || @inbounds(!isassigned(A, i...)) || throw(MethodError(_unsetindex!, (A, i...)))
-    return A
-end
+_unsetindex!(A::AbstractArray, i::Integer) = _unsetindex!(A, to_index(i))
 
 """
     parent(A)
diff --git a/base/array.jl b/base/array.jl
@@ -219,12 +219,7 @@ function _unsetindex!(A::Array, i::Int)
     @inbounds _unsetindex!(GenericMemoryRef(A.ref, i))
     return A
 end
-function _unsetindex!(A::Array, i::Int...)
-    @inline
-    @boundscheck checkbounds(A, i...)
-    @inbounds _unsetindex!(A, _to_linear_index(A, i...))
-    return A
-end
+
 
 # TODO: deprecate this (aligned_sizeof and/or elsize and/or sizeof(Some{T}) are more correct)
 elsize(::Type{A}) where {T,A<:Array{T}} = aligned_sizeof(T)
diff --git a/base/multidimensional.jl b/base/multidimensional.jl
@@ -1612,12 +1612,6 @@ end
     end
 end
 
-# _unsetindex
-@propagate_inbounds function Base._unsetindex!(A::AbstractArray, i::CartesianIndex)
-    Base._unsetindex!(A, to_indices(A, (i,))...)
-    return A
-end
-
 ## permutedims
 
 ## Permute array dims ##
diff --git a/base/subarray.jl b/base/subarray.jl
@@ -409,25 +409,6 @@ function isassigned(V::FastSubArray{<:Any, 1}, i::Int)
     r
 end
 
-function _unsetindex!(V::FastSubArray, i::Int)
-    @inline
-    @boundscheck checkbounds(V, i)
-    @inbounds _unsetindex!(V.parent, _reindexlinear(V, i))
-    return V
-end
-function _unsetindex!(V::FastSubArray{<:Any,1}, i::Int)
-    @inline
-    @boundscheck checkbounds(V, i)
-    @inbounds _unsetindex!(V.parent, _reindexlinear(V, i))
-    return V
-end
-function _unsetindex!(V::SubArray{T,N}, i::Vararg{Int,N}) where {T,N}
-    @inline
-    @boundscheck checkbounds(V, i...)
-    @inbounds _unsetindex!(V.parent, reindex(V.indices, i)...)
-    return V
-end
-
 IndexStyle(::Type{<:FastSubArray}) = IndexLinear()
 
 # Strides are the distance in memory between adjacent elements in a given dimension
diff --git a/stdlib/LinearAlgebra/src/bidiag.jl b/stdlib/LinearAlgebra/src/bidiag.jl
@@ -195,14 +195,19 @@ end
 
 #Converting from Bidiagonal to dense Matrix
 function Matrix{T}(A::Bidiagonal) where T
-    B = Matrix{T}(undef, size(A))
-    if haszero(T) # optimized path for types with zero(T) defined
-        size(B,1) > 1 && fill!(B, zero(T))
-        copyto!(view(B, diagind(B)), A.dv)
-        copyto!(view(B, diagind(B, A.uplo == 'U' ? 1 : -1)), A.ev)
-    else
-        copyto!(B, A)
+    n = size(A, 1)
+    B = Matrix{T}(undef, n, n)
+    n == 0 && return B
+    n > 1 && fill!(B, zero(T))
+    @inbounds for i = 1:n - 1
+        B[i,i] = A.dv[i]
+        if A.uplo == 'U'
+            B[i,i+1] = A.ev[i]
+        else
+            B[i+1,i] = A.ev[i]
+        end
     end
+    B[n,n] = A.dv[n]
     return B
 end
 Matrix(A::Bidiagonal{T}) where {T} = Matrix{promote_type(T, typeof(zero(T)))}(A)
diff --git a/stdlib/LinearAlgebra/src/diagonal.jl b/stdlib/LinearAlgebra/src/diagonal.jl
@@ -116,12 +116,11 @@ AbstractMatrix{T}(D::Diagonal{T}) where {T} = copy(D)
 Matrix(D::Diagonal{T}) where {T} = Matrix{promote_type(T, typeof(zero(T)))}(D)
 Array(D::Diagonal{T}) where {T} = Matrix(D)
 function Matrix{T}(D::Diagonal) where {T}
-    B = Matrix{T}(undef, size(D))
-    if haszero(T) # optimized path for types with zero(T) defined
-        size(B,1) > 1 && fill!(B, zero(T))
-        copyto!(view(B, diagind(B)), D.diag)
-    else
-        copyto!(B, D)
+    n = size(D, 1)
+    B = Matrix{T}(undef, n, n)
+    n > 1 && fill!(B, zero(T))
+    @inbounds for i in 1:n
+        B[i,i] = D.diag[i]
     end
     return B
 end
diff --git a/stdlib/LinearAlgebra/src/structuredbroadcast.jl b/stdlib/LinearAlgebra/src/structuredbroadcast.jl
@@ -8,8 +8,8 @@ struct StructuredMatrixStyle{T} <: Broadcast.AbstractArrayStyle{2} end
 StructuredMatrixStyle{T}(::Val{2}) where {T} = StructuredMatrixStyle{T}()
 StructuredMatrixStyle{T}(::Val{N}) where {T,N} = Broadcast.DefaultArrayStyle{N}()
 
-const StructuredMatrix{T} = Union{Diagonal{T},Bidiagonal{T},SymTridiagonal{T},Tridiagonal{T},LowerTriangular{T},UnitLowerTriangular{T},UpperTriangular{T},UnitUpperTriangular{T}}
-for ST in (Diagonal,Bidiagonal,SymTridiagonal,Tridiagonal,LowerTriangular,UnitLowerTriangular,UpperTriangular,UnitUpperTriangular)
+const StructuredMatrix = Union{Diagonal,Bidiagonal,SymTridiagonal,Tridiagonal,LowerTriangular,UnitLowerTriangular,UpperTriangular,UnitUpperTriangular}
+for ST in Base.uniontypes(StructuredMatrix)
     @eval Broadcast.BroadcastStyle(::Type{<:$ST}) = $(StructuredMatrixStyle{ST}())
 end
 
@@ -133,7 +133,6 @@ fails as `zero(::Tuple{Int})` is not defined. However,
 iszerodefined(::Type) = false
 iszerodefined(::Type{<:Number}) = true
 iszerodefined(::Type{<:AbstractArray{T}}) where T = iszerodefined(T)
-iszerodefined(::Type{<:UniformScaling{T}}) where T = iszerodefined(T)
 
 count_structedmatrix(T, bc::Broadcasted) = sum(Base.Fix2(isa, T), Broadcast.cat_nested(bc); init = 0)
 
@@ -147,7 +146,6 @@ fzero(::Type{T}) where T = T
 fzero(r::Ref) = r[]
 fzero(t::Tuple{Any}) = t[1]
 fzero(S::StructuredMatrix) = zero(eltype(S))
-fzero(::StructuredMatrix{<:AbstractMatrix{T}}) where {T<:Number} = haszero(T) ? zero(T)*I : missing
 fzero(x) = missing
 function fzero(bc::Broadcast.Broadcasted)
     args = map(fzero, bc.args)
diff --git a/stdlib/LinearAlgebra/src/tridiag.jl b/stdlib/LinearAlgebra/src/tridiag.jl
@@ -135,17 +135,13 @@ function Matrix{T}(M::SymTridiagonal) where T
     n = size(M, 1)
     Mf = Matrix{T}(undef, n, n)
     n == 0 && return Mf
-    if haszero(T) # optimized path for types with zero(T) defined
-        n > 2 && fill!(Mf, zero(T))
-        @inbounds for i = 1:n-1
-            Mf[i,i] = symmetric(M.dv[i], :U)
-            Mf[i+1,i] = transpose(M.ev[i])
-            Mf[i,i+1] = M.ev[i]
-        end
-        Mf[n,n] = symmetric(M.dv[n], :U)
-    else
-        copyto!(Mf, M)
+    n > 2 && fill!(Mf, zero(T))
+    @inbounds for i = 1:n-1
+        Mf[i,i] = symmetric(M.dv[i], :U)
+        Mf[i+1,i] = transpose(M.ev[i])
+        Mf[i,i+1] = M.ev[i]
     end
+    Mf[n,n] = symmetric(M.dv[n], :U)
     return Mf
 end
 Matrix(M::SymTridiagonal{T}) where {T} = Matrix{promote_type(T, typeof(zero(T)))}(M)
@@ -590,14 +586,15 @@ axes(M::Tridiagonal) = (ax = axes(M.d,1); (ax, ax))
 
 function Matrix{T}(M::Tridiagonal) where {T}
     A = Matrix{T}(undef, size(M))
-    if haszero(T) # optimized path for types with zero(T) defined
-        size(A,1) > 2 && fill!(A, zero(T))
-        copyto!(view(A, diagind(A)), M.d)
-        copyto!(view(A, diagind(A,1)), M.du)
-        copyto!(view(A, diagind(A,-1)), M.dl)
-    else
-        copyto!(A, M)
-    end
+    n = length(M.d)
+    n == 0 && return A
+    n > 2 && fill!(A, zero(T))
+    for i in 1:n-1
+        A[i,i] = M.d[i]
+        A[i+1,i] = M.dl[i]
+        A[i,i+1] = M.du[i]
+    end
+    A[n,n] = M.d[n]
     A
 end
 Matrix(M::Tridiagonal{T}) where {T} = Matrix{promote_type(T, typeof(zero(T)))}(M)
diff --git a/stdlib/LinearAlgebra/test/bidiag.jl b/stdlib/LinearAlgebra/test/bidiag.jl
@@ -904,17 +904,4 @@ end
     @test mul!(C1, B, sv, 1, 2) == mul!(C2, B, v, 1 ,2)
 end
 
-@testset "Matrix conversion for non-numeric and undef" begin
-    B = Bidiagonal(Vector{BigInt}(undef, 4), fill(big(3), 3), :U)
-    M = Matrix(B)
-    B[diagind(B)] .= 4
-    M[diagind(M)] .= 4
-    @test diag(B) == diag(M)
-
-    B = Bidiagonal(fill(Diagonal([1,3]), 3), fill(Diagonal([1,3]), 2), :U)
-    M = Matrix{eltype(B)}(B)
-    @test M isa Matrix{eltype(B)}
-    @test M == B
-end
-
 end # module TestBidiagonal
diff --git a/stdlib/LinearAlgebra/test/diagonal.jl b/stdlib/LinearAlgebra/test/diagonal.jl
@@ -1298,17 +1298,4 @@ end
     @test yadj == x'
 end
 
-@testset "Matrix conversion for non-numeric and undef" begin
-    D = Diagonal(Vector{BigInt}(undef, 4))
-    M = Matrix(D)
-    D[diagind(D)] .= 4
-    M[diagind(M)] .= 4
-    @test diag(D) == diag(M)
-
-    D = Diagonal(fill(Diagonal([1,3]), 2))
-    M = Matrix{eltype(D)}(D)
-    @test M isa Matrix{eltype(D)}
-    @test M == D
-end
-
 end # module TestDiagonal
diff --git a/stdlib/LinearAlgebra/test/special.jl b/stdlib/LinearAlgebra/test/special.jl
@@ -111,11 +111,8 @@ Random.seed!(1)
         struct TypeWithZero end
         Base.promote_rule(::Type{TypeWithoutZero}, ::Type{TypeWithZero}) = TypeWithZero
         Base.convert(::Type{TypeWithZero}, ::TypeWithoutZero) = TypeWithZero()
-        Base.zero(x::Union{TypeWithoutZero, TypeWithZero}) = zero(typeof(x))
         Base.zero(::Type{<:Union{TypeWithoutZero, TypeWithZero}}) = TypeWithZero()
         LinearAlgebra.symmetric(::TypeWithoutZero, ::Symbol) = TypeWithoutZero()
-        LinearAlgebra.symmetric_type(::Type{TypeWithoutZero}) = TypeWithoutZero
-        Base.copy(A::TypeWithoutZero) = A
         Base.transpose(::TypeWithoutZero) = TypeWithoutZero()
         d  = fill(TypeWithoutZero(), 3)
         du = fill(TypeWithoutZero(), 2)
diff --git a/stdlib/LinearAlgebra/test/structuredbroadcast.jl b/stdlib/LinearAlgebra/test/structuredbroadcast.jl
@@ -298,62 +298,4 @@ end
 # structured broadcast with function returning non-number type
 @test tuple.(Diagonal([1, 2])) == [(1,) (0,); (0,) (2,)]
 
-@testset "broadcast over structured matrices with matrix elements" begin
-    function standardbroadcastingtests(D, T)
-        M = [x for x in D]
-        Dsum = D .+ D
-        @test Dsum isa T
-        @test Dsum == M .+ M
-        Dcopy = copy.(D)
-        @test Dcopy isa T
-        @test Dcopy == D
-        Df = float.(D)
-        @test Df isa T
-        @test Df == D
-        @test eltype(eltype(Df)) <: AbstractFloat
-        @test (x -> (x,)).(D) == (x -> (x,)).(M)
-        @test (x -> 1).(D) == ones(Int,size(D))
-        @test all(==(2), ndims.(D))
-        @test_throws MethodError size.(D)
-    end
-    @testset "Diagonal" begin
-        @testset "square" begin
-            A = [1 3; 2 4]
-            D = Diagonal([A, A])
-            standardbroadcastingtests(D, Diagonal)
-            @test sincos.(D) == sincos.(Matrix{eltype(D)}(D))
-            M = [x for x in D]
-            @test cos.(D) == cos.(M)
-        end
-
-        @testset "different-sized square blocks" begin
-            D = Diagonal([ones(3,3), fill(3.0,2,2)])
-            standardbroadcastingtests(D, Diagonal)
-        end
-
-        @testset "rectangular blocks" begin
-            D = Diagonal([ones(Bool,3,4), ones(Bool,2,3)])
-            standardbroadcastingtests(D, Diagonal)
-        end
-
-        @testset "incompatible sizes" begin
-            A = reshape(1:12, 4, 3)
-            B = reshape(1:12, 3, 4)
-            D1 = Diagonal(fill(A, 2))
-            D2 = Diagonal(fill(B, 2))
-            @test_throws DimensionMismatch D1 .+ D2
-        end
-    end
-    @testset "Bidiagonal" begin
-        A = [1 3; 2 4]
-        B = Bidiagonal(fill(A,3), fill(A,2), :U)
-        standardbroadcastingtests(B, Bidiagonal)
-    end
-    @testset "UpperTriangular" begin
-        A = [1 3; 2 4]
-        U = UpperTriangular([(i+j)*A for i in 1:3, j in 1:3])
-        standardbroadcastingtests(U, UpperTriangular)
-    end
-end
-
 end
diff --git a/stdlib/LinearAlgebra/test/tridiag.jl b/stdlib/LinearAlgebra/test/tridiag.jl
@@ -859,12 +859,4 @@ end
     @test axes(B) === (ax, ax)
 end
 
-@testset "Matrix conversion for non-numeric and undef" begin
-    T = Tridiagonal(fill(big(3), 3), Vector{BigInt}(undef, 4), fill(big(3), 3))
-    M = Matrix(T)
-    T[diagind(T)] .= 4
-    M[diagind(M)] .= 4
-    @test diag(T) == diag(M)
-end
-
 end # module TestTridiagonal
diff --git a/test/abstractarray.jl b/test/abstractarray.jl
diff --git a/test/subarray.jl b/test/subarray.jl
