Skip to content

Commit

Permalink
Restrict signature for getindex(::AbstractString, ::AbstractVector} (#…
Browse files Browse the repository at this point in the history
…20248)

Only integer indices are supported by the generic interface, so better
raise a MethodError for other element types rather than failing later.
Ideally the Bool-specific method would not be needed, but it is required
as long as Bool<:Integer.
  • Loading branch information
nalimilan authored and ararslan committed Feb 6, 2017
1 parent 7fb758a commit 1e085b9
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 7 deletions.
4 changes: 3 additions & 1 deletion base/strings/basic.jl
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@ getindex(s::AbstractString, i::Integer) = s[Int(i)]
getindex(s::AbstractString, i::Colon) = s
getindex{T<:Integer}(s::AbstractString, r::UnitRange{T}) = s[Int(first(r)):Int(last(r))]
# TODO: handle other ranges with stride ±1 specially?
getindex(s::AbstractString, v::AbstractVector) =
getindex{T<:Integer}(s::AbstractString, v::AbstractVector{T}) =
sprint(length(v), io->(for i in v; write(io,s[i]) end))
getindex(s::AbstractString, v::AbstractVector{Bool}) =
throw(ArgumentError("logical indexing not supported for strings"))

Symbol(s::AbstractString) = Symbol(String(s))

Expand Down
15 changes: 9 additions & 6 deletions test/strings/basic.jl
Original file line number Diff line number Diff line change
Expand Up @@ -172,9 +172,9 @@ gstr = GenericString("12")
@test convert(Array{Char,1}, gstr) ==['1';'2']
@test convert(Symbol, gstr)==Symbol("12")

@test getindex(gstr, Bool(1))=='1'
@test getindex(gstr,Bool(1):Bool(1))=="1"
@test getindex(gstr,AbstractVector([Bool(1):Bool(1);]))=="1"
@test gstr[1] == '1'
@test gstr[1:1] == "1"
@test gstr[[1]] == "1"

@test done(eachindex("foobar"),7)
@test eltype(Base.EachStringIndex) == Int
Expand All @@ -187,9 +187,8 @@ gstr = GenericString("12")

@test length(GenericString(""))==0

@test getindex(gstr,AbstractVector([Bool(1):Bool(1);]))=="1"

@test nextind(AbstractArray([Bool(1):Bool(1);]),1)==2
@test nextind(1:1, 1) == 2
@test nextind([1], 1) == 2

@test ind2chr(gstr,2)==2

Expand Down Expand Up @@ -461,3 +460,7 @@ Base.endof(x::CharStr) = endof(x.chars)
# Case with Unicode characters
@test cmp("\U1f596\U1f596", CharStr("\U1f596")) == 1 # Gives BoundsError with bug
@test cmp(CharStr("\U1f596"), "\U1f596\U1f596") == -1

# issue #12495: check that logical indexing attempt raises ArgumentError
@test_throws ArgumentError "abc"[[true, false, true]]
@test_throws ArgumentError "abc"[BitArray([true, false, true])]

2 comments on commit 1e085b9

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Something went wrong when running your job:

NanosoldierError: failed to run benchmarks against primary commit: ErrorException("failed process: Process(`sudo cset shield -e su nanosoldier -- -c ./benchscript.sh`, ProcessExited(1)) [1]")

Logs and partial data can be found here
cc @jrevels

Please sign in to comment.