[ES6] Make RegExp.prototype.toString spec compliant
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Mar 2016 07:10:18 +0000 (07:10 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Mar 2016 07:10:18 +0000 (07:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=155341

Reviewed by Filip Pizlo.

Before we were directly calling into the flagsString
function. Instead, we must get the "flags" property
of the thisObject. This will usually call into the flags
getter, but not always. Specifically, you can you a Proxy
to observe this behavior.

* runtime/RegExpPrototype.cpp:
(JSC::regExpProtoFuncToString):
(JSC::regExpProtoGetterGlobal):
* tests/es6.yaml:
* tests/es6/Proxy_internal_get_calls_RegExp.prototype.toString.js: Added.
(test.get var):
(test.):
* tests/stress/regexp-prototype-tostring.js: Added.
(assert):
(test):
(test.get var):
(test.):
(let.handler.get switch):
(let.handler):
(get test):
(test.get RegExp):

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

LayoutTests/js/regexp-toString-expected.txt
LayoutTests/js/script-tests/regexp-toString.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/RegExpPrototype.cpp
Source/JavaScriptCore/tests/es6.yaml
Source/JavaScriptCore/tests/es6/Proxy_internal_get_calls_RegExp.prototype.toString.js [new file with mode: 0644]
Source/JavaScriptCore/tests/stress/regexp-prototype-tostring.js [new file with mode: 0644]

index 11e3efa..b4c7bf7 100644 (file)
@@ -11,11 +11,11 @@ PASS typeof Object.getOwnPropertyDescriptor(RegExp.prototype, 'toString').value
 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({}) is '/undefined/undefined'
+PASS RegExp.prototype.toString.call({source: 'hi'}) is '/hi/undefined'
+PASS RegExp.prototype.toString.call({ __proto__: { source: 'yo' } }) is '/yo/undefined'
+PASS RegExp.prototype.toString.call({source: ''}) is '//undefined'
+PASS RegExp.prototype.toString.call({source: '/'}) is '///undefined'
 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.
index 4a0811c..20ef02e 100644 (file)
@@ -10,11 +10,11 @@ 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: '/'})", "'///'");
+shouldBe("RegExp.prototype.toString.call({})", "'/undefined/undefined'");
+shouldBe("RegExp.prototype.toString.call({source: 'hi'})", "'/hi/undefined'");
+shouldBe("RegExp.prototype.toString.call({ __proto__: { source: 'yo' } })", "'/yo/undefined'");
+shouldBe("RegExp.prototype.toString.call({source: ''})", "'//undefined'");
+shouldBe("RegExp.prototype.toString.call({source: '/'})", "'///undefined'");
 
 shouldThrow("RegExp.prototype.toString.call(undefined)");
 shouldThrow("RegExp.prototype.toString.call(null)");
index 34d6f50..a46976f 100644 (file)
@@ -1,3 +1,33 @@
+2016-03-10  Saam barati  <sbarati@apple.com>
+
+        [ES6] Make RegExp.prototype.toString spec compliant
+        https://bugs.webkit.org/show_bug.cgi?id=155341
+
+        Reviewed by Filip Pizlo.
+
+        Before we were directly calling into the flagsString
+        function. Instead, we must get the "flags" property
+        of the thisObject. This will usually call into the flags
+        getter, but not always. Specifically, you can you a Proxy
+        to observe this behavior.
+
+        * runtime/RegExpPrototype.cpp:
+        (JSC::regExpProtoFuncToString):
+        (JSC::regExpProtoGetterGlobal):
+        * tests/es6.yaml:
+        * tests/es6/Proxy_internal_get_calls_RegExp.prototype.toString.js: Added.
+        (test.get var):
+        (test.):
+        * tests/stress/regexp-prototype-tostring.js: Added.
+        (assert):
+        (test):
+        (test.get var):
+        (test.):
+        (let.handler.get switch):
+        (let.handler):
+        (get test):
+        (test.get RegExp):
+
 2016-03-10  Benjamin Poulain  <bpoulain@apple.com>
 
         [JSC] Add register reuse for ArithAdd of an Int32 and constant in DFG
index 2ef0699..0c2df71 100644 (file)
@@ -208,18 +208,22 @@ EncodedJSValue JSC_HOST_CALL regExpProtoFuncToString(ExecState* exec)
     if (JSValue earlyReturnValue = checker.earlyReturnValue())
         return JSValue::encode(earlyReturnValue);
 
-    JSValue sourceValue = thisObject->get(exec, exec->propertyNames().source);
-    if (exec->hadException())
+    VM& vm = exec->vm();
+    JSValue sourceValue = thisObject->get(exec, vm.propertyNames->source);
+    if (vm.exception())
         return JSValue::encode(jsUndefined());
     String source = sourceValue.toString(exec)->value(exec);
-    if (exec->hadException())
+    if (vm.exception())
         return JSValue::encode(jsUndefined());
 
-    auto flags = flagsString(exec, thisObject);
-    if (exec->hadException())
+    JSValue flagsValue = thisObject->get(exec, vm.propertyNames->flags);
+    if (vm.exception())
+        return JSValue::encode(jsUndefined());
+    String flags = flagsValue.toString(exec)->value(exec);
+    if (vm.exception())
         return JSValue::encode(jsUndefined());
 
-    return JSValue::encode(jsMakeNontrivialString(exec, '/', source, '/', flags.data()));
+    return JSValue::encode(jsMakeNontrivialString(exec, '/', source, '/', flags));
 }
 
 EncodedJSValue JSC_HOST_CALL regExpProtoGetterGlobal(ExecState* exec)
index 6391d8e..2fa46be 100644 (file)
   cmd: runES6 :normal
 - path: es6/Proxy_internal_get_calls_RegExp.prototype.test.js
   cmd: runES6 :fail
+- path: es6/Proxy_internal_get_calls_RegExp.prototype.toString.js
+  cmd: runES6 :normal
 - path: es6/Proxy_internal_get_calls_RegExp.prototype[Symbol.match].js
   cmd: runES6 :fail
 - path: es6/Proxy_internal_get_calls_RegExp.prototype[Symbol.replace].js
diff --git a/Source/JavaScriptCore/tests/es6/Proxy_internal_get_calls_RegExp.prototype.toString.js b/Source/JavaScriptCore/tests/es6/Proxy_internal_get_calls_RegExp.prototype.toString.js
new file mode 100644 (file)
index 0000000..fc244c5
--- /dev/null
@@ -0,0 +1,10 @@
+// RegExp.prototype.toString -> Get -> [[Get]]
+function test() {
+    var get = [];
+    var p = new Proxy({}, { get: function(o, k) { get.push(k); return o[k]; }});
+    RegExp.prototype.toString.call(p);
+    return get + '' === "source,flags";
+}
+
+if (!test())
+    throw new Error("Test failed.")
diff --git a/Source/JavaScriptCore/tests/stress/regexp-prototype-tostring.js b/Source/JavaScriptCore/tests/stress/regexp-prototype-tostring.js
new file mode 100644 (file)
index 0000000..ae12e93
--- /dev/null
@@ -0,0 +1,90 @@
+function assert(b) {
+    if (!b)
+        throw new Error("bad assertion")
+}
+function test(f) {
+    for (let i = 0; i < 100; i++)
+        f();
+}
+
+test(function() {
+    var get = [];
+    var p = new Proxy({}, { get: function(o, k) { get.push(k); return o[k]; }});
+    RegExp.prototype.toString.call(p);
+    assert(get + '' === "source,flags");
+});
+
+test(function() {
+    let handler = {
+        get: function(o, propName) {
+            switch(propName) {
+            case 'source': 
+                return "foobar";
+            case 'flags': 
+                return "whatever";
+            default:
+                assert(false, "should not be reached");
+            }
+        }
+    }
+    let proxy = new Proxy({}, handler);
+    let result = RegExp.prototype.toString.call(proxy);
+    assert(result === "/foobar/whatever");
+});
+
+test(function() {
+    let handler = {
+        get: function(o, propName) {
+            switch(propName) {
+            case 'source': 
+                return "hello";
+            case 'flags': 
+                return "y";
+            default:
+                assert(false, "should not be reached");
+            }
+        }
+    }
+
+    let proxy = new Proxy({}, handler);
+    let result = RegExp.prototype.toString.call(proxy);
+    assert(result === "/hello/y");
+});
+
+test(function() {
+    let error = null;
+    let obj = {
+        get flags() {
+            error = new Error;
+            throw error;
+        }
+    }
+
+    let threw = false;
+    try {
+        RegExp.prototype.toString.call(obj);
+    } catch(e) {
+        assert(e === error);
+        threw = true;
+    }
+    assert(threw);
+});
+
+test(function() {
+    let error = null;
+    let obj = {
+        get source() {
+            error = new Error;
+            throw error;
+        }
+    }
+
+    let threw = false;
+    try {
+        RegExp.prototype.toString.call(obj);
+    } catch(e) {
+        assert(e === error);
+        threw = true;
+    }
+    assert(threw);
+});