Skip to content

Commit

Permalink
Merge pull request #11797 from ScottPJones/spj/cover1
Browse files Browse the repository at this point in the history 
Improve error messages in abstractarray.jl
  • Loading branch information
@mbauman
mbauman committed Jul 14, 2015
2 parents 3961970 + f47c9b7 commit cecee20
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 51 deletions.
137 changes: 90 additions & 47 deletions base/abstractarray.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,12 @@ ndims{T<:AbstractArray}(::Type{T}) = ndims(super(T))
length(t::AbstractArray) = prod(size(t))::Int
endof(a::AbstractArray) = length(a)
first(a::AbstractArray) = a[1]
first(a) = (s = start(a); done(a, s) ? throw(ArgumentError("collection must be non-empty")) : next(a, s)[1])

function first(itr)
state = start(itr)
done(itr, state) && throw(ArgumentError("collection must be non-empty"))
next(itr, state)[1]
end
last(a) = a[end]

function stride(a::AbstractArray, i::Integer)
Expand Down Expand Up @@ -154,11 +159,13 @@ checkbounds(A::AbstractArray, I::AbstractVector{Bool}) = length(A) == length(I)
checkbounds(A::AbstractArray, I::Union{Real,AbstractArray,Colon}) = (@_inline_meta; _checkbounds(length(A), I) || throw_boundserror(A, I))
function checkbounds(A::AbstractMatrix, I::Union{Real,AbstractArray,Colon}, J::Union{Real,AbstractArray,Colon})
@_inline_meta
(_checkbounds(size(A,1), I) && _checkbounds(size(A,2), J)) || throw_boundserror(A, (I, J))
(_checkbounds(size(A,1), I) && _checkbounds(size(A,2), J)) ||
throw_boundserror(A, (I, J))
end
function checkbounds(A::AbstractArray, I::Union{Real,AbstractArray,Colon}, J::Union{Real,AbstractArray,Colon})
@_inline_meta
(_checkbounds(size(A,1), I) && _checkbounds(trailingsize(A,Val{2}), J)) || throw_boundserror(A, (I, J))
(_checkbounds(size(A,1), I) && _checkbounds(trailingsize(A,Val{2}), J)) ||
throw_boundserror(A, (I, J))
end
@generated function checkbounds(A::AbstractArray, I::Union{Real,AbstractArray,Colon}...)
meta = Expr(:meta, :inline)
Expand Down Expand Up @@ -197,7 +204,7 @@ similar( a::AbstractArray, T, dims::Dims) = Array(T, dims)

function reshape(a::AbstractArray, dims::Dims)
if prod(dims) != length(a)
throw(ArgumentError("dimensions must be consistent with array size"))
throw(ArgumentError("dimensions must be consistent with array size (expected $(length(a)), got $(prod(dims)))"))
end
copy!(similar(a, dims), a)
end
Expand All @@ -216,32 +223,40 @@ end

# if src is not an AbstractArray, moving to the offset might be O(n)
function copy!(dest::AbstractArray, doffs::Integer, src)
doffs < 1 && throw(BoundsError())
doffs < 1 && throw(BoundsError(dest, doffs))
st = start(src)
i, dmax = doffs, length(dest)
@inbounds while !done(src, st)
i > dmax && throw(BoundsError())
i > dmax && throw(BoundsError(dest, i))
val, st = next(src, st)
dest[i] = val
i += 1
end
return dest
end

# copy from an some iterable object into an AbstractArray
function copy!(dest::AbstractArray, doffs::Integer, src, soffs::Integer)
if (doffs < 1) | (soffs < 1)
throw(BoundsError())
doffs < 1 && throw(BoundsError(dest, doffs))
throw(ArgumentError(string("source start offset (",soffs,") is < 1")))
end
st = start(src)
for j = 1:(soffs-1)
done(src, st) && throw(BoundsError())
if done(src, st)
throw(ArgumentError(string("source has fewer elements than required, ",
"expected at least ",soffs,", got ",j-1)))
end
_, st = next(src, st)
end
dn = done(src, st)
dn && throw(BoundsError())
if dn
throw(ArgumentError(string("source has fewer elements than required, ",
"expected at least ",soffs,", got ",soffs-1)))
end
i, dmax = doffs, length(dest)
@inbounds while !dn
i > dmax && throw(BoundsError())
i > dmax && throw(BoundsError(dest, i))
val, st = next(src, st)
dest[i] = val
i += 1
Expand All @@ -252,15 +267,20 @@ end

# this method must be separate from the above since src might not have a length
function copy!(dest::AbstractArray, doffs::Integer, src, soffs::Integer, n::Integer)
n < 0 && throw(BoundsError())
n < 0 && throw(BoundsError(dest, n))
n == 0 && return dest
dmax = doffs + n - 1
if (dmax > length(dest)) | (doffs < 1) | (soffs < 1)
throw(BoundsError())
doffs < 1 && throw(BoundsError(dest, doffs))
soffs < 1 && throw(ArgumentError(string("source start offset (",soffs,") is < 1")))
throw(BoundsError(dest, dmax))
end
st = start(src)
for j = 1:(soffs-1)
done(src, st) && throw(BoundsError())
if done(src, st)
throw(ArgumentError(string("source has fewer elements than required, ",
"expected at least ",soffs,", got ",j-1)))
end
_, st = next(src, st)
end
i = doffs
Expand All @@ -269,7 +289,7 @@ function copy!(dest::AbstractArray, doffs::Integer, src, soffs::Integer, n::Inte
dest[i] = val
i += 1
end
i <= dmax && throw(BoundsError())
i <= dmax && throw(BoundsError(dest, i))
return dest
end

Expand All @@ -278,9 +298,7 @@ end

function copy!(dest::AbstractArray, src::AbstractArray)
n = length(src)
if n > length(dest)
throw(BoundsError())
end
n > length(dest) && throw(BoundsError(dest, n))
@inbounds for i = 1:n
dest[i] = src[i]
end
Expand All @@ -290,16 +308,21 @@ end
function copy!(dest::AbstractArray, doffs::Integer, src::AbstractArray)
copy!(dest, doffs, src, 1, length(src))
end

function copy!(dest::AbstractArray, doffs::Integer, src::AbstractArray, soffs::Integer)
soffs > length(src) && throw(BoundsError())
soffs > length(src) && throw(BoundsError(src, soffs))
copy!(dest, doffs, src, soffs, length(src)-soffs+1)
end
function copy!(dest::AbstractArray, doffs::Integer, src::AbstractArray, soffs::Integer, n::Integer)
n < 0 && throw(BoundsError())

function copy!(dest::AbstractArray, doffs::Integer,
src::AbstractArray, soffs::Integer,
n::Integer)
n == 0 && return dest
if soffs+n-1 > length(src) || doffs+n-1 > length(dest) || doffs < 1 || soffs < 1
throw(BoundsError())
end
n < 0 && throw(BoundsError(src, n))
soffs+n-1 > length(src) && throw(BoundsError(src, soffs+n-1))
doffs+n-1 > length(dest) && throw(BoundsError(dest, doffs+n-1))
doffs < 1 && throw(BoundsError(dest, doffs))
soffs < 1 && throw(BoundsError(src, soffs))
@inbounds for i = 0:(n-1)
dest[doffs+i] = src[soffs+i]
end
Expand All @@ -308,9 +331,15 @@ end

copy(a::AbstractArray) = copy!(similar(a), a)

function copy!{R,S}(B::AbstractVecOrMat{R}, ir_dest::Range{Int}, jr_dest::Range{Int}, A::AbstractVecOrMat{S}, ir_src::Range{Int}, jr_src::Range{Int})
if length(ir_dest) != length(ir_src) || length(jr_dest) != length(jr_src)
throw(ArgumentError("source and destination must have same size"))
function copy!{R,S}(B::AbstractVecOrMat{R}, ir_dest::Range{Int}, jr_dest::Range{Int},
A::AbstractVecOrMat{S}, ir_src::Range{Int}, jr_src::Range{Int})
if length(ir_dest) != length(ir_src)
throw(ArgumentError(string("source and destination must have same size (got ",
length(ir_src)," and ",length(ir_dest),")")))
end
if length(jr_dest) != length(jr_src)
throw(ArgumentError(string("source and destination must have same size (got ",
length(jr_src)," and ",length(jr_dest),")")))
end
checkbounds(B, ir_dest, jr_dest)
checkbounds(A, ir_src, jr_src)
Expand All @@ -326,9 +355,15 @@ function copy!{R,S}(B::AbstractVecOrMat{R}, ir_dest::Range{Int}, jr_dest::Range{
return B
end

function copy_transpose!{R,S}(B::AbstractVecOrMat{R}, ir_dest::Range{Int}, jr_dest::Range{Int}, A::AbstractVecOrMat{S}, ir_src::Range{Int}, jr_src::Range{Int})
if length(ir_dest) != length(jr_src) || length(jr_dest) != length(ir_src)
throw(ArgumentError("source and destination must have same size"))
function copy_transpose!{R,S}(B::AbstractVecOrMat{R}, ir_dest::Range{Int}, jr_dest::Range{Int},
A::AbstractVecOrMat{S}, ir_src::Range{Int}, jr_src::Range{Int})
if length(ir_dest) != length(jr_src)
throw(ArgumentError(string("source and destination must have same size (got ",
length(jr_src)," and ",length(ir_dest),")")))
end
if length(jr_dest) != length(ir_src)
throw(ArgumentError(string("source and destination must have same size (got ",
length(ir_src)," and ",length(jr_dest),")")))
end
checkbounds(B, ir_dest, jr_dest)
checkbounds(A, ir_src, jr_src)
Expand Down Expand Up @@ -690,7 +725,7 @@ function hcat{T}(A::AbstractVecOrMat{T}...)
for j = 1:nargs
Aj = A[j]
if size(Aj, 1) != nrows
throw(ArgumentError("number of rows must match"))
throw(ArgumentError("number of rows of each array must match (got $(map(x->size(x,1), A)))"))
end
dense &= isa(Aj,Array)
nd = ndims(Aj)
Expand Down Expand Up @@ -722,7 +757,7 @@ function vcat{T}(A::AbstractMatrix{T}...)
ncols = size(A[1], 2)
for j = 2:nargs
if size(A[j], 2) != ncols
throw(ArgumentError("number of columns must match"))
throw(ArgumentError("number of columns of each array must match (got $(map(x->size(x,2), A)))"))
end
end
B = similar(full(A[1]), nrows, ncols)
Expand Down Expand Up @@ -761,11 +796,13 @@ function cat_t(catdims, typeC::Type, X...)
for i = 2:nargs
for d = 1:ndimsC
currentdim = (d <= ndimsX[i] ? size(X[i],d) : 1)
if dims2cat[d]==0
dimsC[d] == currentdim || throw(DimensionMismatch("mismatch in dimension $(d)"))
else
if dims2cat[d] != 0
dimsC[d] += currentdim
catsizes[i,dims2cat[d]] = currentdim
elseif dimsC[d] != currentdim
throw(DimensionMismatch(string("mismatch in dimension ",d,
" (expected ",dimsC[d],
" got ",currentdim,")")))
end
end
end
Expand All @@ -777,7 +814,8 @@ function cat_t(catdims, typeC::Type, X...)

offsets = zeros(Int,length(catdims))
for i=1:nargs
cat_one = [ dims2cat[d]==0 ? (1:dimsC[d]) : (offsets[dims2cat[d]]+(1:catsizes[i,dims2cat[d]])) for d=1:ndimsC]
cat_one = [ dims2cat[d] == 0 ? (1:dimsC[d]) : (offsets[dims2cat[d]]+(1:catsizes[i,dims2cat[d]]))
for d=1:ndimsC ]
C[cat_one...] = X[i]
for k = 1:length(catdims)
offsets[k] += catsizes[i,k]
Expand Down Expand Up @@ -817,9 +855,8 @@ typed_hcat(T::Type, A::AbstractArray...) = cat_t(2, T, A...)
function hvcat(nbc::Integer, as...)
# nbc = # of block columns
n = length(as)
if mod(n,nbc) != 0
throw(ArgumentError("all rows must have the same number of block columns"))
end
mod(n,nbc) != 0 &&
throw(ArgumentError("number of arrays $n is not a multiple of the requested number of block columns $nbc"))
nbr = div(n,nbc)
hvcat(ntuple(i->nbc, nbr), as...)
end
Expand Down Expand Up @@ -850,16 +887,16 @@ function hvcat{T}(rows::Tuple{Vararg{Int}}, as::AbstractMatrix{T}...)
Aj = as[a+j-1]
szj = size(Aj,2)
if size(Aj,1) != szi
throw(ArgumentError("mismatched height in block row $(i)"))
throw(ArgumentError("mismatched height in block row $(i) (expected $szi, got $(size(Aj,1)))"))
end
if c-1+szj > nc
throw(ArgumentError("block row $(i) has mismatched number of columns"))
throw(ArgumentError("block row $(i) has mismatched number of columns (expected $nc, got $(c-1+szj))"))
end
out[r:r-1+szi, c:c-1+szj] = Aj
c += szj
end
if c != nc+1
throw(ArgumentError("block row $(i) has mismatched number of columns"))
throw(ArgumentError("block row $(i) has mismatched number of columns (expected $nc, got $(c-1))"))
end
r += szi
a += rows[i]
Expand All @@ -875,12 +912,12 @@ function hvcat{T<:Number}(rows::Tuple{Vararg{Int}}, xs::T...)

a = Array(T, nr, nc)
if length(a) != length(xs)
throw(ArgumentError("argument count does not match specified shape"))
throw(ArgumentError("argument count does not match specified shape (expected $(length(a)), got $(length(xs)))"))
end
k = 1
@inbounds for i=1:nr
if nc != rows[i]
throw(ArgumentError("row $(i) has mismatched number of columns"))
throw(ArgumentError("row $(i) has mismatched number of columns (expected $nc, got $(rows[i]))"))
end
for j=1:nc
a[i,j] = xs[k]
Expand All @@ -907,7 +944,7 @@ function typed_hvcat(T::Type, rows::Tuple{Vararg{Int}}, xs::Number...)
nc = rows[1]
for i = 2:nr
if nc != rows[i]
throw(ArgumentError("row $(i) has mismatched number of columns"))
throw(ArgumentError("row $(i) has mismatched number of columns (expected $nc, got $(rows[i]))"))
end
end
len = length(xs)
Expand Down Expand Up @@ -1031,7 +1068,7 @@ function sub2ind{T<:Integer}(dims::Tuple{Vararg{Integer}}, I::AbstractVector{T}.
for i=1:M
indices[i] += s*(Ij[i]-1)
end
s*= (j <= N ? dims[j] : 1)
s *= (j <= N ? dims[j] : 1)
end
return indices
end
Expand Down Expand Up @@ -1125,15 +1162,21 @@ function map(f, iters...)
nxtvals = cell(len)
cont = true
for idx in 1:len
done(iters[idx], states[idx]) && (cont = false; break)
if done(iters[idx], states[idx])
cont = false
break
end
end
while cont
for idx in 1:len
nxtvals[idx],states[idx] = next(iters[idx], states[idx])
end
push!(result, f(nxtvals...))
for idx in 1:len
done(iters[idx], states[idx]) && (cont = false; break)
if done(iters[idx], states[idx])
cont = false
break
end
end
end
result
Expand Down
14 changes: 10 additions & 4 deletions test/copy.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,20 @@ for (dest, src, bigsrc, emptysrc, res) in [
@test copy!(copy(dest), 99, src(), 99, 0) == dest

@test copy!(copy(dest), 1, emptysrc()) == dest
@test_throws BoundsError copy!(dest, 1, emptysrc(), 1)
x = emptysrc()
exc = isa(x, AbstractArray) ? BoundsError : ArgumentError
@test_throws exc copy!(dest, 1, emptysrc(), 1)

for idx in [0, 4]
for idx in (0, 4)
@test_throws BoundsError copy!(dest, idx, src())
@test_throws BoundsError copy!(dest, idx, src(), 1)
@test_throws BoundsError copy!(dest, idx, src(), 1, 1)
@test_throws BoundsError copy!(dest, 1, src(), idx)
@test_throws BoundsError copy!(dest, 1, src(), idx, 1)
x = src()
exc = isa(x, AbstractArray) ? BoundsError : ArgumentError
@test_throws exc copy!(dest, 1, x, idx)
x = src()
exc = isa(x, AbstractArray) ? BoundsError : ArgumentError
@test_throws exc copy!(dest, 1, x, idx, 1)
end

@test_throws BoundsError copy!(dest, 1, src(), 1, -1)
Expand Down

0 comments on commit cecee20

Please sign in to comment.