This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
regcomp.c: Move inversion list hdr field to SV hdr
authorKarl Williamson <public@khwilliamson.com>
Wed, 13 Feb 2013 23:33:09 +0000 (16:33 -0700)
committerKarl Williamson <public@khwilliamson.com>
Thu, 4 Jul 2013 01:21:18 +0000 (19:21 -0600)
This moves the final field that can vary from the inversion list data
structure into the header of the SV that contains it.  With this commit,
the body of an inversion list is now const.

The field is converted to a U8, to correspond with the header field in
the SV type that we currently use to hold inversion lists.

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

index 3ec96bd..53413b1 100644 (file)
--- a/embed.fnc
+++ b/embed.fnc
@@ -1428,7 +1428,7 @@ 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   |UV*|get_invlist_offset_addr|NN SV* invlist
+EiMR   |U8*    |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
 EiMR   |IV*    |get_invlist_previous_index_addr|NN SV* invlist
index 21d6282..936a298 100644 (file)
@@ -29,7 +29,7 @@
 /* For safety, when adding new elements, remember to #undef them at the end of
  * the inversion list code section */
 
-#define HEADER_LENGTH (INVLIST_OFFSET_OFFSET + 2) /* includes 1 for the constant
+#define HEADER_LENGTH (INVLIST_OFFSET_OFFSET + 1) /* includes 1 for the constant
                                                    0 element */
 
 /* An element is in an inversion list iff its index is even numbered: 0, 2, 4,
diff --git a/proto.h b/proto.h
index fc4a9a0..744a2b7 100644 (file)
--- a/proto.h
+++ b/proto.h
@@ -6486,7 +6486,7 @@ 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 UV* S_get_invlist_offset_addr(pTHX_ SV* invlist)
+PERL_STATIC_INLINE U8* S_get_invlist_offset_addr(pTHX_ SV* invlist)
                        __attribute__warn_unused_result__
                        __attribute__nonnull__(pTHX_1);
 #define PERL_ARGS_ASSERT_GET_INVLIST_OFFSET_ADDR       \
index 9ff7b3f..5726a46 100644 (file)
--- a/regcomp.c
+++ b/regcomp.c
@@ -7025,10 +7025,9 @@ S_reg_scan_name(pTHX_ RExC_state_t *pRExC_state, U32 flags)
 
 /* This section of code defines the inversion list object and its methods.  The
  * interfaces are highly subject to change, so as much as possible is static to
- * this file.  An inversion list is here implemented as a malloc'd C UV array
- * with some added info that is placed as UVs at the beginning in a header
- * portion.  Currently it is a SVt_PVLV, with some of the header fields from
- * that repurposed for uses here.
+ * this file.  An inversion list is here implemented as a malloc'd C UV array.
+ * Currently it is a SVt_PVLV, with some of the header fields from that
+ * repurposed for uses here.
  *
  * An inversion list for Unicode is an array of code points, sorted by ordinal
  * number.  The zeroth element is the first code point in the list.  The 1th
@@ -7068,8 +7067,13 @@ S_reg_scan_name(pTHX_ RExC_state_t *pRExC_state, U32 flags)
  * should eventually be made public */
 
 /* The header definitions are in F<inline_invlist.c> */
-#define TO_INTERNAL_SIZE(x) (((x) + HEADER_LENGTH) * sizeof(UV))
-#define FROM_INTERNAL_SIZE(x) (((x)/ sizeof(UV)) - HEADER_LENGTH)
+
+/* This converts to/from our UVs to what the SV code is expecting: bytes.  The
+ * first element is always a 0, which may or may not currently be in the list.
+ * Just assume the worst case, that it isn't, and so the length of the list
+ * passed in has to add 1 to account for it */
+#define TO_INTERNAL_SIZE(x) (((x) + 1) * sizeof(UV))
+#define FROM_INTERNAL_SIZE(x) (((x)/ sizeof(UV)) - 1)
 
 #define INVLIST_INITIAL_LEN 10
 
@@ -7081,20 +7085,22 @@ S__invlist_array_init(pTHX_ SV* const invlist, const bool will_have_0)
      * array begins depends on whether the list has the code point U+0000 in it
      * or not.  The other parameter tells it whether the code that follows this
      * call is about to put a 0 in the inversion list or not.  The first
-     * element is either the final part of the header reserved for 0, if TRUE,
-     * or the first element of the non-heading part, if FALSE */
+     * element is either the element reserved for 0, if TRUE, or the element
+     * after it, if FALSE */
 
-    UV* offset = get_invlist_offset_addr(invlist);
+    U8* offset = get_invlist_offset_addr(invlist);
+    UV* zero_addr = (UV *) SvPVX(invlist);
 
     PERL_ARGS_ASSERT__INVLIST_ARRAY_INIT;
 
     /* Must be empty */
     assert(! *_get_invlist_len_addr(invlist));
 
+    *zero_addr = 0;
+
     /* 1^1 = 0; 1^0 = 1 */
     *offset = 1 ^ will_have_0;
-    *(offset + 1) = 0;
-    return (UV *) (1 + offset + *offset);
+    return zero_addr + *offset;
 }
 
 PERL_STATIC_INLINE UV*
@@ -7112,13 +7118,12 @@ S_invlist_array(pTHX_ SV* const invlist)
     assert(*get_invlist_offset_addr(invlist) == 0
           || *get_invlist_offset_addr(invlist) == 1);
 
-    /* The array begins either at the header element reserved for zero or the
-     * element after that.  The reserved element is 1 past the offset_addr
-     * element; the latter contains 0 or 1 to indicate how much additionally to
-     * add */
-    assert(0 == *(1 + get_invlist_offset_addr(invlist)));
-    return (UV *) (1 + get_invlist_offset_addr(invlist)
-                  + *get_invlist_offset_addr(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.
+     * The offset header field determines which; it contains 0 or 1 to indicate
+     * how much additionally to add */
+    assert(0 == *(SvPVX(invlist)));
+    return ((UV *) SvPVX(invlist) + *get_invlist_offset_addr(invlist));
 }
 
 PERL_STATIC_INLINE void
@@ -7182,15 +7187,15 @@ S_invlist_max(pTHX_ SV* const invlist)
            : FROM_INTERNAL_SIZE(SvLEN(invlist));
 }
 
-PERL_STATIC_INLINE UV*
+PERL_STATIC_INLINE U8*
 S_get_invlist_offset_addr(pTHX_ SV* invlist)
 {
-    /* Return the address of the UV that says whether the inversion list is
+    /* 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 (UV *) (SvPVX(invlist) + (INVLIST_OFFSET_OFFSET * sizeof (UV)));
+    return (U8*) &(LvFLAGS(invlist));
 }
 
 #ifndef PERL_IN_XSUB_RE
@@ -7203,7 +7208,7 @@ Perl__new_invlist(pTHX_ IV initial_size)
      * system default is used instead */
 
     SV* new_list;
-    UV* offset_addr;
+    U8* offset_addr;
 
     if (initial_size < 0) {
        initial_size = INVLIST_INITIAL_LEN;
@@ -7219,13 +7224,13 @@ Perl__new_invlist(pTHX_ IV initial_size)
     *get_invlist_iter_addr(new_list) = (STRLEN) UV_MAX;
 
     /* This should force a segfault if a method doesn't initialize this
-     * properly */
+     * properly.  XXX Although now that the max is currently just 255, it might
+     * not segfault. */
     offset_addr = get_invlist_offset_addr(new_list);
-    *offset_addr = (unsigned) UV_MAX;
-    *(offset_addr + 1) = 0;
+    *offset_addr = (U8) UV_MAX;
 
     *get_invlist_previous_index_addr(new_list) = 0;
-#if HEADER_LENGTH != 4
+#if HEADER_LENGTH != 3
 #   error Need to regenerate INVLIST_VERSION_ID by running perl -E 'say int(rand 2**31-1)', and then changing the #if to the new length
 #endif
 
@@ -7242,18 +7247,32 @@ S__new_invlist_C_array(pTHX_ UV* list)
      * should not be used in the wrong hands */
 
     SV* invlist = newSV_type(SVt_PVLV);
+    STRLEN length = (STRLEN) list[INVLIST_LEN_OFFSET];
+    U8 offset = (U8) list[INVLIST_OFFSET_OFFSET];
 
     PERL_ARGS_ASSERT__NEW_INVLIST_C_ARRAY;
 
-    SvPV_set(invlist, (char *) list);
-    SvLEN_set(invlist, 0);  /* Means we own the contents, and the system
-                              shouldn't touch it */
-    SvCUR_set(invlist, TO_INTERNAL_SIZE(_invlist_len(invlist)));
-
     if (list[INVLIST_VERSION_ID_OFFSET] != INVLIST_VERSION_ID) {
         Perl_croak(aTHX_ "panic: Incorrect version for previously generated inversion list");
     }
-    invlist_set_len(invlist, list[INVLIST_LEN_OFFSET]);
+
+    /* The generated array passed in includes header elements that aren't part
+     * of the list proper, so start it just after them */
+    SvPV_set(invlist, (char *) (list + HEADER_LENGTH));
+
+    SvLEN_set(invlist, 0);  /* Means we own the contents, and the system
+                              shouldn't touch it */
+    /* The 'length' passed to us is the number of elements in the inversion
+     * list.  If the list doesn't include the always present 0 element, the
+     * length should be adjusted upwards to include it.  The TO_INTERNAL_SIZE
+     * macro returns a worst case scenario, always making that adjustment, even
+     * if not needed.  To get the precise size, then, we have to subtract 1
+     * when that adjustment wasn't needed.  That happens when the offset was 0;
+     * the exclusive-or flips the 0 to 1, and vice versa */
+    SvCUR_set(invlist, TO_INTERNAL_SIZE(length - (offset ^ 1)));
+
+    invlist_set_len(invlist, length);
+    *(get_invlist_offset_addr(invlist)) = offset;
     invlist_set_previous_index(invlist, 0);
 
     /* Initialize the iteration pointer. */
@@ -8123,6 +8142,8 @@ S_invlist_clone(pTHX_ SV* const invlist)
 
     SvCUR_set(new_invlist, length); /* This isn't done automatically */
     invlist_set_len(new_invlist, _invlist_len(invlist));
+    *(get_invlist_offset_addr(new_invlist))
+                                = *(get_invlist_offset_addr(invlist));
     Copy(SvPVX(invlist), SvPVX(new_invlist), length, char);
 
     return new_invlist;