From f4bb1baa20d2ecca71abae394673c57e80f4a8c1 Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Wed, 30 Aug 2017 18:26:50 -0400 Subject: [PATCH 1/2] Document boundscheck macro Fix #20469. [ci skip] --- base/essentials.jl | 42 ++++++++++++++++++++++++++++++++++++++++++ doc/src/stdlib/base.md | 1 + 2 files changed, 43 insertions(+) diff --git a/base/essentials.jl b/base/essentials.jl index fcb94f19822f8..b43d3087ed838 100644 --- a/base/essentials.jl +++ b/base/essentials.jl @@ -412,6 +412,46 @@ section of the Metaprogramming chapter of the manual for more details and exampl """ esc(@nospecialize(e)) = Expr(:escape, e) +""" + @boundscheck(blk) + +Annotates the expression `blk` as a bounds checking block, allowing it to be elided by [`@inbounds`](@ref). + +Note that the function in which `@boundscheck` is written must be inlined into +its caller with [`@inline`](@ref) in order for `@inbounds` to have effect. + +```jldoctest +julia> @inline function g(A, i) + @boundscheck checkbounds(A, i) + return "accessing (\$A)[\$i]" + end + f1() = return g(1:2, -1) + f2() = @inbounds return g(1:2, -1) +f2 (generic function with 1 method) + +julia> f1() +ERROR: BoundsError: attempt to access 2-element UnitRange{Int64} at index [-1] +Stacktrace: + [1] throw_boundserror(::UnitRange{Int64}, ::Tuple{Int64}) at ./abstractarray.jl:428 + [2] checkbounds at ./abstractarray.jl:392 [inlined] + [3] g at ./REPL[20]:2 [inlined] + [4] f1() at ./REPL[20]:5 + +julia> f2() +"accessing (1:2)[-1]" +``` + +!!! warning + + The `@boundscheck` annotation allows you, as a library writer, to opt-in to + allowing *other code* to remove your bounds checks with [`@inbounds`](@ref). + As noted there, the caller must verify—using information they can access—that + their accesses are valid before using `@inbounds`. For indexing into your + [`AbstractArray`](@ref) subclasses, for example, this involves checking the + indices against its [`size`](@ref). Therefore, `@boundscheck` annotations + should only be added to a [`getindex`](@ref) or [`setindex!`](@ref) + implementation after you are certain its behavior is correct. +""" macro boundscheck(blk) return Expr(:if, Expr(:boundscheck), esc(blk)) end @@ -438,6 +478,8 @@ end Using `@inbounds` may return incorrect results/crashes/corruption for out-of-bounds indices. The user is responsible for checking it manually. + Only use `@inbounds` when it is certain from the information locally available + that all accesses are in bounds. """ macro inbounds(blk) return Expr(:block, diff --git a/doc/src/stdlib/base.md b/doc/src/stdlib/base.md index cee6d5872db44..15587998cb32d 100644 --- a/doc/src/stdlib/base.md +++ b/doc/src/stdlib/base.md @@ -146,6 +146,7 @@ Base.@eval Base.evalfile Base.esc Base.@inbounds +Base.@boundscheck Base.@inline Base.@noinline Base.@nospecialize From 09f727eeb084c4cc060196b2db2edeace4475d08 Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Wed, 30 Aug 2017 18:34:57 -0400 Subject: [PATCH 2/2] Add test for issue #20469 --- test/boundscheck_exec.jl | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/boundscheck_exec.jl b/test/boundscheck_exec.jl index d94cd5e95e915..35e61536ddb2d 100644 --- a/test/boundscheck_exec.jl +++ b/test/boundscheck_exec.jl @@ -229,4 +229,14 @@ else @test inbounds_isassigned(Int[], 2) == false end +# Test that @inbounds annotations don't propagate too far for Array; Issue #20469 +struct BadVector20469{T} <: AbstractVector{Int} + data::T +end +Base.size(X::BadVector20469) = size(X.data) +Base.getindex(X::BadVector20469, i::Int) = X.data[i-1] +if bc_opt != bc_off + @test_throws BoundsError BadVector20469([1,2,3])[:] +end + end