Add reverseind() for AbstractString, re-enable tests with GenericString

reverse() for GenericString/AbstractString returns a RevString, whose
indexing behavior is very different from a reverse()'d String which
is returned for String. Thus, calling reverseind() on the underlying String
object is not correct for GenericString. Add a generic but O(n) method for
AbstractString and use it for GenericString.

Author: nalimilan

base/strings/string.jl

@@ -105,7 +105,7 @@ function first_utf8_byte(ch::Char)
 end
 
 function reverseind(s::String, i::Integer)
-    j = lastidx(s) + 1 - i
+    j = length(s.data) + 1 - i
     d = s.data
     while is_valid_continuation(d[j])
         j -= 1
@@ -117,8 +117,6 @@ end
 
 sizeof(s::String) = sizeof(s.data)
 
-lastidx(s::String) = length(s.data)
-
 isvalid(s::String, i::Integer) =
     (1 <= i <= endof(s.data)) && !is_valid_continuation(s.data[i])
 

base/strings/types.jl

@@ -118,12 +118,11 @@ reverse(s::RevString) = s.string
 
 ## reverse an index i so that reverse(s)[i] == s[reverseind(s,i)]
 
+reverseind(s::AbstractString, i) = chr2ind(s, length(s) + 1 - ind2chr(reverse(s), i))
 reverseind(s::Union{DirectIndexString,SubString{DirectIndexString}}, i::Integer) = length(s) + 1 - i
 reverseind(s::RevString, i::Integer) = endof(s) - i + 1
-lastidx(s::AbstractString) = nextind(s, endof(s)) - 1
-lastidx(s::DirectIndexString) = length(s)
-reverseind(s::SubString, i::Integer) =
-    reverseind(s.string, lastidx(s.string)-s.offset-s.endof+i) - s.offset
+reverseind(s::SubString{String}, i::Integer) =
+    reverseind(s.string, nextind(s.string, endof(s.string))-s.offset-s.endof+i-1) - s.offset
 
 ## efficient representation of repeated strings ##
 

base/test.jl

@@ -1011,6 +1011,5 @@ end
 Base.convert(::Type{GenericString}, s::AbstractString) = GenericString(s)
 Base.endof(s::GenericString) = endof(s.string)
 Base.next(s::GenericString, i::Int) = next(s.string, i)
-Base.reverseind(s::GenericString, i::Integer) = reverseind(s.string, i)
 
 end # module

test/strings/types.jl

@@ -163,29 +163,30 @@ rs = RevString("foobar")
 @test rsplit(RevString("ailuj"),'l') == ["ju","ia"]
 @test parse(Float64,RevString("64")) === 46.0
 
-# # reverseind
-# for T in (String, GenericString)
-#     for prefix in ("", "abcd", "\U0001d6a4\U0001d4c1", "\U0001d6a4\U0001d4c1c", " \U0001d6a4\U0001d4c1")
-#         for suffix in ("", "abcde", "\U0001d4c1β\U0001d6a4", "\U0001d4c1β\U0001d6a4c", " \U0001d4c1β\U0001d6a4")
-#             for c in ('X', 'δ', '\U0001d6a5')
-#                 @show (T,prefix,suffix,c)
-#                 s = convert(T, string(prefix, c, suffix))
-#                 r = convert(T, String(reverse(s)))
-#                 ri = search(r, c)
-#                 @test r == RevString(s)
-#                 @test c == s[reverseind(s, ri)] == r[ri]
-#                 s = RevString(s)
-#                 ri = search(r, c)
-#                 @test c == s[reverseind(s, ri)] == r[ri]
-#                 s = convert(T, string(prefix, prefix, c, suffix, suffix))
-#                 pre = convert(T, prefix)
-#                 sb = SubString(s, nextind(pre, endof(pre)), endof(convert(T, string(prefix, prefix, c, suffix))))
-#                 ri = search(reverse(sb), c)
-#                 @test c == sb[reverseind(sb, ri)] == reverse(sb)[ri]
-#             end
-#         end
-#     end
-# end
+# reverseind
+for T in (String, GenericString)
+    for prefix in ("", "abcd", "\U0001d6a4\U0001d4c1", "\U0001d6a4\U0001d4c1c", " \U0001d6a4\U0001d4c1")
+        for suffix in ("", "abcde", "\U0001d4c1β\U0001d6a4", "\U0001d4c1β\U0001d6a4c", " \U0001d4c1β\U0001d6a4")
+            for c in ('X', 'δ', '\U0001d6a5')
+                s = convert(T, string(prefix, c, suffix))
+                r = reverse(s)
+                ri = search(r, c)
+                @test r == RevString(s)
+                @test c == s[reverseind(s, ri)] == r[ri]
+                s = RevString(s)
+                r = reverse(s)
+                ri = search(r, c)
+                @test c == s[reverseind(s, ri)] == r[ri]
+                s = convert(T, string(prefix, prefix, c, suffix, suffix))
+                pre = convert(T, prefix)
+                sb = SubString(s, nextind(pre, endof(pre)), endof(convert(T, string(prefix, prefix, c, suffix))))
+                r = reverse(sb)
+                ri = search(r, c)
+                @test c == sb[reverseind(sb, ri)] == r[ri]
+            end
+        end
+    end
+end
 
 ## Repeat strings ##