Refactor the guts of String.reverse_index into a new Parrot_str_find_…

…reverse_index API routine. Make the 'start' param to the reverse_index method optional, and default to the string length for easy searching.

include/parrot/string_funcs.h

@@ -166,6 +166,16 @@ INTVAL Parrot_str_find_not_cclass(PARROT_INTERP,
     UINTVAL count)
         __attribute__nonnull__(1);
 
+PARROT_EXPORT
+PARROT_WARN_UNUSED_RESULT
+INTVAL Parrot_str_find_reverse_index(PARROT_INTERP,
+    ARGIN(const STRING *src),
+    ARGIN(const STRING *search),
+    INTVAL start)
+        __attribute__nonnull__(1)
+        __attribute__nonnull__(2)
+        __attribute__nonnull__(3);
+
 PARROT_EXPORT
 void Parrot_str_finish(PARROT_INTERP)
         __attribute__nonnull__(1);
@@ -540,6 +550,10 @@ STRING * Parrot_str_new_from_cstring(PARROT_INTERP,
     , PARROT_ASSERT_ARG(search))
 #define ASSERT_ARGS_Parrot_str_find_not_cclass __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
        PARROT_ASSERT_ARG(interp))
+#define ASSERT_ARGS_Parrot_str_find_reverse_index __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
+       PARROT_ASSERT_ARG(interp) \
+    , PARROT_ASSERT_ARG(src) \
+    , PARROT_ASSERT_ARG(search))
 #define ASSERT_ARGS_Parrot_str_finish __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
        PARROT_ASSERT_ARG(interp))
 #define ASSERT_ARGS_Parrot_str_format_data __attribute__unused__ int _ASSERT_ARGS_CHECK = (\

src/pmc/string.pmc

@@ -742,26 +742,15 @@ Find last occurrence of C<substring>, but not after the C<start> position.
 
 */
 
-    METHOD reverse_index(STRING *substring, INTVAL start) {
+    METHOD reverse_index(STRING *substring,
+            INTVAL start :optional, INTVAL has_start :opt_flag) {
         const STRING * const src = VTABLE_get_string(INTERP, SELF);
-        INTVAL         res = -1;
+        INTVAL         res;
         UINTVAL        len;
 
-        if (start < 0)
-            RETURN(INTVAL res);
-
-        len = Parrot_str_length(INTERP, src);
-
-        if (!len)
-            RETURN(INTVAL res);
-
-        if (start >= (INTVAL)len)
-            RETURN(INTVAL res);
-
-        if (!Parrot_str_length(INTERP, substring))
-            RETURN(INTVAL res);
-
-        res = STRING_rindex(INTERP, src, substring, (UINTVAL)start);
+        if (!has_start)
+            start = STRING_length(src);
+        res = Parrot_str_find_reverse_index(INTERP, src, substring, start);
         RETURN(INTVAL res);
     }
 

src/string/api.c

@@ -912,6 +912,15 @@ exception if C<src> is null.
 
 Identical to the STRING_index macro.
 
+=item C<INTVAL Parrot_str_find_reverse_index(PARROT_INTERP, const STRING *src,
+const STRING *search, INTVAL start)>
+
+Returns the last character position of the second Parrot string C<search> in
+C<src>, not after C<start>. Returns the last position found, or -1 if no
+instances are found.
+
+Mostly identical to the STRING_rindex macro.
+
 =cut
 
 */
@@ -930,6 +939,23 @@ Parrot_str_find_index(PARROT_INTERP, ARGIN(const STRING *src),
     return STRING_index(interp, src, search, start);
 }
 
+PARROT_EXPORT
+PARROT_WARN_UNUSED_RESULT
+INTVAL
+Parrot_str_find_reverse_index(PARROT_INTERP, ARGIN(const STRING *src),
+    ARGIN(const STRING *search), INTVAL start)
+{
+    ASSERT_ARGS(Parrot_str_find_reverse_index)
+    INTVAL len = Parrot_str_length(interp, src);
+
+    if (start <= 0 || !len || start > len)
+        return -1;
+
+    if (!Parrot_str_length(interp, search))
+        return -1;
+
+    return STRING_rindex(interp, src, search, (UINTVAL)start);
+}
 
 /*
 

t/pmc/string.t

@@ -762,7 +762,7 @@ TEST:
   is( $I0, -1, "far far away -1" )
 
   $I0 = $P0.'reverse_index'('l', 0)
-  is( $I0, 9, "search1 9" )
+  is( $I0, -1, "reverse_index starting from 0 is -1" )
 
   $I0 = $P0.'reverse_index'('l', 8)
   is( $I0, 3, "search2 3" )
@@ -775,7 +775,7 @@ TEST:
   is( $I0, -1, "negative start -1 unicode" )
 
   $I0 = $P0.'reverse_index'('o', 24)
-  is( $I0, -1, "out of bounds -1 unicode" )
+  is( $I0, 20, "start = strlen works" )
 
   $I0 = $P0.'reverse_index'('string', 23)
   is( $I0, 0, "search1 unicode" )