Skip to content

Commit

Permalink
Treat bounds check in isassigned as, well, bounds check
Browse files Browse the repository at this point in the history
The change to `isassigned(::SimpleVector, ...)` and `isassigned(::BitArray, ...)`
are currently no op since they will unlikely be inlined.
The `SimpleVector` version is also currently unsafe to inline.
These might change with a better inlining heuristic (#22210) and a smarter GC frame
allocation pass (#21888).
  • Loading branch information
yuyichao committed Jun 6, 2017
1 parent 80369bf commit c574949
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 3 deletions.
2 changes: 1 addition & 1 deletion base/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ sizeof(a::Array) = elsize(a) * length(a)
function isassigned(a::Array, i::Int...)
@_inline_meta
ii = (sub2ind(size(a), i...) % UInt) - 1
ii < length(a) % UInt || return false
@boundscheck ii < length(a) % UInt || return false
ccall(:jl_array_isassigned, Cint, (Any, UInt), a, ii) == 1
end

Expand Down
2 changes: 1 addition & 1 deletion base/essentials.jl
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ false
function isassigned end

function isassigned(v::SimpleVector, i::Int)
1 <= i <= length(v) || return false
@boundscheck 1 <= i <= length(v) || return false
x = unsafe_load(convert(Ptr{Ptr{Void}},data_pointer_from_objref(v)) + i*sizeof(Ptr))
return x != C_NULL
end
Expand Down
2 changes: 1 addition & 1 deletion base/multidimensional.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1229,7 +1229,7 @@ end
@nexprs $N d->begin
l = size(B,d)
stride *= l
1 <= I_{d-1} <= l || return false
@boundscheck 1 <= I_{d-1} <= l || return false
index += (I_d - 1) * stride
end
return isassigned(B, index)
Expand Down
10 changes: 10 additions & 0 deletions test/boundscheck_exec.jl
Original file line number Diff line number Diff line change
Expand Up @@ -209,4 +209,14 @@ else
@test_throws BoundsError V1()
end

# This tests both the bounds check elision and the behavior of `jl_array_isassigned`
# For `isbits` array the `ccall` should return a constant `true` and does not access
# the array
inbounds_isassigned(a, i) = @inbounds return isassigned(a, i)
if bc_opt == bc_default || bc_opt == bc_off
@test inbounds_isassigned(Int[], 2) == true
else
@test inbounds_isassigned(Int[], 2) == false
end

end

0 comments on commit c574949

Please sign in to comment.