Fix minor ES6 compliance issue in RegExp.prototype.toString and optimize performance...
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 13 Jun 2015 01:35:08 +0000 (01:35 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 13 Jun 2015 01:35:08 +0000 (01:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=145935

Reviewed by Anders Carlsson.

Source/JavaScriptCore:

Test: js/regexp-toString.html

* runtime/RegExpPrototype.cpp:
(JSC::getFlags): Avoid memory allocation for the flags string by returning it in a character
buffer instead of constructing a WTF::String for it.
(JSC::regExpProtoFuncToString): Require only that the this value be an object; don't require
that it is actually a regular expression object. This is covered in the ES6 specification.
Also removed comment about the "/(?:)/" trick since that is now the repsonsibility of the
getter for the "source" property. Updated to use getFlags so we do one less memory allocation.
(JSC::regExpProtoGetterFlags): Chagned to use getFlags instead of the old flagsString.

LayoutTests:

* js/regexp-toString-expected.txt: Added.
* js/regexp-toString.html: Added.
* js/script-tests/regexp-toString.js: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@185528 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/js/regexp-toString-expected.txt [new file with mode: 0644]
LayoutTests/js/regexp-toString.html [new file with mode: 0644]
LayoutTests/js/script-tests/regexp-toString.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/RegExpPrototype.cpp

index 3127767..3ece6f6 100644 (file)
@@ -1,3 +1,14 @@
+2015-06-12  Darin Adler  <darin@apple.com>
+
+        Fix minor ES6 compliance issue in RegExp.prototype.toString and optimize performance a little
+        https://bugs.webkit.org/show_bug.cgi?id=145935
+
+        Reviewed by Anders Carlsson.
+
+        * js/regexp-toString-expected.txt: Added.
+        * js/regexp-toString.html: Added.
+        * js/script-tests/regexp-toString.js: Added.
+
 2015-06-12  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [CSS Grid Layout] Fix grid-template-areas parsing to avoid spaces
diff --git a/LayoutTests/js/regexp-toString-expected.txt b/LayoutTests/js/regexp-toString-expected.txt
new file mode 100644 (file)
index 0000000..11e3efa
--- /dev/null
@@ -0,0 +1,29 @@
+Test RegExp#toString
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Object.getOwnPropertyDescriptor(RegExp.prototype, 'toString').configurable is true
+PASS Object.getOwnPropertyDescriptor(RegExp.prototype, 'toString').enumerable is false
+PASS Object.getOwnPropertyDescriptor(RegExp.prototype, 'toString').get is undefined
+PASS Object.getOwnPropertyDescriptor(RegExp.prototype, 'toString').set is undefined
+PASS typeof Object.getOwnPropertyDescriptor(RegExp.prototype, 'toString').value is 'function'
+PASS RegExp.prototype.toString.call(new RegExp) is '/(?:)/'
+PASS RegExp.prototype.toString.call(new RegExp('a')) is '/a/'
+PASS RegExp.prototype.toString.call(new RegExp('\\\\')) is '/\\\\/'
+PASS RegExp.prototype.toString.call({}) is '/undefined/'
+PASS RegExp.prototype.toString.call({source: 'hi'}) is '/hi/'
+PASS RegExp.prototype.toString.call({ __proto__: { source: 'yo' } }) is '/yo/'
+PASS RegExp.prototype.toString.call({source: ''}) is '//'
+PASS RegExp.prototype.toString.call({source: '/'}) is '///'
+PASS RegExp.prototype.toString.call(undefined) threw exception TypeError: Type error.
+PASS RegExp.prototype.toString.call(null) threw exception TypeError: Type error.
+PASS RegExp.prototype.toString.call(false) threw exception TypeError: Type error.
+PASS RegExp.prototype.toString.call(true) threw exception TypeError: Type error.
+PASS RegExp.prototype.toString.call(0) threw exception TypeError: Type error.
+PASS RegExp.prototype.toString.call(0.5) threw exception TypeError: Type error.
+PASS RegExp.prototype.toString.call('x') threw exception TypeError: Type error.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/js/regexp-toString.html b/LayoutTests/js/regexp-toString.html
new file mode 100644 (file)
index 0000000..0b1acce
--- /dev/null
@@ -0,0 +1,10 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script src="script-tests/regexp-toString.js"></script>
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/js/script-tests/regexp-toString.js b/LayoutTests/js/script-tests/regexp-toString.js
new file mode 100644 (file)
index 0000000..4a0811c
--- /dev/null
@@ -0,0 +1,25 @@
+description('Test RegExp#toString');
+
+shouldBe("Object.getOwnPropertyDescriptor(RegExp.prototype, 'toString').configurable", "true");
+shouldBe("Object.getOwnPropertyDescriptor(RegExp.prototype, 'toString').enumerable", "false");
+shouldBe("Object.getOwnPropertyDescriptor(RegExp.prototype, 'toString').get", "undefined");
+shouldBe("Object.getOwnPropertyDescriptor(RegExp.prototype, 'toString').set", "undefined");
+shouldBe("typeof Object.getOwnPropertyDescriptor(RegExp.prototype, 'toString').value", "'function'");
+
+shouldBe("RegExp.prototype.toString.call(new RegExp)", "'/(?:)/'");
+shouldBe("RegExp.prototype.toString.call(new RegExp('a'))", "'/a/'");
+shouldBe("RegExp.prototype.toString.call(new RegExp('\\\\\\\\'))", "'/\\\\\\\\/'");
+
+shouldBe("RegExp.prototype.toString.call({})", "'/undefined/'");
+shouldBe("RegExp.prototype.toString.call({source: 'hi'})", "'/hi/'");
+shouldBe("RegExp.prototype.toString.call({ __proto__: { source: 'yo' } })", "'/yo/'");
+shouldBe("RegExp.prototype.toString.call({source: ''})", "'//'");
+shouldBe("RegExp.prototype.toString.call({source: '/'})", "'///'");
+
+shouldThrow("RegExp.prototype.toString.call(undefined)");
+shouldThrow("RegExp.prototype.toString.call(null)");
+shouldThrow("RegExp.prototype.toString.call(false)");
+shouldThrow("RegExp.prototype.toString.call(true)");
+shouldThrow("RegExp.prototype.toString.call(0)");
+shouldThrow("RegExp.prototype.toString.call(0.5)");
+shouldThrow("RegExp.prototype.toString.call('x')");
index c480a21..be6ca3e 100644 (file)
@@ -1,3 +1,21 @@
+2015-06-12  Darin Adler  <darin@apple.com>
+
+        Fix minor ES6 compliance issue in RegExp.prototype.toString and optimize performance a little
+        https://bugs.webkit.org/show_bug.cgi?id=145935
+
+        Reviewed by Anders Carlsson.
+
+        Test: js/regexp-toString.html
+
+        * runtime/RegExpPrototype.cpp:
+        (JSC::getFlags): Avoid memory allocation for the flags string by returning it in a character
+        buffer instead of constructing a WTF::String for it.
+        (JSC::regExpProtoFuncToString): Require only that the this value be an object; don't require
+        that it is actually a regular expression object. This is covered in the ES6 specification.
+        Also removed comment about the "/(?:)/" trick since that is now the repsonsibility of the
+        getter for the "source" property. Updated to use getFlags so we do one less memory allocation.
+        (JSC::regExpProtoGetterFlags): Chagned to use getFlags instead of the old flagsString.
+
 2015-06-12  Basile Clement  <basile_clement@apple.com>
 
         DFG Object Allocation Sinking should not consider GetClosureVar as escapes
index 84a7163..fb6fc5f 100644 (file)
@@ -137,37 +137,39 @@ EncodedJSValue JSC_HOST_CALL regExpProtoFuncCompile(ExecState* exec)
     return JSValue::encode(jsUndefined());
 }
 
-static inline String flagsString(ExecState *exec, JSObject* regexp)
+typedef std::array<char, 3 + 1> FlagsString; // 3 different flags and a null character terminator.
+
+static inline FlagsString flagsString(ExecState* exec, JSObject* regexp)
 {
-    char flags[4] = { 0, 0, 0, 0 };
-    int index = 0;
+    FlagsString string;
 
     JSValue globalValue = regexp->get(exec, exec->propertyNames().global);
     if (exec->hadException())
-        return String();
+        return string;
     JSValue ignoreCaseValue = regexp->get(exec, exec->propertyNames().ignoreCase);
     if (exec->hadException())
-        return String();
+        return string;
     JSValue multilineValue = regexp->get(exec, exec->propertyNames().multiline);
-    if (exec->hadException())
-        return String();
 
+    unsigned index = 0;
     if (globalValue.toBoolean(exec))
-        flags[index++] = 'g';
+        string[index++] = 'g';
     if (ignoreCaseValue.toBoolean(exec))
-        flags[index++] = 'i';
+        string[index++] = 'i';
     if (multilineValue.toBoolean(exec))
-        flags[index] = 'm';
-    return String(flags);
+        string[index++] = 'm';
+    ASSERT(index < string.size());
+    string[index] = 0;
+    return string;
 }
 
 EncodedJSValue JSC_HOST_CALL regExpProtoFuncToString(ExecState* exec)
 {
     JSValue thisValue = exec->thisValue();
-    if (!thisValue.inherits(RegExpObject::info()))
+    if (!thisValue.isObject())
         return throwVMTypeError(exec);
 
-    RegExpObject* thisObject = asRegExpObject(thisValue);
+    JSObject* thisObject = asObject(thisValue);
 
     StringRecursionChecker checker(exec, thisObject);
     if (JSValue earlyReturnValue = checker.earlyReturnValue())
@@ -180,12 +182,11 @@ EncodedJSValue JSC_HOST_CALL regExpProtoFuncToString(ExecState* exec)
     if (exec->hadException())
         return JSValue::encode(jsUndefined());
 
-    String flags = flagsString(exec, thisObject);
+    auto flags = flagsString(exec, thisObject);
     if (exec->hadException())
         return JSValue::encode(jsUndefined());
 
-    // If source is empty, use "/(?:)/" to avoid colliding with comment syntax
-    return JSValue::encode(jsMakeNontrivialString(exec, "/", source, "/", flags));
+    return JSValue::encode(jsMakeNontrivialString(exec, '/', source, '/', flags.data()));
 }
 
 EncodedJSValue JSC_HOST_CALL regExpProtoGetterGlobal(ExecState* exec)
@@ -219,16 +220,15 @@ EncodedJSValue JSC_HOST_CALL regExpProtoGetterFlags(ExecState* exec)
 {
     JSValue thisValue = exec->thisValue();
     if (!thisValue.isObject())
-        return JSValue::encode(throwTypeError(exec));
+        return throwVMTypeError(exec);
 
-    String flags = flagsString(exec, asObject(thisValue));
+    auto flags = flagsString(exec, asObject(thisValue));
     if (exec->hadException())
         return JSValue::encode(jsUndefined());
 
-    return JSValue::encode(jsString(exec, flags));
+    return JSValue::encode(jsString(exec, flags.data()));
 }
 
-
 template <typename CharacterType>
 static inline void appendLineTerminatorEscape(StringBuilder&, CharacterType);