RegExp(RegExp object, flags) no longer throws

Issue #1634

4 files changed, 52 insertions, 132 deletions

diff --git a/js/src/builtin/RegExp.cpp b/js/src/builtin/RegExp.cpp
index 4793be324..c4ea6edce 100644
--- a/js/src/builtin/RegExp.cpp
+++ b/js/src/builtin/RegExp.cpp
@@ -141,18 +141,11 @@ js::ExecuteRegExpLegacy(JSContext* cx, RegExpStatics* res, RegExpObject& reobj,
 
 /*
  * Compile a new |RegExpShared| for the |RegExpObject|.
- *
- * Per ECMAv5 15.10.4.1, we act on combinations of (pattern, flags) as
- * arguments:
- *
- *  RegExp, undefined => flags := pattern.flags
- *  RegExp, _ => throw TypeError
- *  _ => pattern := ToString(pattern) if defined(pattern) else ''
- *       flags := ToString(flags) if defined(flags) else ''
  */
 static bool
 CompileRegExpObject(JSContext* cx, RegExpObjectBuilder& builder,
-                    const CallArgs& args, RegExpStaticsUse staticsUse)
+                    const CallArgs& args, RegExpStaticsUse staticsUse,
+                    RegExpCreationMode creationMode)
 {
     if (args.length() == 0) {
         MOZ_ASSERT(staticsUse == UseRegExpStatics);
@@ -169,34 +162,46 @@ CompileRegExpObject(JSContext* cx, RegExpObjectBuilder& builder,
 
     RootedValue sourceValue(cx, args[0]);
 
-    /*
-     * If we get passed in an object whose internal [[Class]] property is
-     * "RegExp", return a new object with the same source/flags.
-     */
     if (IsObjectWithClass(sourceValue, ESClass_RegExp, cx)) {
         /*
-         * Beware, sourceObj may be a (transparent) proxy to a RegExp, so only
-         * use generic (proxyable) operations on sourceObj that do not assume
-         * sourceObj.is<RegExpObject>().
+         * For RegExp.prototype.compile, if the first argument is a RegExp object,
+         * the second argument must be undefined. Otherwise, throw a TypeError.
          */
-        RootedObject sourceObj(cx, &sourceValue.toObject());
-
-        if (args.hasDefined(1)) {
+        if (args.hasDefined(1) && creationMode == CreateForCompile) {
             JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_NEWREGEXP_FLAGGED);
             return false;
         }
 
         /*
-         * Only extract the 'flags' out of sourceObj; do not reuse the
-         * RegExpShared since it may be from a different compartment.
+         * Beware, sourceObj may be a (transparent) proxy to a RegExp, so only
+         * use generic (proxyable) operations on sourceObj that do not assume
+         * sourceObj.is<RegExpObject>().
          */
+        RootedObject sourceObj(cx, &sourceValue.toObject());
+
         RegExpFlag flags;
         {
+            /*
+             * Only extract the 'flags' out of sourceObj; do not reuse the
+             * RegExpShared since it may be from a different compartment.
+             */
             RegExpGuard g(cx);
             if (!RegExpToShared(cx, sourceObj, &g))
                 return false;
-
-            flags = g->getFlags();
+            /*
+             * If args[1] is not undefined, then parse the 'flags' from args[1].
+             * Otherwise, extract the 'flags' from sourceObj.
+             */
+            if (args.hasDefined(1)) {
+                flags = RegExpFlag(0);
+                RootedString flagStr(cx, ToString<CanGC>(cx, args[1]));
+                if (!flagStr)
+                    return false;
+                if (!ParseRegExpFlags(cx, flagStr, &flags))
+                    return false;
+            } else {
+                flags = g->getFlags();
+            }
         }
 
         /*
@@ -233,7 +238,6 @@ CompileRegExpObject(JSContext* cx, RegExpObjectBuilder& builder,
         RootedString flagStr(cx, ToString<CanGC>(cx, args[1]));
         if (!flagStr)
             return false;
-        args[1].setString(flagStr);
         if (!ParseRegExpFlags(cx, flagStr, &flags))
             return false;
     }
@@ -273,7 +277,7 @@ regexp_compile_impl(JSContext* cx, const CallArgs& args)
 {
     MOZ_ASSERT(IsRegExp(args.thisv()));
     RegExpObjectBuilder builder(cx, &args.thisv().toObject().as<RegExpObject>());
-    return CompileRegExpObject(cx, builder, args, UseRegExpStatics);
+    return CompileRegExpObject(cx, builder, args, UseRegExpStatics, CreateForCompile);
 }
 
 static bool
@@ -304,7 +308,7 @@ regexp_construct(JSContext* cx, unsigned argc, Value* vp)
     }
 
     RegExpObjectBuilder builder(cx);
-    return CompileRegExpObject(cx, builder, args, UseRegExpStatics);
+    return CompileRegExpObject(cx, builder, args, UseRegExpStatics, CreateForConstruct);
 }
 
 bool
@@ -318,7 +322,7 @@ js::regexp_construct_no_statics(JSContext* cx, unsigned argc, Value* vp)
     MOZ_ASSERT(!args.isConstructing());
 
     RegExpObjectBuilder builder(cx);
-    return CompileRegExpObject(cx, builder, args, DontUseRegExpStatics);
+    return CompileRegExpObject(cx, builder, args, DontUseRegExpStatics, CreateForConstruct);
 }
 
 MOZ_ALWAYS_INLINE bool
diff --git a/js/src/builtin/RegExp.h b/js/src/builtin/RegExp.h
index bbf86c337..d067cb922 100644
--- a/js/src/builtin/RegExp.h
+++ b/js/src/builtin/RegExp.h
@@ -29,6 +29,9 @@ 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);
diff --git a/js/src/tests/ecma_3/RegExp/15.10.4.1-5-n.js b/js/src/tests/ecma_3/RegExp/15.10.4.1-5-n.js
deleted file mode 100644
index 23ce2ff51..000000000
--- a/js/src/tests/ecma_3/RegExp/15.10.4.1-5-n.js
+++ /dev/null
@@ -1,105 +0,0 @@
-/* -*- indent-tabs-mode: nil; js-indent-level: 4 -*- */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-/*
- *
- * Date: 26 November 2000
- *
- *
- *SUMMARY: Passing a RegExp object to a RegExp() constructor.
- *This test arose from Bugzilla bug 61266. The ECMA3 section is:      
- *
- *  15.10.4.1 new RegExp(pattern, flags)
- *
- *  If pattern is an object R whose [[Class]] property is "RegExp" and
- *  flags is undefined, then let P be the pattern used to construct R
- *  and let F be the flags used to construct R. If pattern is an object R
- *  whose [[Class]] property is "RegExp" and flags is not undefined,
- *  then throw a TypeError exception. Otherwise, let P be the empty string 
- *  if pattern is undefined and ToString(pattern) otherwise, and let F be
- *  the empty string if flags is undefined and ToString(flags) otherwise.
- *
- *
- *The current test will check the second scenario outlined above:
- *
- *   "pattern" is itself a RegExp object R
- *   "flags" is NOT undefined
- *
- * This should throw an exception ... we test for this.
- *
- */
-
-//-------------------------------------------------------------------------------------------------
-var BUGNUMBER = '61266';
-var summary = 'Negative test: Passing (RegExp object, flag) to RegExp() constructor';
-var statprefix = 'Passing RegExp object on pattern ';
-var statsuffix =  '; passing flag ';
-var cnFAILURE = 'Expected an exception to be thrown, but none was -';
-var singlequote = "'";
-var i = -1; var j = -1; var s = ''; var f = '';
-var obj1 = {}; var obj2 = {};
-var patterns = new Array();
-var flags = new Array(); 
-
-
-// various regular expressions to try -
-patterns[0] = '';
-patterns[1] = 'abc';
-patterns[2] = '(.*)(3-1)\s\w';
-patterns[3] = '(.*)(...)\\s\\w';
-patterns[4] = '[^A-Za-z0-9_]';
-patterns[5] = '[^\f\n\r\t\v](123.5)([4 - 8]$)';
-
-// various flags to try -
-flags[0] = 'i';
-flags[1] = 'g';
-flags[2] = 'm';
-
-
-DESCRIPTION = "Negative test: Passing (RegExp object, flag) to RegExp() constructor"
-  EXPECTED = "error";
-
-
-//-------------------------------------------------------------------------------------------------
-test();
-//-------------------------------------------------------------------------------------------------
-
-
-function test()
-{
-  enterFunc ('test');
-  printBugNumber(BUGNUMBER);
-  printStatus (summary);
-
-  for (i in patterns)
-  {
-    s = patterns[i];
-
-    for (j in flags)
-    {
-      f = flags[j];
-      printStatus(getStatus(s, f));
-      obj1 = new RegExp(s, f);  
-      obj2 = new RegExp(obj1, f);   // this should cause an exception
-
-      // WE SHOULD NEVER REACH THIS POINT -
-      reportCompare('PASS', 'FAIL', cnFAILURE);
-    }
-  }
-
-  exitFunc ('test');
-}
-
-
-function getStatus(regexp, flag)
-{
-  return (statprefix  +  quote(regexp) +  statsuffix  +   flag);
-}
-
-
-function quote(text)
-{
-  return (singlequote  +  text  + singlequote);
-}
diff --git a/js/src/tests/ecma_6/RegExp/flags-param-handling.js b/js/src/tests/ecma_6/RegExp/flags-param-handling.js
new file mode 100644
index 000000000..e47799601
--- /dev/null
+++ b/js/src/tests/ecma_6/RegExp/flags-param-handling.js
@@ -0,0 +1,18 @@
+assertEq(RegExp(/foo/my).flags, "my");
+assertEq(RegExp(/foo/, "gi").flags, "gi");
+assertEq(RegExp(/foo/my, "gi").flags, "gi");
+assertEq(RegExp(/foo/my, "").flags, "");
+assertEq(RegExp(/foo/my, undefined).flags, "my");
+assertThrowsInstanceOf(() => RegExp(/foo/my, null), SyntaxError);
+assertThrowsInstanceOf(() => RegExp(/foo/my, "foo"), SyntaxError);
+
+assertEq(/a/.compile("b", "gi").flags, "gi");
+assertEq(/a/.compile(/b/my).flags, "my");
+assertEq(/a/.compile(/b/my, undefined).flags, "my");
+assertThrowsInstanceOf(() => /a/.compile(/b/my, "gi"), TypeError);
+assertThrowsInstanceOf(() => /a/.compile(/b/my, ""), TypeError);
+assertThrowsInstanceOf(() => /a/.compile(/b/my, null), TypeError);
+assertThrowsInstanceOf(() => /a/.compile(/b/my, "foo"), TypeError);
+
+if (typeof reportCompare === "function")
+    reportCompare(true, true);