Bug 1055472 - Part 8: Make the RegExp constructor properly subclassable. (r=Waldo)

js/src/builtin/RegExp.cpp

@@ -307,11 +307,11 @@ js::regexp_construct(JSContext* cx, unsigned argc, Value* vp)
     if (!IsRegExp(cx, args.get(0), &patternIsRegExp))
         return false;
 
-    if (args.isConstructing()) {
-        // XXX Step 3!
-    } else {
-        // XXX Step 4a
 
+    // We can delay step 3 and step 4a until later, during
+    // GetPrototypeFromCallableConstructor calls. Accessing the new.target
+    // and the callee from the stack is unobservable.
+    if (!args.isConstructing()) {
         // Step 4b.
         if (patternIsRegExp && !args.hasDefined(1)) {
             RootedObject patternObj(cx, &args[0].toObject());
@@ -341,6 +341,7 @@ js::regexp_construct(JSContext* cx, unsigned argc, Value* vp)
         // don't reuse the RegExpShared below.
         RootedObject patternObj(cx, &patternValue.toObject());
 
+        // Step 5
         RootedAtom sourceAtom(cx);
         RegExpFlag flags;
         {
@@ -353,26 +354,29 @@ js::regexp_construct(JSContext* cx, unsigned argc, Value* vp)
             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-9.
-        // XXX Note bug in step 5c, with respect to step 8.
-        Rooted<RegExpObject*> regexp(cx, RegExpAlloc(cx));
+        RootedObject proto(cx);
+        if (!GetPrototypeFromCallableConstructor(cx, args, &proto))
+            return false;
+
+        Rooted<RegExpObject*> regexp(cx, RegExpAlloc(cx, proto));
         if (!regexp)
             return false;
 
         // Step 10.
+        if (args.hasDefined(1)) {
+            // Step 5c / 21.2.3.2.2 RegExpInitialize step 5.
+            flags = RegExpFlag(0);
+            RootedString flagStr(cx, ToString<CanGC>(cx, args[1]));
+            if (!flagStr)
+                return false;
+            if (!ParseRegExpFlags(cx, flagStr, &flags))
+                return false;
+        }
+
         if (!RegExpObject::initFromAtom(cx, regexp, sourceAtom, flags))
             return false;
 
@@ -404,7 +408,11 @@ js::regexp_construct(JSContext* cx, unsigned argc, Value* vp)
     }
 
     // Steps 8-9.
-    Rooted<RegExpObject*> regexp(cx, RegExpAlloc(cx));
+    RootedObject proto(cx);
+    if (!GetPrototypeFromCallableConstructor(cx, args, &proto))
+        return false;
+
+    Rooted<RegExpObject*> regexp(cx, RegExpAlloc(cx, proto));
     if (!regexp)
         return false;
 

js/src/tests/ecma_6/Class/extendBuiltinConstructors.js

@@ -29,6 +29,9 @@ testBuiltin(Number);
 testBuiltin(Date);
 testBuiltin(Date, 5);
 testBuiltin(Date, 5, 10);
+testBuiltin(RegExp);
+testBuiltin(RegExp, /Regexp Argument/);
+testBuiltin(RegExp, "String Argument");
 
 `;
 

js/src/tests/ecma_6/RegExp/constructor-ordering-2.js

@@ -0,0 +1,21 @@
+// Make sure that we don't ToString the second argument until /after/ doing
+// the appropriate subclassing lookups
+
+var didLookup = false;
+
+var re = /a/;
+var flags = { toString() { assertEq(didLookup, true); return "g"; } };
+var newRe = Reflect.construct(RegExp, [re, flags],
+                              Object.defineProperty(function(){}.bind(null), "prototype", {
+  get() {
+    didLookup = true;
+    return RegExp.prototype;
+  }
+}));
+
+assertEq(Object.getPrototypeOf(newRe), RegExp.prototype);
+assertEq(didLookup, true);
+
+
+if (typeof reportCompare === 'function')
+    reportCompare(0,0,"OK");

js/src/tests/ecma_6/RegExp/constructor-ordering.js

@@ -0,0 +1,16 @@
+// Make sure that we don't misorder subclassing accesses with respect to
+// accessing regex arg internal slots
+//
+// Test credit André Bargull.
+
+var re = /a/;
+var newRe = Reflect.construct(RegExp, [re], Object.defineProperty(function(){}.bind(null), "prototype", {
+  get() {
+    re.compile("b");
+    return RegExp.prototype;
+  }
+}));
+assertEq(newRe.source, "a");
+
+if (typeof reportCompare === 'function')
+    reportCompare(0,0,"OK");

js/src/vm/RegExpObject.cpp

@@ -41,14 +41,13 @@ JS_STATIC_ASSERT(MultilineFlag == JSREG_MULTILINE);
 JS_STATIC_ASSERT(StickyFlag == JSREG_STICKY);
 
 RegExpObject*
-js::RegExpAlloc(ExclusiveContext* cx)
+js::RegExpAlloc(ExclusiveContext* cx, HandleObject proto /* = nullptr */)
 {
     // Note: RegExp objects are always allocated in the tenured heap. This is
     // not strictly required, but simplifies embedding them in jitcode.
-    RegExpObject* regexp = NewBuiltinClassInstance<RegExpObject>(cx, TenuredObject);
+    RegExpObject* regexp = NewObjectWithClassProto<RegExpObject>(cx, proto, TenuredObject);
     if (!regexp)
         return nullptr;
-
     regexp->initPrivate(nullptr);
     return regexp;
 }

js/src/vm/RegExpObject.h

@@ -64,7 +64,7 @@ enum RegExpRunStatus
 };
 
 extern RegExpObject*
-RegExpAlloc(ExclusiveContext* cx);
+RegExpAlloc(ExclusiveContext* cx, HandleObject proto = nullptr);
 
 // |regexp| is under-typed because this function's used in the JIT.
 extern JSObject*