bounds checks on string length(s, i, j)

Author: StefanKarpinski

base/strings/basic.jl

@@ -313,15 +313,17 @@ isless(a::Symbol, b::Symbol) = cmp(a, b) < 0
 ## character index arithmetic ##
 
 """
-    length(s::AbstractString, lo::Integer=1, hi::Integer=ncodeunits(s)) -> Integer
+    length(s::AbstractString, i::Integer, j::Integer) -> Int
+    length(s::AbstractString) -> Int
 
-The number of characters in string `s` from indices `lo` through `hi`. This is
-computed as the number of code unit indices from `lo` to `hi` which are valid
+The number of characters in string `s` from indices `i` through `j`. This is
+computed as the number of code unit indices from `i` to `j` which are valid
 character indices. Without only a single string argument, this computes the
-number of characters in the entire string. With `lo` and `hi` arguments it computes
-the number of indices between `lo` and `hi` inclusive that are valid indices in
-the string `s`. Note that the trailing character may include code units past `hi`
-and still be counted.
+number of characters in the entire string. With `i` and `j` arguments it
+computes the number of indices between `i` and `j` inclusive that are valid
+indices in the string `s`. In addition to in-bounds values, `i` may take the
+out-of-bounds value `ncodeunits(s) + 1` and `j` may take the out-of-bounds
+value `0`.
 
 See also: [`isvalid`](@ref), [`ncodeunits`](@ref), [`endof`](@ref),
 [`thisind`](@ref), [`nextind`](@ref), [`prevind`](@ref)
@@ -332,18 +334,23 @@ julia> length("jμΛIα")
 5
 ```
 """
-function length(s::AbstractString, lo::Integer=1, hi::Integer=ncodeunits(s))
-    lo ≤ hi || return 0
-    z = ncodeunits(s)
-    a = Int(max(1, min(z, lo)))
-    b = Int(min(z, max(1, hi)))
-    n = a - b
-    for i = a:b
-        n += isvalid(s, i)
+@propagate_inbounds length(s::AbstractString, i::Integer, j::Integer) =
+    length(s, Int(i), Int(j))
+
+function length(s::AbstractString, i::Int, j::Int)
+    @boundscheck begin
+        0 < i ≤ ncodeunits(s)+1 || throw(BoundsError(s, i))
+        0 ≤ j < ncodeunits(s)+1 || throw(BoundsError(s, j))
     end
-    return n + hi - lo
+    n = 0
+    for k = i:j
+        @inbounds n += isvalid(s, k)
+    end
+    return n
 end
 
+length(s::AbstractString) = @inbounds return length(s, 1, ncodeunits(s))
+
 """
     thisind(s::AbstractString, i::Integer) -> Int
 

base/strings/string.jl

@@ -232,18 +232,16 @@ function getindex(s::String, r::UnitRange{Int})
     return ss
 end
 
-function length(s::String, lo::Int, hi::Int)
-    i, n = lo, hi
-    c = max(0, hi - lo + 1)
+function length(s::String, i::Int, j::Int)
     @boundscheck begin
-        z = ncodeunits(s)
-        i = Int(max(1, min(z, lo)))
-        n = Int(min(z, max(1, hi)))
+        0 < i ≤ ncodeunits(s)+1 || throw(BoundsError(s, i))
+        0 ≤ j < ncodeunits(s)+1 || throw(BoundsError(s, j))
     end
-    i < n || return c
-    @inbounds i, j = thisind(s, i), i
-    c -= i < j
-    _length(s, i, n, c)
+    j < i && return 0
+    c = j - i + 1
+    @inbounds i, k = thisind(s, i), i
+    c -= i < k
+    _length(s, i, j, c)
 end
 
 length(s::String) = _length(s, 1, ncodeunits(s), ncodeunits(s))

test/strings/basic.jl

@@ -99,13 +99,13 @@ end
 end
 
 @testset "issue #7248" begin
-    @test length("hello", 1, -1) == 0
+    @test_throws BoundsError length("hello", 1, -1) == 0
     @test prevind("hello", 0, 1) == -1
-    @test length("hellø", 1, -1) == 0
+    @test_throws BoundsError length("hellø", 1, -1) == 0
     @test prevind("hellø", 0, 1) == -1
-    @test length("hello", 1, 10) == 10
+    @test_throws BoundsError length("hello", 1, 10) == 10
     @test nextind("hello", 0, 10) == 10
-    @test length("hellø", 1, 10) == 9
+    @test_throws BoundsError length("hellø", 1, 10) == 9
     @test nextind("hellø", 0, 10) == 11
     @test_throws BoundsError checkbounds("hello", 0)
     @test_throws BoundsError checkbounds("hello", 6)

test/strings/types.jl

@@ -205,8 +205,10 @@ let s = "Σx + βz - 2"
 end
 
 let ss = SubString("hello", 1, 5)
-    @test length(ss, 1, -1) == 0
-    @test length(ss, 1, 10) == 10
+    @test length(ss, 1, 0) == 0
+    @test_throws BoundsError length(ss, 1, -1) == 0
+    @test_throws BoundsError length(ss, 1, 6)
+    @test_throws BoundsError length(ss, 1, 10)
     @test prevind(ss, 0, 1) == -1
     @test nextind(ss, 0, 10) == 10
 end