Make paren indexing uniform. (r=dmandelin, b=605754)
authorChris Leary <cdleary@mozilla.com>
Wed, 01 Dec 2010 16:34:10 -0800
changeset 58698 c0f4983e83d6d76b62d7c2a2869f721b3d0932e7
parent 58697 d3c713bb75e3831ccb401c89cf619c6c3829c61f
child 58699 bdc3aa93dc265b9745b987934593b398a41df881
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewersdmandelin
bugs605754
milestone2.0b8pre
Make paren indexing uniform. (r=dmandelin, b=605754)
js/src/jsregexp.cpp
js/src/jsregexp.h
js/src/jsregexpinlines.h
js/src/jsstr.cpp
--- a/js/src/jsregexp.cpp
+++ b/js/src/jsregexp.cpp
@@ -396,25 +396,25 @@ regexp_resolve(JSContext *cx, JSObject *
 DEFINE_STATIC_GETTER(static_input_getter,        return res->createPendingInput(cx, Valueify(vp)))
 DEFINE_STATIC_GETTER(static_multiline_getter,    *vp = BOOLEAN_TO_JSVAL(res->multiline());
                                                  return true)
 DEFINE_STATIC_GETTER(static_lastMatch_getter,    return res->createLastMatch(cx, Valueify(vp)))
 DEFINE_STATIC_GETTER(static_lastParen_getter,    return res->createLastParen(cx, Valueify(vp)))
 DEFINE_STATIC_GETTER(static_leftContext_getter,  return res->createLeftContext(cx, Valueify(vp)))
 DEFINE_STATIC_GETTER(static_rightContext_getter, return res->createRightContext(cx, Valueify(vp)))
 
-DEFINE_STATIC_GETTER(static_paren1_getter,       return res->createParen(cx, 0, Valueify(vp)))
-DEFINE_STATIC_GETTER(static_paren2_getter,       return res->createParen(cx, 1, Valueify(vp)))
-DEFINE_STATIC_GETTER(static_paren3_getter,       return res->createParen(cx, 2, Valueify(vp)))
-DEFINE_STATIC_GETTER(static_paren4_getter,       return res->createParen(cx, 3, Valueify(vp)))
-DEFINE_STATIC_GETTER(static_paren5_getter,       return res->createParen(cx, 4, Valueify(vp)))
-DEFINE_STATIC_GETTER(static_paren6_getter,       return res->createParen(cx, 5, Valueify(vp)))
-DEFINE_STATIC_GETTER(static_paren7_getter,       return res->createParen(cx, 6, Valueify(vp)))
-DEFINE_STATIC_GETTER(static_paren8_getter,       return res->createParen(cx, 7, Valueify(vp)))
-DEFINE_STATIC_GETTER(static_paren9_getter,       return res->createParen(cx, 8, Valueify(vp)))
+DEFINE_STATIC_GETTER(static_paren1_getter,       return res->createParen(cx, 1, Valueify(vp)))
+DEFINE_STATIC_GETTER(static_paren2_getter,       return res->createParen(cx, 2, Valueify(vp)))
+DEFINE_STATIC_GETTER(static_paren3_getter,       return res->createParen(cx, 3, Valueify(vp)))
+DEFINE_STATIC_GETTER(static_paren4_getter,       return res->createParen(cx, 4, Valueify(vp)))
+DEFINE_STATIC_GETTER(static_paren5_getter,       return res->createParen(cx, 5, Valueify(vp)))
+DEFINE_STATIC_GETTER(static_paren6_getter,       return res->createParen(cx, 6, Valueify(vp)))
+DEFINE_STATIC_GETTER(static_paren7_getter,       return res->createParen(cx, 7, Valueify(vp)))
+DEFINE_STATIC_GETTER(static_paren8_getter,       return res->createParen(cx, 8, Valueify(vp)))
+DEFINE_STATIC_GETTER(static_paren9_getter,       return res->createParen(cx, 9, Valueify(vp)))
 
 #define DEFINE_STATIC_SETTER(name, code)                                        \
     static JSBool                                                               \
     name(JSContext *cx, JSObject *obj, jsid id, jsval *vp)                      \
     {                                                                           \
         RegExpStatics *res = cx->regExpStatics();                               \
         code;                                                                   \
         return true;                                                            \
--- a/js/src/jsregexp.h
+++ b/js/src/jsregexp.h
@@ -65,26 +65,16 @@ class RegExpStatics
     /* The input last set on the statics. */
     JSString        *pendingInput;
     uintN           flags;
     RegExpStatics   *bufferLink;
     bool            copied;
 
     bool createDependent(JSContext *cx, size_t start, size_t end, Value *out) const;
 
-    size_t pairCount() const {
-        JS_ASSERT(matchPairs.length() % 2 == 0);
-        return matchPairs.length() / 2;
-    }
-
-    size_t pairCountCrash() const {
-        JS_CRASH_UNLESS(matchPairs.length() % 2 == 0);
-        return pairCount();
-    }
-
     void copyTo(RegExpStatics &dst) {
         dst.matchPairs.clear();
         /* 'save' has already reserved space in matchPairs */
         JS_ALWAYS_TRUE(dst.matchPairs.append(matchPairs));
         dst.matchPairsInput = matchPairsInput;
         dst.pendingInput = pendingInput;
         dst.flags = flags;
     }
@@ -134,25 +124,32 @@ class RegExpStatics
                 continue;
             int start = get(i, 0);
             int limit = get(i, 1);
             JS_ASSERT(mpiLen >= size_t(limit) && limit >= start && start >= 0);
         }
 #endif
     }
 
+    /* 
+     * Since the first pair indicates the whole match, the paren pair numbers have to be in the
+     * range [1, pairCount).
+     */
+    void checkParenNum(size_t pairNum) const {
+        JS_CRASH_UNLESS(1 <= pairNum);
+        JS_CRASH_UNLESS(pairNum < pairCount());
+    }
+
     bool pairIsPresent(size_t pairNum) const {
         return getCrash(pairNum, 0) >= 0;
     }
 
     /* Precondition: paren is present. */
-    size_t getParenLength(size_t parenNum) const {
-        size_t pairNum = parenNum + 1;
-        if (pairCountCrash() <= pairNum)
-            return 0;
+    size_t getParenLength(size_t pairNum) const {
+        checkParenNum(pairNum);
         JS_CRASH_UNLESS(pairIsPresent(pairNum));
         return getCrash(pairNum, 1) - getCrash(pairNum, 0);
     }
 
     int get(size_t pairNum, bool which) const {
         JS_ASSERT(pairNum < pairCount());
         return matchPairs[2 * pairNum + which];
     }
@@ -226,16 +223,41 @@ class RegExpStatics
 
     void setPendingInput(JSString *newInput) {
         aboutToWrite();
         pendingInput = newInput;
     }
 
     /* Accessors. */
 
+    /*
+     * When there is a match present, the pairCount is at least 1 for the whole
+     * match. There is one additional pair per parenthesis.
+     *
+     * Getting a parenCount requires there to be a match result as a precondition.
+     */
+
+  private:
+    size_t pairCount() const {
+        JS_ASSERT(matchPairs.length() % 2 == 0);
+        return matchPairs.length() / 2;
+    }
+
+    size_t pairCountCrash() const {
+        JS_CRASH_UNLESS(matchPairs.length() % 2 == 0);
+        return pairCount();
+    }
+
+  public:
+    size_t parenCount() const {
+        size_t pc = pairCount();
+        JS_CRASH_UNLESS(pc);
+        return pc - 1;
+    }
+
     JSString *getPendingInput() const { return pendingInput; }
     uintN getFlags() const { return flags; }
     bool multiline() const { return flags & JSREG_MULTILINE; }
 
     size_t matchStart() const {
         int start = get(0, 0);
         JS_ASSERT(start >= 0);
         return size_t(start);
@@ -249,45 +271,44 @@ class RegExpStatics
 
     /* Returns whether results for a non-empty match are present. */
     bool matched() const {
         JS_ASSERT(pairCount() > 0);
         JS_ASSERT_IF(get(0, 1) == -1, get(1, 1) == -1);
         return get(0, 1) - get(0, 0) > 0;
     }
 
-    size_t getParenCount() const {
-        /* The first pair is the whole match. */
-        JS_ASSERT(pairCount() > 0);
-        return pairCount() - 1;
-    }
-
     void mark(JSTracer *trc) const {
         if (pendingInput)
             JS_CALL_STRING_TRACER(trc, pendingInput, "res->pendingInput");
         if (matchPairsInput)
             JS_CALL_STRING_TRACER(trc, matchPairsInput, "res->matchPairsInput");
     }
 
     /* Value creators. */
 
     bool createPendingInput(JSContext *cx, Value *out) const;
     bool createLastMatch(JSContext *cx, Value *out) const { return makeMatch(cx, 0, 0, out); }
     bool createLastParen(JSContext *cx, Value *out) const;
     bool createLeftContext(JSContext *cx, Value *out) const;
     bool createRightContext(JSContext *cx, Value *out) const;
 
-    bool createParen(JSContext *cx, size_t parenNum, Value *out) const {
-        return makeMatch(cx, (parenNum + 1) * 2, parenNum + 1, out);
+    /* @param pairNum   Any number >= 1. */
+    bool createParen(JSContext *cx, size_t pairNum, Value *out) const {
+        JS_CRASH_UNLESS(pairNum >= 1);
+        if (pairNum >= pairCount()) {
+            out->setString(cx->runtime->emptyString);
+            return true;
+        }
+        return makeMatch(cx, pairNum * 2, pairNum, out);
     }
 
     /* Substring creators. */
 
-    /* @param num   Zero-indexed paren number (i.e. $1 is 0). */
-    void getParen(size_t num, JSSubString *out) const;
+    void getParen(size_t pairNum, JSSubString *out) const;
     void getLastMatch(JSSubString *out) const;
     void getLastParen(JSSubString *out) const;
     void getLeftContext(JSSubString *out) const;
     void getRightContext(JSSubString *out) const;
 };
 
 class PreserveRegExpStatics
 {
--- a/js/src/jsregexpinlines.h
+++ b/js/src/jsregexpinlines.h
@@ -239,17 +239,17 @@ RegExp::checkMatchPairs(JSString *input,
 }
 
 inline JSObject *
 RegExp::createResult(JSContext *cx, JSString *input, int *buf, size_t matchItemCount)
 {
     /*
      * Create the result array for a match. Array contents:
      *  0:              matched string
-     *  1..parenCount:  paren matches
+     *  1..pairCount-1: paren matches
      */
     JSObject *array = js_NewSlowArrayObject(cx);
     if (!array)
         return NULL;
 
     RegExpMatchBuilder builder(cx, array);
     for (size_t i = 0; i < matchItemCount; i += 2) {
         int start = buf[i];
@@ -613,25 +613,25 @@ RegExpStatics::createRightContext(JSCont
     if (matchPairs[1] < 0) {
         *out = UndefinedValue();
         return true;
     }
     return createDependent(cx, matchPairs[1], matchPairsInput->length(), out);
 }
 
 inline void
-RegExpStatics::getParen(size_t num, JSSubString *out) const
+RegExpStatics::getParen(size_t pairNum, JSSubString *out) const
 {
-    size_t pairNum = num + 1;
+    checkParenNum(pairNum);
     if (!pairIsPresent(pairNum)) {
         *out = js_EmptySubString;
         return;
     }
     out->chars = matchPairsInput->chars() + getCrash(pairNum, 0);
-    out->length = getParenLength(num);
+    out->length = getParenLength(pairNum);
 }
 
 inline void
 RegExpStatics::getLastMatch(JSSubString *out) const
 {
     if (!pairCountCrash()) {
         *out = js_EmptySubString;
         return;
@@ -640,22 +640,23 @@ RegExpStatics::getLastMatch(JSSubString 
     out->chars = matchPairsInput->chars() + getCrash(0, 0);
     JS_CRASH_UNLESS(getCrash(0, 1) >= getCrash(0, 0));
     out->length = get(0, 1) - get(0, 0);
 }
 
 inline void
 RegExpStatics::getLastParen(JSSubString *out) const
 {
-    size_t parenCount = getParenCount();
-    if (!parenCount) {
+    size_t pairCount = pairCountCrash();
+    /* Note: the first pair is the whole match. */
+    if (pairCount <= 1) {
         *out = js_EmptySubString;
         return;
     }
-    getParen(parenCount - 1, out);
+    getParen(pairCount - 1, out);
 }
 
 inline void
 RegExpStatics::getLeftContext(JSSubString *out) const
 {
     if (!pairCountCrash()) {
         *out = js_EmptySubString;
         return;
--- a/js/src/jsstr.cpp
+++ b/js/src/jsstr.cpp
@@ -1998,37 +1998,38 @@ InterpretDollar(JSContext *cx, RegExpSta
     if (dp + 1 >= ep)
         return false;
 
     /* Interpret all Perl match-induced dollar variables. */
     jschar dc = dp[1];
     if (JS7_ISDEC(dc)) {
         /* ECMA-262 Edition 3: 1-9 or 01-99 */
         uintN num = JS7_UNDEC(dc);
-        if (num > res->getParenCount())
+        if (num > res->parenCount())
             return false;
 
         jschar *cp = dp + 2;
         if (cp < ep && (dc = *cp, JS7_ISDEC(dc))) {
             uintN tmp = 10 * num + JS7_UNDEC(dc);
-            if (tmp <= res->getParenCount()) {
+            if (tmp <= res->parenCount()) {
                 cp++;
                 num = tmp;
             }
         }
         if (num == 0)
             return false;
 
-        /* Adjust num from 1 $n-origin to 0 array-index-origin. */
-        num--;
         *skip = cp - dp;
-        if (num < res->getParenCount())
-            res->getParen(num, out);
-        else
-            *out = js_EmptySubString;
+
+        JS_CRASH_UNLESS(num <= res->parenCount());
+        /* 
+         * Note: we index to get the paren with the (1-indexed) pair
+         * number, as opposed to a (0-indexed) paren number.
+         */
+        res->getParen(num, out);
         return true;
     }
 
     *skip = 2;
     switch (dc) {
       case '$':
         rdata.dollarStr.chars = dp;
         rdata.dollarStr.length = 1;
@@ -2115,17 +2116,17 @@ FindReplaceLength(JSContext *cx, RegExpS
         /*
          * In the lambda case, not only do we find the replacement string's
          * length, we compute repstr and return it via rdata for use within
          * DoReplace.  The lambda is called with arguments ($&, $1, $2, ...,
          * index, input), i.e., all the properties of a regexp match array.
          * For $&, etc., we must create string jsvals from cx->regExpStatics.
          * We grab up stack space to keep the newborn strings GC-rooted.
          */
-        uintN p = res->getParenCount();
+        uintN p = res->parenCount();
         uintN argc = 1 + p + 2;
 
         InvokeSessionGuard &session = rdata.session;
         if (!session.started()) {
             Value lambdav = ObjectValue(*lambda);
             if (!session.start(cx, lambdav, UndefinedValue(), argc))
                 return false;
         }
@@ -2134,18 +2135,18 @@ FindReplaceLength(JSContext *cx, RegExpS
         if (!staticsGuard.init(cx))
             return false;
 
         /* Push $&, $1, $2, ... */
         uintN argi = 0;
         if (!res->createLastMatch(cx, &session[argi++]))
             return false;
 
-        for (size_t i = 0; i < res->getParenCount(); ++i) {
-            if (!res->createParen(cx, i, &session[argi++]))
+        for (size_t i = 0; i < res->parenCount(); ++i) {
+            if (!res->createParen(cx, i + 1, &session[argi++]))
                 return false;
         }
 
         /* Push match index and input string. */
         session[argi++].setInt32(res->matchStart());
         session[argi].setString(rdata.str);
 
         if (!session.invoke(cx))
@@ -2753,21 +2754,21 @@ str_split(JSContext *cx, uintN argc, Val
         len++;
 
         /*
          * Imitate perl's feature of including parenthesized substrings that
          * matched part of the delimiter in the new array, after the split
          * substring that was delimited.
          */
         if (re && sep->chars) {
-            for (uintN num = 0; num < res->getParenCount(); num++) {
+            for (uintN num = 0; num < res->parenCount(); num++) {
                 if (limited && len >= limit)
                     break;
                 JSSubString parsub;
-                res->getParen(num, &parsub);
+                res->getParen(num + 1, &parsub);
                 sub = js_NewStringCopyN(cx, parsub.chars, parsub.length);
                 if (!sub || !splits.append(StringValue(sub)))
                     return false;
                 len++;
             }
             sep->chars = NULL;
         }
         i = j + sep->length;