This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
Remove redundant field from inversion lists
authorKarl Williamson <public@khwilliamson.com>
Fri, 12 Jul 2013 21:11:26 +0000 (15:11 -0600)
committerKarl Williamson <public@khwilliamson.com>
Tue, 16 Jul 2013 19:58:12 +0000 (13:58 -0600)
The number of elements in an inversion list is a simple calculation
based on SvCUR().  Prior to this patch there was a field that contained
that number directly, and the two values diverged, causing a bug.  A
single value can't get out-of-sync with itself.

embed.fnc
embed.h
inline_invlist.c
proto.h
regcomp.c
sv.h

index c471de2..a9e4215 100644 (file)
--- a/embed.fnc
+++ b/embed.fnc
@@ -1434,7 +1434,6 @@ EsM       |void   |_append_range_to_invlist   |NN SV* const invlist|const UV start|const
 EiMR   |UV*    |_invlist_array_init    |NN SV* const invlist|const bool will_have_0
 EiMR   |UV*    |invlist_array  |NN SV* const invlist
 EsM    |void   |invlist_extend    |NN SV* const invlist|const UV len
-EiMR   |bool*  |get_invlist_offset_addr|NN SV* invlist
 EiMR   |UV     |invlist_max    |NN SV* const invlist
 EiM    |void   |invlist_set_len|NN SV* const invlist|const UV len|const bool offset
 EiMR   |IV*    |get_invlist_previous_index_addr|NN SV* invlist
@@ -1473,7 +1472,7 @@ EXp       |SV*    |_core_swash_init|NN const char* pkg|NN const char* name \
 #endif
 #if defined(PERL_IN_REGCOMP_C) || defined(PERL_IN_REGEXEC_C) || defined(PERL_IN_UTF8_C)
 EXMpR  |SV*    |_invlist_contents|NN SV* const invlist
-EiMR   |STRLEN*|_get_invlist_len_addr  |NN SV* invlist
+EiMR   |bool*  |get_invlist_offset_addr|NN SV* invlist
 EiMR   |UV     |_invlist_len   |NN SV* const invlist
 EMiR   |bool   |_invlist_contains_cp|NN SV* const invlist|const UV cp
 EXpMR  |IV     |_invlist_search        |NN SV* const invlist|const UV cp
diff --git a/embed.h b/embed.h
index 468d2de..8e9b059 100644 (file)
--- a/embed.h
+++ b/embed.h
 #define compute_EXACTish(a)    S_compute_EXACTish(aTHX_ a)
 #define could_it_be_a_POSIX_class(a)   S_could_it_be_a_POSIX_class(aTHX_ a)
 #define get_invlist_iter_addr(a)       S_get_invlist_iter_addr(aTHX_ a)
-#define get_invlist_offset_addr(a)     S_get_invlist_offset_addr(aTHX_ a)
 #define get_invlist_previous_index_addr(a)     S_get_invlist_previous_index_addr(aTHX_ a)
 #define grok_bslash_N(a,b,c,d,e,f,g)   S_grok_bslash_N(aTHX_ a,b,c,d,e,f,g)
 #define handle_regex_sets(a,b,c,d,e)   S_handle_regex_sets(aTHX_ a,b,c,d,e)
 #define study_chunk(a,b,c,d,e,f,g,h,i,j,k)     S_study_chunk(aTHX_ a,b,c,d,e,f,g,h,i,j,k)
 #  endif
 #  if defined(PERL_IN_REGCOMP_C) || defined(PERL_IN_REGEXEC_C) || defined(PERL_IN_UTF8_C)
-#define _get_invlist_len_addr(a)       S__get_invlist_len_addr(aTHX_ a)
 #define _get_swash_invlist(a)  Perl__get_swash_invlist(aTHX_ a)
 #define _invlist_contains_cp(a,b)      S__invlist_contains_cp(aTHX_ a,b)
 #define _invlist_contents(a)   Perl__invlist_contents(aTHX_ a)
 #define _invlist_len(a)                S__invlist_len(aTHX_ a)
 #define _invlist_search(a,b)   Perl__invlist_search(aTHX_ a,b)
 #define _swash_inversion_hash(a)       Perl__swash_inversion_hash(aTHX_ a)
+#define get_invlist_offset_addr(a)     S_get_invlist_offset_addr(aTHX_ a)
 #  endif
 #  if defined(PERL_IN_REGCOMP_C) || defined(PERL_IN_REGEXEC_C) || defined(PERL_IN_UTF8_C) || defined(PERL_IN_TOKE_C)
 #define _core_swash_init(a,b,c,d,e,f,g)        Perl__core_swash_init(aTHX_ a,b,c,d,e,f,g)
index a299645..470659b 100644 (file)
 #define ELEMENT_RANGE_MATCHES_INVLIST(i) (! ((i) & 1))
 #define PREV_RANGE_MATCHES_INVLIST(i) (! ELEMENT_RANGE_MATCHES_INVLIST(i))
 
-PERL_STATIC_INLINE STRLEN*
-S__get_invlist_len_addr(pTHX_ SV* invlist)
+/* This converts to/from our UVs to what the SV code is expecting: bytes. */
+#define TO_INTERNAL_SIZE(x) ((x) * sizeof(UV))
+#define FROM_INTERNAL_SIZE(x) ((x)/ sizeof(UV))
+
+PERL_STATIC_INLINE bool*
+S_get_invlist_offset_addr(pTHX_ SV* invlist)
 {
-    /* Return the address of the UV that contains the current number
-     * of used elements in the inversion list */
+    /* Return the address of the field that says whether the inversion list is
+     * offset (it contains 1) or not (contains 0) */
 
-    PERL_ARGS_ASSERT__GET_INVLIST_LEN_ADDR;
+    PERL_ARGS_ASSERT_GET_INVLIST_OFFSET_ADDR;
 
-    return &(((XINVLIST*) SvANY(invlist))->count);
+    return &(((XINVLIST*) SvANY(invlist))->is_offset);
 }
 
 PERL_STATIC_INLINE UV
@@ -32,7 +36,9 @@ S__invlist_len(pTHX_ SV* const invlist)
 
     PERL_ARGS_ASSERT__INVLIST_LEN;
 
-    return *_get_invlist_len_addr(invlist);
+    return (SvCUR(invlist) == 0)
+           ? 0
+           : FROM_INTERNAL_SIZE(SvCUR(invlist)) - *get_invlist_offset_addr(invlist);
 }
 
 PERL_STATIC_INLINE bool
@@ -47,4 +53,11 @@ S__invlist_contains_cp(pTHX_ SV* const invlist, const UV cp)
     return index >= 0 && ELEMENT_RANGE_MATCHES_INVLIST(index);
 }
 
+#   if defined(PERL_IN_UTF8_C) || defined(PERL_IN_REGEXEC_C)
+
+/* These symbols are only needed later in regcomp.c */
+#       undef TO_INTERNAL_SIZE
+#       undef FROM_INTERNAL_SIZE
+#   endif
+
 #endif
diff --git a/proto.h b/proto.h
index bbe0b3e..9a6f5dd 100644 (file)
--- a/proto.h
+++ b/proto.h
@@ -6509,12 +6509,6 @@ PERL_STATIC_INLINE STRLEN*       S_get_invlist_iter_addr(pTHX_ SV* invlist)
 #define PERL_ARGS_ASSERT_GET_INVLIST_ITER_ADDR \
        assert(invlist)
 
-PERL_STATIC_INLINE bool*       S_get_invlist_offset_addr(pTHX_ SV* invlist)
-                       __attribute__warn_unused_result__
-                       __attribute__nonnull__(pTHX_1);
-#define PERL_ARGS_ASSERT_GET_INVLIST_OFFSET_ADDR       \
-       assert(invlist)
-
 PERL_STATIC_INLINE IV* S_get_invlist_previous_index_addr(pTHX_ SV* invlist)
                        __attribute__warn_unused_result__
                        __attribute__nonnull__(pTHX_1);
@@ -6760,12 +6754,6 @@ STATIC I32       S_study_chunk(pTHX_ struct RExC_state_t *pRExC_state, regnode **scanp
 
 #endif
 #if defined(PERL_IN_REGCOMP_C) || defined(PERL_IN_REGEXEC_C) || defined(PERL_IN_UTF8_C)
-PERL_STATIC_INLINE STRLEN*     S__get_invlist_len_addr(pTHX_ SV* invlist)
-                       __attribute__warn_unused_result__
-                       __attribute__nonnull__(pTHX_1);
-#define PERL_ARGS_ASSERT__GET_INVLIST_LEN_ADDR \
-       assert(invlist)
-
 PERL_CALLCONV SV*      Perl__get_swash_invlist(pTHX_ SV* const swash)
                        __attribute__warn_unused_result__
                        __attribute__nonnull__(pTHX_1);
@@ -6802,6 +6790,12 @@ PERL_CALLCONV HV*        Perl__swash_inversion_hash(pTHX_ SV* const swash)
 #define PERL_ARGS_ASSERT__SWASH_INVERSION_HASH \
        assert(swash)
 
+PERL_STATIC_INLINE bool*       S_get_invlist_offset_addr(pTHX_ SV* invlist)
+                       __attribute__warn_unused_result__
+                       __attribute__nonnull__(pTHX_1);
+#define PERL_ARGS_ASSERT_GET_INVLIST_OFFSET_ADDR       \
+       assert(invlist)
+
 #endif
 #if defined(PERL_IN_REGCOMP_C) || defined(PERL_IN_REGEXEC_C) || defined(PERL_IN_UTF8_C) || defined(PERL_IN_TOKE_C)
 PERL_CALLCONV SV*      Perl__core_swash_init(pTHX_ const char* pkg, const char* name, SV* listsv, I32 minbits, I32 none, SV* invlist, U8* const flags_p)
index 2de7501..b40425f 100644 (file)
--- a/regcomp.c
+++ b/regcomp.c
@@ -7067,11 +7067,6 @@ S_reg_scan_name(pTHX_ RExC_state_t *pRExC_state, U32 flags)
 
 /* The header definitions are in F<inline_invlist.c> */
 
-/* This converts to/from our UVs to what the SV code is expecting: bytes. */
-#define TO_INTERNAL_SIZE(x) ((x) * sizeof(UV))
-#define FROM_INTERNAL_SIZE(x) ((x)/ sizeof(UV))
-
-
 PERL_STATIC_INLINE UV*
 S__invlist_array_init(pTHX_ SV* const invlist, const bool will_have_0)
 {
@@ -7089,7 +7084,7 @@ S__invlist_array_init(pTHX_ SV* const invlist, const bool will_have_0)
     PERL_ARGS_ASSERT__INVLIST_ARRAY_INIT;
 
     /* Must be empty */
-    assert(! *_get_invlist_len_addr(invlist));
+    assert(! _invlist_len(invlist));
 
     *zero_addr = 0;
 
@@ -7109,7 +7104,7 @@ S_invlist_array(pTHX_ SV* const invlist)
 
     /* Must not be empty.  If these fail, you probably didn't check for <len>
      * being non-zero before trying to get the array */
-    assert(*_get_invlist_len_addr(invlist));
+    assert(_invlist_len(invlist));
 
     /* The very first element always contains zero, The array begins either
      * there, or if the inversion list is offset, at the element after it.
@@ -7127,8 +7122,6 @@ S_invlist_set_len(pTHX_ SV* const invlist, const UV len, const bool offset)
 
     PERL_ARGS_ASSERT_INVLIST_SET_LEN;
 
-    *_get_invlist_len_addr(invlist) = len;
-
     SvCUR_set(invlist,
               (len == 0)
                ? 0
@@ -7184,17 +7177,6 @@ S_invlist_max(pTHX_ SV* const invlist)
            : FROM_INTERNAL_SIZE(SvLEN(invlist)) - 1;
 }
 
-PERL_STATIC_INLINE bool*
-S_get_invlist_offset_addr(pTHX_ SV* invlist)
-{
-    /* Return the address of the field that says whether the inversion list is
-     * offset (it contains 1) or not (contains 0) */
-
-    PERL_ARGS_ASSERT_GET_INVLIST_OFFSET_ADDR;
-
-    return &(((XINVLIST*) SvANY(invlist))->is_offset);
-}
-
 #ifndef PERL_IN_XSUB_RE
 SV*
 Perl__new_invlist(pTHX_ IV initial_size)
@@ -8070,27 +8052,17 @@ Perl__invlist_invert(pTHX_ SV* const invlist)
      * have a zero; removes it otherwise.  As described above, the data
      * structure is set up so that this is very efficient */
 
-    STRLEN* len_pos = _get_invlist_len_addr(invlist);
-
     PERL_ARGS_ASSERT__INVLIST_INVERT;
 
     assert(! invlist_is_iterating(invlist));
 
     /* The inverse of matching nothing is matching everything */
-    if (*len_pos == 0) {
+    if (_invlist_len(invlist) == 0) {
        _append_range_to_invlist(invlist, 0, UV_MAX);
        return;
     }
 
-    /* The exclusive or complents 0 to 1; and 1 to 0.  If the result is 1, the
-     * zero element was a 0, so it is being removed, so the length decrements
-     * by 1; and vice-versa.  SvCUR is unaffected */
-    if (*get_invlist_offset_addr(invlist) ^= 1) {
-       (*len_pos)--;
-    }
-    else {
-       (*len_pos)++;
-    }
+    *get_invlist_offset_addr(invlist) = ! *get_invlist_offset_addr(invlist);
 }
 
 void
diff --git a/sv.h b/sv.h
index a891ce9..7110b4c 100644 (file)
--- a/sv.h
+++ b/sv.h
@@ -532,7 +532,6 @@ struct xpvinvlist {
     _XPV_HEAD;
     IV          prev_index;
     STRLEN     iterator;
-    STRLEN     count;
     bool       is_offset;      /* */
 };