Skip to content

Commit

Permalink
don't capture arrays/tuples in BoundsError
Browse files Browse the repository at this point in the history
This commit changes the semantics of `BoundsError` so that it doesn't
capture arrays/tuples passed to its constructor.

The change wouldn't improve any performance by itself because the
`BoundsError` constructor is mostly called on error paths and thus
it is opaque to analyses/optimizers when compiling a caller context.
Rather it's supposed to be a building block for broadening the possibility
of certain memory optimizations in the future such as copy elision for
`ImmutableArray` construction and stack allocation of `Array`s, by
allowing us to assume the invariant that primitive indexing operations
into array/tuple indexing don't escape it so that our escape analyses
can achieve further accuracy.

Specifically, when `BoundsError` constructor will now compute the "summary"
of its `Array`/`Tuple` argument, rather than capturing it for the later
inspection. Assuming `Base.summary(x::Array)` or `Base.summary(x::Tuple)`
don't escape `x`, we can assume that `BoundsError` doesn't escape `x`.
This change won't appear as breaking in most cases since `showerror` will
print the exact same error message as before, but obviously this is
technically breaking since we can no longer access to the original
arrays/tuples by catching `BoundsError`.
I'd say this breaking semantic change would be still acceptable, since
I think it's enough if we can know size/type of arrays/tuples from
`BoundsError` in most cases.

As a last note, I didn't change the semantics for arbitrary user objects,
since we still don't have a good infrastructure to tell the compiler some
primitive assumptions/rules about user type objects anyway.
  • Loading branch information
aviatesk authored and Ian Atol committed Mar 4, 2022
1 parent 96d6d86 commit a3dfdbd
Show file tree
Hide file tree
Showing 40 changed files with 244 additions and 125 deletions.
3 changes: 3 additions & 0 deletions base/Base.jl
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ modifyproperty!(x, f::Symbol, op, v, order::Symbol=:notatomic) =
replaceproperty!(x, f::Symbol, expected, desired, success_order::Symbol=:notatomic, fail_order::Symbol=success_order) =
(@inline; Core.replacefield!(x, f, expected, convert(fieldtype(typeof(x), f), desired), success_order, fail_order))

throw_boundserror(a, i) = (@noinline; throw(BoundsError(a, i)))
throw_boundserror() = (@noinline; throw(BoundsError()))

convert(::Type{Any}, Core.@nospecialize x) = x
convert(::Type{T}, x::T) where {T} = x
include("coreio.jl")
Expand Down
20 changes: 9 additions & 11 deletions base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -700,8 +700,6 @@ end
checkbounds_indices(::Type{Bool}, IA::Tuple, ::Tuple{}) = (@inline; all(x->length(x)==1, IA))
checkbounds_indices(::Type{Bool}, ::Tuple{}, ::Tuple{}) = true

throw_boundserror(A, I) = (@noinline; throw(BoundsError(A, I)))

# check along a single dimension
"""
checkindex(Bool, inds::AbstractUnitRange, index)
Expand Down Expand Up @@ -956,7 +954,7 @@ function copyto!(dest::AbstractArray, dstart::Integer, src, sstart::Integer, n::
if (dstart inds || dmax inds) | (sstart < 1)
sstart < 1 && throw(ArgumentError(LazyString("source start offset (",
sstart,") is < 1")))
throw(BoundsError(dest, dstart:dmax))
throw_boundserror(dest, dstart:dmax)
end
y = iterate(src)
for j = 1:(sstart-1)
Expand All @@ -974,7 +972,7 @@ function copyto!(dest::AbstractArray, dstart::Integer, src, sstart::Integer, n::
y = iterate(src, st)
i += 1
end
i <= dmax && throw(BoundsError(dest, i))
i <= dmax && throw_boundserror(dest, i)
return dest
end

Expand Down Expand Up @@ -1027,7 +1025,7 @@ function copyto_unaliased!(deststyle::IndexStyle, dest::AbstractArray, srcstyle:
idf, isf = first(destinds), first(srcinds)
Δi = idf - isf
(checkbounds(Bool, destinds, isf+Δi) & checkbounds(Bool, destinds, last(srcinds)+Δi)) ||
throw(BoundsError(dest, srcinds))
throw_boundserror(dest, srcinds)
if deststyle isa IndexLinear
if srcstyle isa IndexLinear
# Single-index implementation
Expand Down Expand Up @@ -1067,7 +1065,7 @@ end

function copyto!(dest::AbstractArray, dstart::Integer, src::AbstractArray, sstart::Integer)
srcinds = LinearIndices(src)
checkbounds(Bool, srcinds, sstart) || throw(BoundsError(src, sstart))
checkbounds(Bool, srcinds, sstart) || throw_boundserror(src, sstart)
copyto!(dest, dstart, src, sstart, last(srcinds)-sstart+1)
end

Expand All @@ -1078,8 +1076,8 @@ function copyto!(dest::AbstractArray, dstart::Integer,
n < 0 && throw(ArgumentError(LazyString("tried to copy n=",
n," elements, but n should be nonnegative")))
destinds, srcinds = LinearIndices(dest), LinearIndices(src)
(checkbounds(Bool, destinds, dstart) && checkbounds(Bool, destinds, dstart+n-1)) || throw(BoundsError(dest, dstart:dstart+n-1))
(checkbounds(Bool, srcinds, sstart) && checkbounds(Bool, srcinds, sstart+n-1)) || throw(BoundsError(src, sstart:sstart+n-1))
(checkbounds(Bool, destinds, dstart) && checkbounds(Bool, destinds, dstart+n-1)) || throw_boundserror(dest, dstart:dstart+n-1)
(checkbounds(Bool, srcinds, sstart) && checkbounds(Bool, srcinds, sstart+n-1)) || throw_boundserror(src, sstart:sstart+n-1)
@inbounds for i = 0:(n-1)
dest[dstart+i] = src[sstart+i]
end
Expand Down Expand Up @@ -1306,7 +1304,7 @@ _to_subscript_indices(A, J::Tuple, Jrem::Tuple) = J # already bounds-checked, sa
_to_subscript_indices(A::AbstractArray{T,N}, I::Vararg{Int,N}) where {T,N} = I
_remaining_size(::Tuple{Any}, t::Tuple) = t
_remaining_size(h::Tuple, t::Tuple) = (@inline; _remaining_size(tail(h), tail(t)))
_unsafe_ind2sub(::Tuple{}, i) = () # _ind2sub may throw(BoundsError()) in this case
_unsafe_ind2sub(::Tuple{}, i) = () # _ind2sub may throw_boundserror() in this case
_unsafe_ind2sub(sz, i) = (@inline; _ind2sub(sz, i))

## Setindex! is defined similarly. We first dispatch to an internal _setindex!
Expand Down Expand Up @@ -2667,7 +2665,7 @@ nextL(L, r::Slice) = L*length(r.indices)
offsetin(i, l::Integer) = i-1
offsetin(i, r::AbstractUnitRange) = i-first(r)

_ind2sub(::Tuple{}, ind::Integer) = (@inline; ind == 1 ? () : throw(BoundsError()))
_ind2sub(::Tuple{}, ind::Integer) = (@inline; ind == 1 ? () : throw_boundserror())
_ind2sub(dims::DimsInteger, ind::Integer) = (@inline; _ind2sub_recurse(dims, ind-1))
_ind2sub(inds::Indices, ind::Integer) = (@inline; _ind2sub_recurse(inds, ind-1))
_ind2sub(inds::Indices{1}, ind::Integer) =
Expand Down Expand Up @@ -3163,7 +3161,7 @@ function _keepat!(a::AbstractVector, inds)
end

function _keepat!(a::AbstractVector, m::AbstractVector{Bool})
length(m) == length(a) || throw(BoundsError(a, m))
length(m) == length(a) || throw_boundserror(a, m)
j = firstindex(a)
for i in eachindex(a, m)
@inbounds begin
Expand Down
2 changes: 1 addition & 1 deletion base/abstractarraymath.jl
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ julia> selectdim(A, 2, 3:4)
@noinline function _selectdim(A, d, i, idxs)
d >= 1 || throw(ArgumentError("dimension must be ≥ 1, got $d"))
nd = ndims(A)
d > nd && (i == 1 || throw(BoundsError(A, (ntuple(Returns(Colon()),d-1)..., i))))
d > nd && (i == 1 || throw_boundserror(A, (ntuple(Returns(Colon()),d-1)..., i)))
return view(A, idxs...)
end

Expand Down
10 changes: 5 additions & 5 deletions base/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ function _copyto_impl!(dest::Array, doffs::Integer, src::Array, soffs::Integer,
n == 0 && return dest
n > 0 || _throw_argerror()
if soffs < 1 || doffs < 1 || soffs+n-1 > length(src) || doffs+n-1 > length(dest)
throw(BoundsError())
throw_boundserror()
end
unsafe_copyto!(dest, doffs, src, soffs, n)
return dest
Expand Down Expand Up @@ -1568,7 +1568,7 @@ function _deleteat!(a::Vector, inds, dltd=Nowhere())
if i < q
throw(ArgumentError("indices must be unique and sorted"))
else
throw(BoundsError())
throw_boundserror()
end
end
while q < i
Expand All @@ -1589,7 +1589,7 @@ end
# Simpler and more efficient version for logical indexing
function deleteat!(a::Vector, inds::AbstractVector{Bool})
n = length(a)
length(inds) == n || throw(BoundsError(a, inds))
length(inds) == n || throw_boundserror(a, inds)
p = 1
for (q, i) in enumerate(inds)
_copy_item!(a, p, q)
Expand Down Expand Up @@ -1864,9 +1864,9 @@ function reverse!(v::AbstractVector, start::Integer, stop::Integer=lastindex(v))
liv = LinearIndices(v)
if n <= s # empty case; ok
elseif !(first(liv) s last(liv))
throw(BoundsError(v, s))
throw_boundserror(v, s)
elseif !(first(liv) n last(liv))
throw(BoundsError(v, n))
throw_boundserror(v, n)
end
r = n
@inbounds for i in s:div(s+n-1, 2)
Expand Down
44 changes: 22 additions & 22 deletions base/bitarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ function one(x::BitMatrix)
end

function copyto!(dest::BitArray, src::BitArray)
length(src) > length(dest) && throw(BoundsError(dest, length(dest)+1))
length(src) > length(dest) && throw_boundserror(dest, length(dest)+1)
destc = dest.chunks; srcc = src.chunks
nc = min(length(destc), length(srcc))
nc == 0 && return dest
Expand Down Expand Up @@ -462,15 +462,15 @@ copyto!(dest::BitArray, doffs::Integer, src::Array, soffs::Integer, n::Integer)
_copyto_int!(dest, Int(doffs), src, Int(soffs), Int(n))
function _copyto_int!(dest::BitArray, doffs::Int, src::Array, soffs::Int, n::Int)
n == 0 && return dest
soffs < 1 && throw(BoundsError(src, soffs))
doffs < 1 && throw(BoundsError(dest, doffs))
soffs+n-1 > length(src) && throw(BoundsError(src, length(src)+1))
doffs+n-1 > length(dest) && throw(BoundsError(dest, length(dest)+1))
soffs < 1 && throw_boundserror(src, soffs)
doffs < 1 && throw_boundserror(dest, doffs)
soffs+n-1 > length(src) && throw_boundserror(src, length(src)+1)
doffs+n-1 > length(dest) && throw_boundserror(dest, length(dest)+1)
return unsafe_copyto!(dest, doffs, src, soffs, n)
end

function copyto!(dest::BitArray, src::Array)
length(src) > length(dest) && throw(BoundsError(dest, length(dest)+1))
length(src) > length(dest) && throw_boundserror(dest, length(dest)+1)
length(src) == 0 && return dest
return unsafe_copyto!(dest, 1, src, 1, length(src))
end
Expand Down Expand Up @@ -811,7 +811,7 @@ resize!(B::BitVector, n::Integer) = _resize_int!(B, Int(n))
function _resize_int!(B::BitVector, n::Int)
n0 = length(B)
n == n0 && return B
n >= 0 || throw(BoundsError(B, n))
n >= 0 || throw_boundserror(B, n)
if n < n0
deleteat!(B, n+1:n0)
return B
Expand Down Expand Up @@ -888,7 +888,7 @@ insert!(B::BitVector, i::Integer, item) = _insert_int!(B, Int(i), item)
function _insert_int!(B::BitVector, i::Int, item)
i = Int(i)
n = length(B)
1 <= i <= n+1 || throw(BoundsError(B, i))
1 <= i <= n+1 || throw_boundserror(B, i)
item = convert(Bool, item)

Bc = B.chunks
Expand Down Expand Up @@ -950,7 +950,7 @@ function deleteat!(B::BitVector, i::Integer)
i isa Bool && depwarn("passing Bool as an index is deprecated", :deleteat!)
i = Int(i)
n = length(B)
1 <= i <= n || throw(BoundsError(B, i))
1 <= i <= n || throw_boundserror(B, i)

return _deleteat!(B, i)
end
Expand All @@ -959,8 +959,8 @@ function deleteat!(B::BitVector, r::AbstractUnitRange{Int})
n = length(B)
i_f = first(r)
i_l = last(r)
1 <= i_f || throw(BoundsError(B, i_f))
i_l <= n || throw(BoundsError(B, n+1))
1 <= i_f || throw_boundserror(B, i_f)
i_l <= n || throw_boundserror(B, n+1)

Bc = B.chunks
new_l = length(B) - length(r)
Expand Down Expand Up @@ -997,7 +997,7 @@ function deleteat!(B::BitVector, inds)
if !(q <= i <= n)
i isa Bool && throw(ArgumentError("invalid index $i of type Bool"))
i < q && throw(ArgumentError("indices must be unique and sorted"))
throw(BoundsError(B, i))
throw_boundserror(B, i)
end
new_l -= 1
if i > q
Expand All @@ -1023,7 +1023,7 @@ function deleteat!(B::BitVector, inds)
end

function deleteat!(B::BitVector, inds::AbstractVector{Bool})
length(inds) == length(B) || throw(BoundsError(B, inds))
length(inds) == length(B) || throw_boundserror(B, inds)

n = new_l = length(B)
y = findfirst(inds)
Expand Down Expand Up @@ -1073,7 +1073,7 @@ function splice!(B::BitVector, i::Integer)
i isa Bool && depwarn("passing Bool as an index is deprecated", :splice!)
i = Int(i)
n = length(B)
1 <= i <= n || throw(BoundsError(B, i))
1 <= i <= n || throw_boundserror(B, i)

v = B[i] # TODO: change to a copy if/when subscripting becomes an ArrayView
_deleteat!(B, i)
Expand All @@ -1090,8 +1090,8 @@ end
function _splice_int!(B::BitVector, r, ins)
n = length(B)
i_f, i_l = first(r), last(r)
1 <= i_f <= n+1 || throw(BoundsError(B, i_f))
i_l <= n || throw(BoundsError(B, n+1))
1 <= i_f <= n+1 || throw_boundserror(B, i_f)
i_l <= n || throw_boundserror(B, n+1)

Bins = convert(BitArray, ins)

Expand Down Expand Up @@ -1471,7 +1471,7 @@ end
# returns the index of the next true element, or nothing if all false
function findnext(B::BitArray, start::Integer)
start = Int(start)
start > 0 || throw(BoundsError(B, start))
start > 0 || throw_boundserror(B, start)
start > length(B) && return nothing
unsafe_bitfindnext(B.chunks, start)
end
Expand All @@ -1480,7 +1480,7 @@ end

# aux function: same as findnext(~B, start), but performed without temporaries
function findnextnot(B::BitArray, start::Int)
start > 0 || throw(BoundsError(B, start))
start > 0 || throw_boundserror(B, start)
start > length(B) && return nothing

Bc = B.chunks
Expand Down Expand Up @@ -1528,7 +1528,7 @@ function _findnext_int(testf::Function, B::BitArray, start::Int)
!f0 && f1 && return findnext(B, start)
f0 && !f1 && return findnextnot(B, start)

start > 0 || throw(BoundsError(B, start))
start > 0 || throw_boundserror(B, start)
start > length(B) && return nothing
f0 && f1 && return start
return nothing # last case: !f0 && !f1
Expand Down Expand Up @@ -1557,14 +1557,14 @@ end
function findprev(B::BitArray, start::Integer)
start = Int(start)
start > 0 || return nothing
start > length(B) && throw(BoundsError(B, start))
start > length(B) && throw_boundserror(B, start)
unsafe_bitfindprev(B.chunks, start)
end

function findprevnot(B::BitArray, start::Int)
start = Int(start)
start > 0 || return nothing
start > length(B) && throw(BoundsError(B, start))
start > length(B) && throw_boundserror(B, start)

Bc = B.chunks

Expand Down Expand Up @@ -1605,7 +1605,7 @@ function _findprev_int(testf::Function, B::BitArray, start::Int)
f0 && !f1 && return findprevnot(B, start)

start > 0 || return nothing
start > length(B) && throw(BoundsError(B, start))
start > length(B) && throw_boundserror(B, start)
f0 && f1 && return start
return nothing # last case: !f0 && !f1
end
Expand Down
24 changes: 20 additions & 4 deletions base/boot.jl
Original file line number Diff line number Diff line change
Expand Up @@ -269,14 +269,30 @@ end

macro inline() Expr(:meta, :inline) end
macro noinline() Expr(:meta, :noinline) end

import .Intrinsics: ult_int
struct Summarized
axes #=::Union{Base.OneTo, Integer}=#
type::Type
function Summarized(a)
isa(a, Array) || return a
if isdefined(Main, :Base)
return new(Main.Base.axes(a), typeof(a))
else
return new(nothing, typeof(a))
end
end
Summarized(ax, typ) = new(ax, typ)
end
struct BoundsError <: Exception
a::Any
a
i::Any
BoundsError() = new()
BoundsError(@nospecialize(a)) = (@noinline; new(a))
BoundsError(@nospecialize(a), i) = (@noinline; new(a,i))
BoundsError(@nospecialize(a)) = (@noinline; new(Summarized(a)))
BoundsError(@nospecialize(a), i) = (@noinline; new(Summarized(a), i))
BoundsError(ax, typ::Type) = (@noinline; new(Summarized((ax,), typ)))
BoundsError(ax, typ::Type, i) = (@noinline; new(Summarized((ax,), typ), i))
end

struct DivideError <: Exception end
struct OutOfMemoryError <: Exception end
struct ReadOnlyMemoryError <: Exception end
Expand Down
6 changes: 3 additions & 3 deletions base/char.jl
Original file line number Diff line number Diff line change
Expand Up @@ -192,16 +192,16 @@ typemax(::Type{Char}) = bitcast(Char, typemax(UInt32))
typemin(::Type{Char}) = bitcast(Char, typemin(UInt32))

size(c::AbstractChar) = ()
size(c::AbstractChar, d::Integer) = d < 1 ? throw(BoundsError()) : 1
size(c::AbstractChar, d::Integer) = d < 1 ? throw_boundserror() : 1
ndims(c::AbstractChar) = 0
ndims(::Type{<:AbstractChar}) = 0
length(c::AbstractChar) = 1
IteratorSize(::Type{Char}) = HasShape{0}()
firstindex(c::AbstractChar) = 1
lastindex(c::AbstractChar) = 1
getindex(c::AbstractChar) = c
getindex(c::AbstractChar, i::Integer) = i == 1 ? c : throw(BoundsError())
getindex(c::AbstractChar, I::Integer...) = all(x -> x == 1, I) ? c : throw(BoundsError())
getindex(c::AbstractChar, i::Integer) = i == 1 ? c : throw_boundserror()
getindex(c::AbstractChar, I::Integer...) = all(x -> x == 1, I) ? c : throw_boundserror()
first(c::AbstractChar) = c
last(c::AbstractChar) = c
eltype(::Type{T}) where {T<:AbstractChar} = T
Expand Down
8 changes: 4 additions & 4 deletions base/combinatorics.jl
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ isperm(P::Any32) = _isperm(P)
function swapcols!(a::AbstractMatrix, i, j)
i == j && return
cols = axes(a,2)
@boundscheck i in cols || throw(BoundsError(a, (:,i)))
@boundscheck j in cols || throw(BoundsError(a, (:,j)))
@boundscheck i in cols || throw_boundserror(a, (:,i))
@boundscheck j in cols || throw_boundserror(a, (:,j))
for k in axes(a,1)
@inbounds a[k,i],a[k,j] = a[k,j],a[k,i]
end
Expand All @@ -108,8 +108,8 @@ end
function swaprows!(a::AbstractMatrix, i, j)
i == j && return
rows = axes(a,1)
@boundscheck i in rows || throw(BoundsError(a, (:,i)))
@boundscheck j in rows || throw(BoundsError(a, (:,j)))
@boundscheck i in rows || throw_boundserror(a, (:,i))
@boundscheck j in rows || throw_boundserror(a, (:,j))
for k in axes(a,2)
@inbounds a[i,k],a[j,k] = a[j,k],a[i,k]
end
Expand Down
Loading

0 comments on commit a3dfdbd

Please sign in to comment.