Bug 1215430 - Inline the guts of the shared method implementing |new RegExp(...)| and |RegExp.prototype.compile| into each separate method, for clarity. r=efaust
authorJeff Walden <jwalden@mit.edu>
Fri, 16 Oct 2015 00:29:38 -0700
changeset 304310 8009ed0eb3a6ddd265fc236c0478f73130a9f07f
parent 304309 9a1e89df904b953006be78880dbe99ad262c5d6a
child 304311 bc949d6e3aaab935712bf73430cadb7bc6d7fe26
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersefaust
bugs1215430
milestone44.0a1
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
Bug 1215430 - Inline the guts of the shared method implementing |new RegExp(...)| and |RegExp.prototype.compile| into each separate method, for clarity. r=efaust
js/src/builtin/RegExp.cpp
js/src/builtin/RegExp.h
--- a/js/src/builtin/RegExp.cpp
+++ b/js/src/builtin/RegExp.cpp
@@ -187,152 +187,16 @@ RegExpInitialize(JSContext* cx, RegExpOb
     if (!reobj)
         return false;
 
     /* Step 16. */
     result.set(reobj);
     return true;
 }
 
-/*
- * ES6 draft rc4 21.2.3.1 steps 5-10.
- * ES6 draft rc4 B.2.5.1 steps 3-5.
- * Compile a new |RegExpShared| for the |RegExpObject|.
- */
-static bool
-CompileRegExpObject(JSContext* cx, RegExpObjectBuilder& builder, const CallArgs& args,
-                    RegExpCreationMode creationMode, bool patternIsRegExp=false)
-{
-    if (args.length() == 0) {
-        /*
-         * 21.2.3.1 step 10.
-         * B.2.5.1 step 5.
-         */
-        RegExpStatics* res = cx->global()->getRegExpStatics(cx);
-        if (!res)
-            return false;
-
-        RootedAtom empty(cx, cx->runtime()->emptyString);
-        RegExpObject* reobj = builder.build(empty, res->getFlags());
-        if (!reobj)
-            return false;
-
-        args.rval().setObject(*reobj);
-        return true;
-    }
-
-    RootedValue patternValue(cx, args.get(0));
-
-    /*
-     * 21.2.3.1 step 5
-     * B.2.5.1 step 3.
-     */
-    ESClassValue cls;
-    if (!GetClassOfValue(cx, patternValue, &cls))
-        return false;
-    if (cls == ESClass_RegExp) {
-        /*
-         * B.2.5.1 step 3.a.
-         */
-        if (args.hasDefined(1) && creationMode == CreateForCompile) {
-            JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_NEWREGEXP_FLAGGED);
-            return false;
-        }
-
-        /*
-         * Beware, patternObj may be a (transparent) proxy to a RegExp, so only
-         * use generic (proxyable) operations on patternObj that do not assume
-         * patternObj.is<RegExpObject>().
-         */
-        RootedObject patternObj(cx, &patternValue.toObject());
-
-        RootedAtom sourceAtom(cx);
-        RegExpFlag flags;
-        {
-            /*
-             * 21.2.3.1 step 5.a.
-             * B.2.5.1 step 3.a.
-             * Extract the 'source' from patternObj; do not reuse the
-             * RegExpShared since it may be from a different compartment.
-             */
-            RegExpGuard g(cx);
-            if (!RegExpToShared(cx, patternObj, &g))
-                return false;
-            sourceAtom = g->getSource();
-
-            if (args.hasDefined(1)) {
-                /* 21.2.3.1 step 5.c. */
-                flags = RegExpFlag(0);
-                RootedString flagStr(cx, ToString<CanGC>(cx, args[1]));
-                if (!flagStr)
-                    return false;
-                if (!ParseRegExpFlags(cx, flagStr, &flags))
-                    return false;
-            } else {
-                /*
-                 * 21.2.3.1 step 5.b.
-                 * B.2.5.1 step 3.c.
-                 */
-                flags = g->getFlags();
-            }
-        }
-
-        /*
-         * 21.2.3.1 steps 8-10.
-         * B.2.5.1 step 5.
-         */
-        RegExpObject* reobj = builder.build(sourceAtom, flags);
-        if (!reobj)
-            return false;
-
-        args.rval().setObject(*reobj);
-        return true;
-    }
-
-    RootedValue P(cx);
-    RootedValue F(cx);
-    /* 21.2.3.1 step 6. */
-    if (patternIsRegExp) {
-        MOZ_ASSERT(creationMode == CreateForConstruct);
-        RootedObject patternObj(cx, &patternValue.toObject());
-
-        /* 21.2.3.1 steps 6.a-b. */
-        if (!GetProperty(cx, patternObj, patternObj, cx->names().source, &P))
-            return false;
-
-        /* 21.2.3.1 step 6.c. */
-        if (!args.hasDefined(1)) {
-            /* 21.2.3.1 steps 6.c.i-ii. */
-            if (!GetProperty(cx, patternObj, patternObj, cx->names().flags, &F))
-                return false;
-        } else {
-            /* 21.2.3.1 steps 6.d. */
-            F = args[1];
-        }
-    } else {
-        /*
-         * 21.2.3.1 steps 7.a-b.
-         * B.2.5.1 steps 4.a-b.
-         */
-        P = patternValue;
-        F = args.get(1);
-    }
-
-    /*
-     * 21.2.3.1 steps 8-10.
-     * B.2.5.1 step 5.
-     */
-    RootedObject reobj(cx);
-    if (!RegExpInitialize(cx, builder, P, F, UseRegExpStatics, &reobj))
-        return false;
-
-    args.rval().setObject(*reobj);
-    return true;
-}
-
 MOZ_ALWAYS_INLINE bool
 IsRegExpObject(HandleValue v)
 {
     return v.isObject() && v.toObject().is<RegExpObject>();
 }
 
 /* ES6 draft rc3 7.2.8. */
 bool
@@ -361,69 +225,195 @@ js::IsRegExp(JSContext* cx, HandleValue 
     ESClassValue cls;
     if (!GetClassOfValue(cx, value, &cls))
         return false;
 
     *result = cls == ESClass_RegExp;
     return true;
 }
 
-/* ES6 draft rc4 B.2.5.1. */
+/* ES6 B.2.5.1. */
 MOZ_ALWAYS_INLINE bool
 regexp_compile_impl(JSContext* cx, const CallArgs& args)
 {
     MOZ_ASSERT(IsRegExpObject(args.thisv()));
 
-    /* Steps 3-5. */
     RegExpObjectBuilder builder(cx, &args.thisv().toObject().as<RegExpObject>());
-    return CompileRegExpObject(cx, builder, args, CreateForCompile);
+
+    // Step 3.
+    RootedValue patternValue(cx, args.get(0));
+    ESClassValue cls;
+    if (!GetClassOfValue(cx, patternValue, &cls))
+        return false;
+    if (cls == ESClass_RegExp) {
+        // Step 3a.
+        if (args.hasDefined(1)) {
+            JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_NEWREGEXP_FLAGGED);
+            return false;
+        }
+
+        // Beware!  |patternObj| might be a proxy into another compartment, so
+        // don't assume |patternObj.is<RegExpObject>()|.  For the same reason,
+        // don't reuse the RegExpShared below.
+        RootedObject patternObj(cx, &patternValue.toObject());
+
+        RootedAtom sourceAtom(cx);
+        RegExpFlag flags;
+        {
+            // Step 3b.
+            RegExpGuard g(cx);
+            if (!RegExpToShared(cx, patternObj, &g))
+                return false;
+
+            sourceAtom = g->getSource();
+            flags = g->getFlags();
+        }
+
+        // Step 5.
+        RegExpObject* reobj = builder.build(sourceAtom, flags);
+        if (!reobj)
+            return false;
+
+        args.rval().setObject(*reobj);
+        return true;
+    }
+
+    // Step 4.
+    RootedValue P(cx, patternValue);
+    RootedValue F(cx, args.get(1));
+
+    // Step 5.
+    RootedObject reobj(cx);
+    if (!RegExpInitialize(cx, builder, P, F, UseRegExpStatics, &reobj))
+        return false;
+
+    args.rval().setObject(*reobj);
+    return true;
 }
 
 static bool
 regexp_compile(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
     /* Steps 1-2. */
     return CallNonGenericMethod<IsRegExpObject, regexp_compile_impl>(cx, args);
 }
 
-/* ES6 draft rc4 21.2.3.1. */
+/* ES6 21.2.3.1. */
 bool
 js::regexp_construct(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
-    /* Steps 1-2. */
+    // Steps 1-2.
     bool patternIsRegExp;
     if (!IsRegExp(cx, args.get(0), &patternIsRegExp))
         return false;
 
-    /* Step 4. */
-    if (!args.isConstructing()) {
-        /* Step 4.b. */
+    if (args.isConstructing()) {
+        // XXX Step 3!
+    } else {
+        // XXX Step 4a
+
+        // Step 4b.
         if (patternIsRegExp && !args.hasDefined(1)) {
             RootedObject patternObj(cx, &args[0].toObject());
 
-            /* Steps 4.b.i-ii. */
+            // Steps 4b.i-ii.
             RootedValue patternConstructor(cx);
             if (!GetProperty(cx, patternObj, patternObj, cx->names().constructor, &patternConstructor))
                 return false;
 
-            /* Step 4.b.iii. */
+            // Step 4b.iii.
             if (patternConstructor.isObject() && patternConstructor.toObject() == args.callee()) {
                 args.rval().set(args[0]);
                 return true;
             }
         }
     }
 
-    /* Steps 5-10. */
     RegExpObjectBuilder builder(cx);
-    return CompileRegExpObject(cx, builder, args, CreateForConstruct, patternIsRegExp);
+    RootedValue patternValue(cx, args.get(0));
+
+    // Step 5.
+    ESClassValue cls;
+    if (!GetClassOfValue(cx, patternValue, &cls))
+        return false;
+    if (cls == ESClass_RegExp) {
+        // Beware!  |patternObj| might be a proxy into another compartment, so
+        // don't assume |patternObj.is<RegExpObject>()|.  For the same reason,
+        // don't reuse the RegExpShared below.
+        RootedObject patternObj(cx, &patternValue.toObject());
+
+        RootedAtom sourceAtom(cx);
+        RegExpFlag flags;
+        {
+            // Step 5.a.
+            RegExpGuard g(cx);
+            if (!RegExpToShared(cx, patternObj, &g))
+                return false;
+            sourceAtom = g->getSource();
+
+            if (!args.hasDefined(1)) {
+                // Step 5b.
+                flags = g->getFlags();
+            } else {
+                // Step 5c.
+                // XXX We shouldn't be converting to string yet!  This must
+                //     come *after* the .constructor access in step 8.
+                flags = RegExpFlag(0);
+                RootedString flagStr(cx, ToString<CanGC>(cx, args[1]));
+                if (!flagStr)
+                    return false;
+                if (!ParseRegExpFlags(cx, flagStr, &flags))
+                    return false;
+            }
+        }
+
+        // Steps 8-10.
+        // XXX Note bug in step 5c, with respect to step 8.
+        RegExpObject* reobj = builder.build(sourceAtom, flags);
+        if (!reobj)
+            return false;
+
+        args.rval().setObject(*reobj);
+        return true;
+    }
+
+    RootedValue P(cx);
+    RootedValue F(cx);
+
+    // Step 6.
+    if (patternIsRegExp) {
+        RootedObject patternObj(cx, &patternValue.toObject());
+
+        // Steps 6a-b.
+        if (!GetProperty(cx, patternObj, patternObj, cx->names().source, &P))
+            return false;
+
+        // Steps 6c-d.
+        F = args.get(1);
+        if (F.isUndefined()) {
+            if (!GetProperty(cx, patternObj, patternObj, cx->names().flags, &F))
+                return false;
+        }
+    } else {
+        // Steps 7a-b.
+        P = patternValue;
+        F = args.get(1);
+    }
+
+    // Steps 8-10.
+    RootedObject reobj(cx);
+    if (!RegExpInitialize(cx, builder, P, F, UseRegExpStatics, &reobj))
+        return false;
+
+    args.rval().setObject(*reobj);
+    return true;
 }
 
 bool
 js::regexp_construct_no_statics(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
     MOZ_ASSERT(args.length() == 1 || args.length() == 2);
--- a/js/src/builtin/RegExp.h
+++ b/js/src/builtin/RegExp.h
@@ -21,19 +21,16 @@ InitRegExpClass(JSContext* cx, HandleObj
 
 // Whether RegExp statics should be updated with the input and results of a
 // regular expression execution.
 enum RegExpStaticsUpdate { UpdateRegExpStatics, DontUpdateRegExpStatics };
 
 // Whether RegExp statics should be used to create a RegExp instance.
 enum RegExpStaticsUse { UseRegExpStatics, DontUseRegExpStatics };
 
-// This enum is used to indicate whether 'CompileRegExpObject' is called from 'regexp_compile'.
-enum RegExpCreationMode { CreateForCompile, CreateForConstruct };
-
 RegExpRunStatus
 ExecuteRegExp(JSContext* cx, HandleObject regexp, HandleString string,
               MatchPairs* matches, RegExpStaticsUpdate staticsUpdate);
 
 /*
  * Legacy behavior of ExecuteRegExp(), which is baked into the JSAPI.
  *
  * |res| may be nullptr if the RegExpStatics are not to be updated.