Align RegExp[@@match] with other @@ methods
authormsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Apr 2016 10:57:45 +0000 (10:57 +0000)
committermsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Apr 2016 10:57:45 +0000 (10:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=156832

Reviewed by Mark Lam.

Various changes to align the RegExp[@@match] with [@@search] and [@@split].

Made RegExp.prototype.@exec a hidden property on the global object and
called it @regExpBuiltinExec to match the name it has in the standard.
Changed all places that used the old name to use the new one.

Made the match fast path function, which used to be call @match, to be called
@regExpMatchFast and put it on the global object.  Changed it to also handle
expressions both with and without the global flag.  Refactored the builtin
@match accordingly.

Added the builtin function @hasObservableSideEffectsForRegExpMatch() that
checks to see if we can use the fast path of if we need the explicit version.

Put the main RegExp functions @match, @search and @split in alphabetical
order in RegExpPrototype.js.  Did the same for @match, @repeat, @search and
@split in StringPrototype.js.

* builtins/RegExpPrototype.js:
(regExpExec):
(hasObservableSideEffectsForRegExpMatch): New.
(match):
(search):
(hasObservableSideEffectsForRegExpSplit):
Reordered in the file and updated to use @regExpBuiltinExec.

* builtins/StringPrototype.js:
(match):
(repeatSlowPath):
(repeat):
(search):
(split):
Reordered functions in the file.

* runtime/CommonIdentifiers.h:
* runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::setGlobalThis):
(JSC::getById):
(JSC::getGetterById):
(JSC::JSGlobalObject::init):
* runtime/RegExpPrototype.cpp:
(JSC::RegExpPrototype::finishCreation):
(JSC::regExpProtoFuncExec):
(JSC::regExpProtoFuncMatchFast):
(JSC::regExpProtoFuncMatchPrivate): Deleted.
* runtime/RegExpPrototype.h:

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/builtins/RegExpPrototype.js
Source/JavaScriptCore/builtins/StringPrototype.js
Source/JavaScriptCore/runtime/CommonIdentifiers.h
Source/JavaScriptCore/runtime/JSGlobalObject.cpp
Source/JavaScriptCore/runtime/RegExpPrototype.cpp
Source/JavaScriptCore/runtime/RegExpPrototype.h

index 26f138b..23f1283 100644 (file)
@@ -1,3 +1,57 @@
+2016-04-21  Michael Saboff  <msaboff@apple.com>
+
+        Align RegExp[@@match] with other @@ methods
+        https://bugs.webkit.org/show_bug.cgi?id=156832
+
+        Reviewed by Mark Lam.
+
+        Various changes to align the RegExp[@@match] with [@@search] and [@@split].
+
+        Made RegExp.prototype.@exec a hidden property on the global object and
+        called it @regExpBuiltinExec to match the name it has in the standard.
+        Changed all places that used the old name to use the new one.
+
+        Made the match fast path function, which used to be call @match, to be called
+        @regExpMatchFast and put it on the global object.  Changed it to also handle
+        expressions both with and without the global flag.  Refactored the builtin
+        @match accordingly.
+
+        Added the builtin function @hasObservableSideEffectsForRegExpMatch() that
+        checks to see if we can use the fast path of if we need the explicit version.
+
+        Put the main RegExp functions @match, @search and @split in alphabetical
+        order in RegExpPrototype.js.  Did the same for @match, @repeat, @search and 
+        @split in StringPrototype.js.
+        
+        * builtins/RegExpPrototype.js:
+        (regExpExec):
+        (hasObservableSideEffectsForRegExpMatch): New.
+        (match):
+        (search):
+        (hasObservableSideEffectsForRegExpSplit):
+        Reordered in the file and updated to use @regExpBuiltinExec.
+
+        * builtins/StringPrototype.js:
+        (match):
+        (repeatSlowPath):
+        (repeat):
+        (search):
+        (split):
+        Reordered functions in the file.
+
+        * runtime/CommonIdentifiers.h:
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::setGlobalThis):
+        (JSC::getById):
+        (JSC::getGetterById):
+        (JSC::JSGlobalObject::init):
+        * runtime/RegExpPrototype.cpp:
+        (JSC::RegExpPrototype::finishCreation):
+        (JSC::regExpProtoFuncExec):
+        (JSC::regExpProtoFuncMatchFast):
+        (JSC::regExpProtoFuncMatchPrivate): Deleted.
+        * runtime/RegExpPrototype.h:
+
 2016-04-20  Geoffrey Garen  <ggaren@apple.com>
 
         JavaScriptCore garbage collection is missing an autorelease pool
index 61c70dc..f6536dc 100644 (file)
@@ -45,7 +45,38 @@ function advanceStringIndex(string, index, unicode)
     return index + 2;
 }
 
-function match(str)
+function regExpExec(regexp, str)
+{
+    "use strict";
+
+    let exec = regexp.exec;
+    let builtinExec = @regExpBuiltinExec;
+    if (exec !== builtinExec && typeof exec === "function") {
+        let result = exec.@call(regexp, str);
+        if (result !== null && !@isObject(result))
+            throw new @TypeError("The result of a RegExp exec must be null or an object");
+        return result;
+    }
+    return builtinExec.@call(regexp, str);
+}
+
+function hasObservableSideEffectsForRegExpMatch(regexp) {
+    // This is accessed by the RegExpExec internal function.
+    let regexpExec = @tryGetById(regexp, "exec");
+    if (regexpExec !== @regExpBuiltinExec)
+        return true;
+
+    let regexpGlobal = @tryGetById(regexp, "global");
+    if (regexpGlobal !== @regExpProtoGlobalGetter)
+        return true;
+    let regexpUnicode = @tryGetById(regexp, "unicode");
+    if (regexpUnicode !== @regExpProtoUnicodeGetter)
+        return true;
+
+    return !@isRegExpObject(regexp);
+}
+
+function match(strArg)
 {
     "use strict";
 
@@ -53,59 +84,40 @@ function match(str)
         throw new @TypeError("RegExp.prototype.@@match requires that |this| be an Object");
 
     let regexp = this;
-    let stringArg = @toString(str);
 
-    if (!regexp.global)
-        return regexp.exec(stringArg);
+    // Check for observable side effects and call the fast path if there aren't any.
+    if (!@hasObservableSideEffectsForRegExpMatch(regexp))
+        return @regExpMatchFast.@call(regexp, strArg);
 
+    let str = @toString(strArg);
+
+    if (!regexp.global)
+        return @regExpExec(regexp, str);
+    
     let unicode = regexp.unicode;
     regexp.lastIndex = 0;
     let resultList = [];
-    let execFunc = regexp.exec;
-
-    if (execFunc !== @RegExp.prototype.@exec && typeof execFunc === "function") {
-        // Match using the overridden exec.
-        let stringLength = stringArg.length;
-
-        while (true) {
-            let result = execFunc(stringArg);
-            
-            if (result === null) {
-                if (resultList.length === 0)
-                    return null;
-                return resultList;
-            }
-
-            if (!@isObject(result))
-                throw new @TypeError("RegExp.prototype.@@match call to RegExp.exec didn't return null or an object");
-
-            let resultString = @toString(result[0]);
-
-            if (!resultString.length)
-                regexp.lastIndex = @advanceStringIndex(stringArg, regexp.lastIndex, unicode);
-
-            resultList.@push(resultString);
-
-            execFunc = regexp.exec;
+    let stringLength = str.length;
+
+    while (true) {
+        let result = @regExpExec(regexp, str);
+        
+        if (result === null) {
+            if (resultList.length === 0)
+                return null;
+            return resultList;
         }
-    }
 
-    return regexp.@match(stringArg);
-}
+        if (!@isObject(result))
+            throw new @TypeError("RegExp.prototype.@@match call to RegExp.exec didn't return null or an object");
 
-function regExpExec(regexp, str)
-{
-    "use strict";
+        let resultString = @toString(result[0]);
 
-    let exec = regexp.exec;
-    let builtinExec = @RegExp.prototype.@exec;
-    if (exec !== builtinExec && typeof exec === "function") {
-        let result = exec.@call(regexp, str);
-        if (result !== null && !@isObject(result))
-            throw new @TypeError("The result of a RegExp exec must be null or an object");
-        return result;
+        if (!resultString.length)
+            regexp.lastIndex = @advanceStringIndex(str, regexp.lastIndex, unicode);
+
+        resultList.@push(resultString);
     }
-    return builtinExec.@call(regexp, str);
 }
 
 // 21.2.5.9 RegExp.prototype[@@search] (string)
@@ -116,7 +128,7 @@ function search(strArg)
     let regexp = this;
 
     // Check for observable side effects and call the fast path if there aren't any.
-    if (@isRegExpObject(regexp) && @tryGetById(regexp, "exec") === @RegExp.prototype.@exec)
+    if (@isRegExpObject(regexp) && @tryGetById(regexp, "exec") === @regExpBuiltinExec)
         return @regExpSearchFast.@call(regexp, strArg);
 
     // 1. Let rx be the this value.
@@ -145,7 +157,7 @@ function search(strArg)
 function hasObservableSideEffectsForRegExpSplit(regexp) {
     // This is accessed by the RegExpExec internal function.
     let regexpExec = @tryGetById(regexp, "exec");
-    if (regexpExec !== @RegExp.prototype.@exec)
+    if (regexpExec !== @regExpBuiltinExec)
         return true;
     
     // This is accessed by step 5 below.
index 807805e..8fecf00 100644 (file)
@@ -46,27 +46,6 @@ function match(regexp)
     return createdRegExp[@symbolMatch](thisString);
 }
 
-function search(regexp)
-{
-    "use strict";
-
-    if (this == null) {
-        if (this === null)
-            throw new @TypeError("String.prototype.search requires that |this| not be null");
-        throw new @TypeError("String.prototype.search requires that |this| not be undefined");
-    }
-
-    if (regexp != null) {
-        var searcher = regexp[@symbolSearch];
-        if (searcher != @undefined)
-            return searcher.@call(regexp, this);
-    }
-
-    var thisString = @toString(this);
-    var createdRegExp = @regExpCreate(regexp, @undefined);
-    return createdRegExp[@symbolSearch](thisString);
-}
-
 function repeatSlowPath(string, count)
 {
     "use strict";
@@ -126,6 +105,27 @@ function repeat(count)
     return @repeatSlowPath(string, count);
 }
 
+function search(regexp)
+{
+    "use strict";
+
+    if (this == null) {
+        if (this === null)
+            throw new @TypeError("String.prototype.search requires that |this| not be null");
+        throw new @TypeError("String.prototype.search requires that |this| not be undefined");
+    }
+
+    if (regexp != null) {
+        var searcher = regexp[@symbolSearch];
+        if (searcher != @undefined)
+            return searcher.@call(regexp, this);
+    }
+
+    var thisString = @toString(this);
+    var createdRegExp = @regExpCreate(regexp, @undefined);
+    return createdRegExp[@symbolSearch](thisString);
+}
+
 function split(separator, limit)
 {
     "use strict";
index f47c3a2..21b1a54 100644 (file)
     macro(setIteratorNext) \
     macro(MapIterator) \
     macro(mapIteratorNext) \
+    macro(regExpBuiltinExec) \
+    macro(regExpMatchFast) \
     macro(regExpProtoFlagsGetter) \
     macro(regExpProtoGlobalGetter) \
     macro(regExpProtoIgnoreCaseGetter) \
index fa49a99..55ef6ed 100644 (file)
@@ -257,7 +257,6 @@ void JSGlobalObject::setGlobalThis(VM& vm, JSObject* globalThis)
     m_globalThis.set(vm, this, globalThis);
 }
 
-
 static JSObject* getGetterById(ExecState* exec, JSObject* base, const Identifier& ident)
 {
     JSValue baseValue = JSValue(base);
@@ -646,10 +645,13 @@ putDirectWithoutTransition(vm, vm.propertyNames-> jsName, lowerName ## Construct
         GlobalPropertyInfo(vm.propertyNames->regExpProtoUnicodeGetterPrivateName, regExpProtoUnicodeGetterObject, DontEnum | DontDelete | ReadOnly),
 
         // RegExp.prototype helpers.
+        GlobalPropertyInfo(vm.propertyNames->regExpBuiltinExecPrivateName, m_regExpPrototype->getDirect(vm, vm.propertyNames->exec), DontEnum | DontDelete | ReadOnly),
         GlobalPropertyInfo(vm.propertyNames->regExpCreatePrivateName, JSFunction::create(vm, this, 2, String(), esSpecRegExpCreate, NoIntrinsic), DontEnum | DontDelete | ReadOnly),
+        GlobalPropertyInfo(vm.propertyNames->builtinNames().hasObservableSideEffectsForRegExpMatchPrivateName(), JSFunction::createBuiltinFunction(vm, regExpPrototypeHasObservableSideEffectsForRegExpMatchCodeGenerator(vm), this), DontEnum | DontDelete | ReadOnly),
         GlobalPropertyInfo(vm.propertyNames->builtinNames().hasObservableSideEffectsForRegExpSplitPrivateName(), JSFunction::createBuiltinFunction(vm, regExpPrototypeHasObservableSideEffectsForRegExpSplitCodeGenerator(vm), this), DontEnum | DontDelete | ReadOnly),
         GlobalPropertyInfo(vm.propertyNames->builtinNames().advanceStringIndexPrivateName(), JSFunction::createBuiltinFunction(vm, regExpPrototypeAdvanceStringIndexCodeGenerator(vm), this), DontEnum | DontDelete | ReadOnly),
         GlobalPropertyInfo(vm.propertyNames->builtinNames().regExpExecPrivateName(), JSFunction::createBuiltinFunction(vm, regExpPrototypeRegExpExecCodeGenerator(vm), this), DontEnum | DontDelete | ReadOnly),
+        GlobalPropertyInfo(vm.propertyNames->regExpMatchFastPrivateName, JSFunction::create(vm, this, 2, String(), regExpProtoFuncMatchFast), DontEnum | DontDelete | ReadOnly),
         GlobalPropertyInfo(vm.propertyNames->regExpSearchFastPrivateName, JSFunction::create(vm, this, 2, String(), regExpProtoFuncSearchFast), DontEnum | DontDelete | ReadOnly),
         GlobalPropertyInfo(vm.propertyNames->regExpSplitFastPrivateName, JSFunction::create(vm, this, 2, String(), regExpProtoFuncSplitFast), DontEnum | DontDelete | ReadOnly),
 
index e913ef4..f0dc2b6 100644 (file)
@@ -46,7 +46,6 @@ namespace JSC {
 
 static EncodedJSValue JSC_HOST_CALL regExpProtoFuncTest(ExecState*);
 static EncodedJSValue JSC_HOST_CALL regExpProtoFuncExec(ExecState*);
-static EncodedJSValue JSC_HOST_CALL regExpProtoFuncMatchPrivate(ExecState*);
 static EncodedJSValue JSC_HOST_CALL regExpProtoFuncCompile(ExecState*);
 static EncodedJSValue JSC_HOST_CALL regExpProtoFuncToString(ExecState*);
 static EncodedJSValue JSC_HOST_CALL regExpProtoGetterGlobal(ExecState*);
@@ -69,6 +68,7 @@ void RegExpPrototype::finishCreation(VM& vm, JSGlobalObject* globalObject)
     Base::finishCreation(vm);
     ASSERT(inherits(info()));
     JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->compile, regExpProtoFuncCompile, DontEnum, 2);
+    JSC_NATIVE_INTRINSIC_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->exec, regExpProtoFuncExec, DontEnum, 1, RegExpExecIntrinsic);
     JSC_NATIVE_INTRINSIC_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->test, regExpProtoFuncTest, DontEnum, 1, RegExpTestIntrinsic);
     JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->toString, regExpProtoFuncToString, DontEnum, 0);
     JSC_NATIVE_GETTER(vm.propertyNames->global, regExpProtoGetterGlobal, DontEnum | Accessor);
@@ -78,15 +78,10 @@ void RegExpPrototype::finishCreation(VM& vm, JSGlobalObject* globalObject)
     JSC_NATIVE_GETTER(vm.propertyNames->unicode, regExpProtoGetterUnicode, DontEnum | Accessor);
     JSC_NATIVE_GETTER(vm.propertyNames->source, regExpProtoGetterSource, DontEnum | Accessor);
     JSC_NATIVE_GETTER(vm.propertyNames->flags, regExpProtoGetterFlags, DontEnum | Accessor);
-    JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->builtinNames().matchPrivateName(), regExpProtoFuncMatchPrivate, DontEnum | DontDelete | ReadOnly, 1);
     JSC_BUILTIN_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->matchSymbol, regExpPrototypeMatchCodeGenerator, DontEnum);
     JSC_BUILTIN_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->searchSymbol, regExpPrototypeSearchCodeGenerator, DontEnum);
     JSC_BUILTIN_FUNCTION_WITHOUT_TRANSITION(vm.propertyNames->splitSymbol, regExpPrototypeSplitCodeGenerator, DontEnum);
 
-    JSFunction* execFunction = JSFunction::create(vm, globalObject, 1, vm.propertyNames->exec.string(), regExpProtoFuncExec, RegExpExecIntrinsic);
-    putDirectWithoutTransition(vm, vm.propertyNames->execPrivateName, execFunction, DontEnum | DontDelete | ReadOnly);
-    putDirectWithoutTransition(vm, vm.propertyNames->exec, execFunction, DontEnum);
-
     m_emptyRegExp.set(vm, this, RegExp::create(vm, "", NoFlags));
 }
 
@@ -123,7 +118,7 @@ EncodedJSValue JSC_HOST_CALL regExpProtoFuncExec(ExecState* exec)
     return JSValue::encode(asRegExpObject(thisValue)->exec(exec, exec->lexicalGlobalObject(), string));
 }
 
-EncodedJSValue JSC_HOST_CALL regExpProtoFuncMatchPrivate(ExecState* exec)
+EncodedJSValue JSC_HOST_CALL regExpProtoFuncMatchFast(ExecState* exec)
 {
     JSValue thisValue = exec->thisValue();
     if (!thisValue.inherits(RegExpObject::info()))
@@ -131,6 +126,8 @@ EncodedJSValue JSC_HOST_CALL regExpProtoFuncMatchPrivate(ExecState* exec)
     JSString* string = exec->argument(0).toStringOrNull(exec);
     if (!string)
         return JSValue::encode(jsUndefined());
+    if (!asRegExpObject(thisValue)->regExp()->global())
+        return JSValue::encode(asRegExpObject(thisValue)->exec(exec, exec->lexicalGlobalObject(), string));
     return JSValue::encode(asRegExpObject(thisValue)->matchGlobal(exec, exec->lexicalGlobalObject(), string));
 }
 
index d73eea3..56404f1 100644 (file)
@@ -58,6 +58,7 @@ private:
     WriteBarrier<RegExp> m_emptyRegExp;
 };
 
+EncodedJSValue JSC_HOST_CALL regExpProtoFuncMatchFast(ExecState*);
 EncodedJSValue JSC_HOST_CALL regExpProtoFuncSearchFast(ExecState*);
 EncodedJSValue JSC_HOST_CALL regExpProtoFuncSplitFast(ExecState*);