Make paren indexing uniform. (r=dmandelin, a=blocker, b=605754)
authorChris Leary <cdleary@mozilla.com>
Wed, 01 Dec 2010 16:34:10 -0800
changeset 58510 a93d62654d2d43b5176d431768dde5212772db46
parent 58509 03ff7e80545f6713625571dc0442027fe9e193bb
child 58511 be1f9b4e522e94d7db7488d52d8394b0dc883c56
push id17329
push userdmandelin@mozilla.com
push dateThu, 02 Dec 2010 19:57:29 +0000
treeherdermozilla-central@be1f9b4e522e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdmandelin, blocker
bugs605754
milestone2.0b8pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Make paren indexing uniform. (r=dmandelin, a=blocker, 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;