Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Add index hint to BoundsError message #43815

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
8 changes: 8 additions & 0 deletions base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,14 @@ checkbounds_indices(::Type{Bool}, ::Tuple{}, ::Tuple{}) = true

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

function describe_valid_indices(io::IO, a::AbstractArray{<:Any}, i=nothing)
if any(x->!isa(x, OneTo), axes(a))
print(io, ", valid indices are ")
return show_index(io, axes(a))
end
any(iszero, i) && print(io, ", by default indices start from 1")
end

# check along a single dimension
"""
checkindex(Bool, inds::AbstractUnitRange, index)
Expand Down
27 changes: 26 additions & 1 deletion base/errorshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,15 @@ show_index(io::IO, x::Slice) = show_index(io, x.indices)
show_index(io::IO, x::LogicalIndex) = summary(io, x.mask)
show_index(io::IO, x::OneTo) = print(io, "1:", x.stop)
show_index(io::IO, x::Colon) = print(io, ':')

function show_index(io::IO, x::Tuple)
print(io, '[')
show_index(io, first(x))
for el in x[2:end]
print(io, ", ")
show_index(io, el)
end
print(io, ']')
end

function showerror(io::IO, ex::BoundsError)
print(io, "BoundsError")
Expand All @@ -59,6 +67,23 @@ function showerror(io::IO, ex::BoundsError)
Experimental.show_error_hints(io, ex)
end

Experimental.register_error_hint(BoundsError) do io, ex
isdefined(ex, :a) || return nothing
isdefined(ex, :i) && return describe_valid_indices(io, ex.a, ex.i)
describe_valid_indices(io, ex.a)
end

"""
describe_valid_indices(io, a, i)

Describe valid ways to index `a` in human-readable form. This should typically be a full
clause starting ", valid indices are ". Will be shown to the user upon `BoundsError`.

`i` may be ignored, but could be used to determine what information to show.
"""
describe_valid_indices(io::IO, a, i) = nothing


function showerror(io::IO, ex::TypeError)
print(io, "TypeError: ")
if ex.expected === Bool
Expand Down
2 changes: 1 addition & 1 deletion base/strings/basic.jl
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ julia> prevind("α", 1)
0

julia> prevind("α", 0)
ERROR: BoundsError: attempt to access 2-codeunit String at index [0]
ERROR: BoundsError: attempt to access 2-codeunit String at index [0], by default indices start from 1
[...]

julia> prevind("α", 2, 2)
Expand Down
9 changes: 9 additions & 0 deletions base/strings/string.jl
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ function Base.showerror(io::IO, exc::StringIndexError)
end
end

function describe_valid_indices(io::IO, a::AbstractString, i=nothing)
print(io,
", valid indices are between ",
firstindex(a),
" and ",
lastindex(a)
)
end

const ByteArray = Union{CodeUnits{UInt8,String}, Vector{UInt8},Vector{Int8}, FastContiguousSubArray{UInt8,1,CodeUnits{UInt8,String}}, FastContiguousSubArray{UInt8,1,Vector{UInt8}}, FastContiguousSubArray{Int8,1,Vector{Int8}}}

@inline between(b::T, lo::T, hi::T) where {T<:Integer} = (lo ≤ b) & (b ≤ hi)
Expand Down
1 change: 1 addition & 0 deletions base/tuple.jl
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,7 @@ function sym_in(x::Symbol, @nospecialize itr::Tuple{Vararg{Symbol}})
end
in(x::Symbol, @nospecialize itr::Tuple{Vararg{Symbol}}) = sym_in(x, itr)

describe_valid_indices(io::IO, a::Tuple, i=nothing) = iszero(i) && return print(io, ", tuple indices start from 1")

"""
empty(x::Tuple)
Expand Down
32 changes: 32 additions & 0 deletions doc/src/manual/interfaces.md
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ ourselves, we can officially define it as a subtype of an [`AbstractArray`](@ref
| `axes(A)` | `map(OneTo, size(A))` | Return a tuple of `AbstractUnitRange{<:Integer}` of valid indices |
| `similar(A, ::Type{S}, inds)` | `similar(A, S, Base.to_shape(inds))` | Return a mutable array with the specified indices `inds` (see below) |
| `similar(T::Union{Type,Function}, inds)` | `T(Base.to_shape(inds))` | Return an array similar to `T` with the specified indices `inds` (see below) |
| `describe_valid_indices(io, A, i)` | | Human readable version of the sentence "Valid indices are ...", for the `BoundsError` message |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For AbstractArrays a default fallback which shows axes should be enough, no? Feels pretty strange that with the basic methods above you get everything from sorting to matrix multiplication working, but you have to define this extremely specific thing.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is such a fallback, this is in the "optional" section.


If a type is defined as a subtype of `AbstractArray`, it inherits a very large set of rich behaviors
including iteration and multidimensional indexing built on top of single-element access. See
Expand Down Expand Up @@ -400,6 +401,37 @@ so that the `dims` argument (ordinarily a `Dims` size-tuple) can accept `Abstrac
perhaps range-types `Ind` of your own design. For more information, see
[Arrays with custom indices](@ref man-custom-indices).

When a user indexes into an array using an invalid index, a 'BoundsError' is
thrown. Along with this error, a hint can be printed telling the user which
indices they *could* have used. If there is a more intuitive description of your
type's valid indices than the default, which uses `axes(A)`, you can define a
method to `describe_valid_indices(io::IO, A, i)`. If, for instance, we wanted to
protect our `SparseArray` from bounds errors, we could redefine `getindex`:

```jldoctest squarevectype
julia> function Base.getindex(A::SparseArray{T,N}, I::Vararg{Int,N}) where {T,N}
any(I .> A.dims) && throw(BoundsError(A, I))
get(A.data, I, zero(T))
end

julia> A[4, 3]
ERROR: BoundsError: attempt to access 3×3 SparseArray{Float64, 2} at index [4, 3]
```

This message could be made more informative. We implemented a check for indices
above the maxima, but `A[0, 0]` will happily evaluate. If this is by intention,
we can update the error message:

```jldoctest squarevectype
julia> Base.describe_valid_indices(io::IO, A::SparseArray, i=nothing) = print(io, ", valid indices are <= ", A.dims)

julia> A[0, 0]
0.0

julia> A[4, 3]
ERROR: BoundsError: attempt to access 3×3 SparseArray{Float64, 2} at index [4, 3], valid indices are <= (3, 3)
```

## [Strided Arrays](@id man-interface-strided-arrays)

| Methods to implement | | Brief description |
Expand Down
2 changes: 1 addition & 1 deletion doc/src/manual/strings.md
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ Using an index less than `begin` (`1`) or greater than `end` raises an error:

```jldoctest helloworldstring
julia> str[begin-1]
ERROR: BoundsError: attempt to access 14-codeunit String at index [0]
ERROR: BoundsError: attempt to access 14-codeunit String at index [0], valid indices are between 1 and 14
[...]

julia> str[end+1]
Expand Down
3 changes: 2 additions & 1 deletion test/errorshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ struct Bounded # not an AbstractArray
end
Base.getindex(b::Bounded, i) = checkindex(Bool, 1:b.bound, i) || throw(BoundsError(b, i))
Base.summary(io::IO, b::Bounded) = print(io, "$(b.bound)-size Bounded")
Base.describe_valid_indices(io::IO, b::Bounded, i=nothing) = print(io, ", valid indices are 1:$(b.bound)")
let undefvar
err_str = @except_strbt sqrt(-1) DomainError
@test occursin("Try sqrt(Complex(x)).", err_str)
Expand All @@ -313,7 +314,7 @@ let undefvar
@test err_str == "BoundsError: attempt to access 3-element Vector{$Int} at index [6×7 BitMatrix]"

err_str = @except_str Bounded(2)[3] BoundsError
@test err_str == "BoundsError: attempt to access 2-size Bounded at index [3]"
@test err_str == "BoundsError: attempt to access 2-size Bounded at index [3], valid indices are 1:2"

err_str = @except_str 0::Bool TypeError
@test err_str == "TypeError: non-boolean ($Int) used in boolean context"
Expand Down