Title: [252374] trunk
Revision
252374
Author
commit-qu...@webkit.org
Date
2019-11-12 14:09:37 -0800 (Tue, 12 Nov 2019)

Log Message

RegExpBuiltinExec should create "groups" property unconditionally
https://bugs.webkit.org/show_bug.cgi?id=204067

Patch by Alexey Shvayka <shvaikal...@gmail.com> on 2019-11-12
Reviewed by Ross Kirsling.

JSTests:

* test262/expectations.yaml: Mark 4 test cases as passing.

Source/_javascript_Core:

After RegExp named capture groups were initially implemented in JSC, the spec was changed
to unconditionally create "groups" property.
(https://github.com/tc39/proposal-regexp-named-groups/issues/34)

This patch implements the change (that was shipped by V8), reducing number of structures
we use for RegExpMatchesArray, and also sets [[Prototype]] of "groups" object to `null`.
(step 24 of https://tc39.es/ecma262/#sec-regexpbuiltinexec)

* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGStrengthReductionPhase.cpp:
(JSC::DFG::StrengthReductionPhase::handleNode):
* runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::init):
(JSC::JSGlobalObject::fireWatchpointAndMakeAllArrayStructuresSlowPut):
(JSC::JSGlobalObject::visitChildren):
* runtime/JSGlobalObject.h:
(JSC::JSGlobalObject::regExpMatchesArrayStructure const):
(JSC::JSGlobalObject::regExpMatchesArrayWithGroupsStructure const): Deleted.
* runtime/RegExpMatchesArray.cpp:
(JSC::createStructureImpl):
(JSC::createRegExpMatchesArrayWithGroupsStructure): Deleted.
(JSC::createRegExpMatchesArrayWithGroupsSlowPutStructure): Deleted.
* runtime/RegExpMatchesArray.h:
(JSC::createRegExpMatchesArray):
* runtime/StringPrototype.cpp:
(JSC::replaceUsingRegExpSearch):

Modified Paths

Diff

Modified: trunk/JSTests/ChangeLog (252373 => 252374)


--- trunk/JSTests/ChangeLog	2019-11-12 19:49:29 UTC (rev 252373)
+++ trunk/JSTests/ChangeLog	2019-11-12 22:09:37 UTC (rev 252374)
@@ -1,3 +1,12 @@
+2019-11-12  Alexey Shvayka  <shvaikal...@gmail.com>
+
+        RegExpBuiltinExec should create "groups" property unconditionally
+        https://bugs.webkit.org/show_bug.cgi?id=204067
+
+        Reviewed by Ross Kirsling.
+
+        * test262/expectations.yaml: Mark 4 test cases as passing.
+
 2019-11-11  Ross Kirsling  <ross.kirsl...@sony.com>
 
         UTC offset for Samoa is miscalculated when !HAVE(TIMEGM)

Modified: trunk/JSTests/test262/expectations.yaml (252373 => 252374)


--- trunk/JSTests/test262/expectations.yaml	2019-11-12 19:49:29 UTC (rev 252373)
+++ trunk/JSTests/test262/expectations.yaml	2019-11-12 22:09:37 UTC (rev 252374)
@@ -1183,12 +1183,6 @@
 test/built-ins/RegExp/named-groups/groups-object-subclass.js:
   default: 'Test262Error: Expected SameValue(«b», «$<a>») to be true'
   strict mode: 'Test262Error: Expected SameValue(«b», «$<a>») to be true'
-test/built-ins/RegExp/named-groups/groups-object-undefined.js:
-  default: 'Test262Error: Expected true but got false'
-  strict mode: 'Test262Error: Expected true but got false'
-test/built-ins/RegExp/named-groups/groups-object.js:
-  default: 'Test262Error: Expected SameValue(«null», «[object Object]») to be true'
-  strict mode: 'Test262Error: Expected SameValue(«null», «[object Object]») to be true'
 test/built-ins/RegExp/named-groups/string-replace-nocaptures.js:
   default: 'Test262Error: Expected SameValue(«$<snd>$<fst>cd», «$<$<fst>cd») to be true'
   strict mode: 'Test262Error: Expected SameValue(«$<snd>$<fst>cd», «$<$<fst>cd») to be true'

Modified: trunk/Source/_javascript_Core/ChangeLog (252373 => 252374)


--- trunk/Source/_javascript_Core/ChangeLog	2019-11-12 19:49:29 UTC (rev 252373)
+++ trunk/Source/_javascript_Core/ChangeLog	2019-11-12 22:09:37 UTC (rev 252374)
@@ -1,3 +1,38 @@
+2019-11-12  Alexey Shvayka  <shvaikal...@gmail.com>
+
+        RegExpBuiltinExec should create "groups" property unconditionally
+        https://bugs.webkit.org/show_bug.cgi?id=204067
+
+        Reviewed by Ross Kirsling.
+
+        After RegExp named capture groups were initially implemented in JSC, the spec was changed
+        to unconditionally create "groups" property.
+        (https://github.com/tc39/proposal-regexp-named-groups/issues/34)
+
+        This patch implements the change (that was shipped by V8), reducing number of structures
+        we use for RegExpMatchesArray, and also sets [[Prototype]] of "groups" object to `null`.
+        (step 24 of https://tc39.es/ecma262/#sec-regexpbuiltinexec)
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGStrengthReductionPhase.cpp:
+        (JSC::DFG::StrengthReductionPhase::handleNode):
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::init):
+        (JSC::JSGlobalObject::fireWatchpointAndMakeAllArrayStructuresSlowPut):
+        (JSC::JSGlobalObject::visitChildren):
+        * runtime/JSGlobalObject.h:
+        (JSC::JSGlobalObject::regExpMatchesArrayStructure const):
+        (JSC::JSGlobalObject::regExpMatchesArrayWithGroupsStructure const): Deleted.
+        * runtime/RegExpMatchesArray.cpp:
+        (JSC::createStructureImpl):
+        (JSC::createRegExpMatchesArrayWithGroupsStructure): Deleted.
+        (JSC::createRegExpMatchesArrayWithGroupsSlowPutStructure): Deleted.
+        * runtime/RegExpMatchesArray.h:
+        (JSC::createRegExpMatchesArray):
+        * runtime/StringPrototype.cpp:
+        (JSC::replaceUsingRegExpSearch):
+
 2019-11-11  Yusuke Suzuki  <ysuz...@apple.com>
 
         Unreviewed, fix incorrect assertion

Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (252373 => 252374)


--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2019-11-12 19:49:29 UTC (rev 252373)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2019-11-12 22:09:37 UTC (rev 252374)
@@ -2356,7 +2356,6 @@
                     m_graph.watchpoints().addLazily(globalObject->havingABadTimeWatchpoint());
                     RegisteredStructureSet structureSet;
                     structureSet.add(m_graph.registerStructure(globalObject->regExpMatchesArrayStructure()));
-                    structureSet.add(m_graph.registerStructure(globalObject->regExpMatchesArrayWithGroupsStructure()));
                     setForNode(node, structureSet);
                     forNode(node).merge(SpecOther);
                     break;

Modified: trunk/Source/_javascript_Core/dfg/DFGStrengthReductionPhase.cpp (252373 => 252374)


--- trunk/Source/_javascript_Core/dfg/DFGStrengthReductionPhase.cpp	2019-11-12 19:49:29 UTC (rev 252373)
+++ trunk/Source/_javascript_Core/dfg/DFGStrengthReductionPhase.cpp	2019-11-12 22:09:37 UTC (rev 252374)
@@ -606,12 +606,7 @@
 
                 m_graph.watchpoints().addLazily(globalObject->havingABadTimeWatchpoint());
 
-                Structure* structure;
-                if ((m_node->op() == RegExpExec || m_node->op() == RegExpExecNonGlobalOrSticky) && regExp->hasNamedCaptures())
-                    structure = globalObject->regExpMatchesArrayWithGroupsStructure();
-                else
-                    structure = globalObject->regExpMatchesArrayStructure();
-
+                Structure* structure = globalObject->regExpMatchesArrayStructure();
                 if (structure->indexingType() != ArrayWithContiguous) {
                     // This is further protection against a race with haveABadTime.
                     if (verbose)

Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp (252373 => 252374)


--- trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp	2019-11-12 19:49:29 UTC (rev 252373)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp	2019-11-12 22:09:37 UTC (rev 252374)
@@ -708,7 +708,6 @@
     m_regExpPrototype.set(vm, this, RegExpPrototype::create(vm, this, RegExpPrototype::createStructure(vm, this, m_objectPrototype.get())));
     m_regExpStructure.set(vm, this, RegExpObject::createStructure(vm, this, m_regExpPrototype.get()));
     m_regExpMatchesArrayStructure.set(vm, this, createRegExpMatchesArrayStructure(vm, this));
-    m_regExpMatchesArrayWithGroupsStructure.set(vm, this, createRegExpMatchesArrayWithGroupsStructure(vm, this));
 
     m_moduleRecordStructure.initLater(
         [] (const Initializer<Structure>& init) {
@@ -1557,8 +1556,6 @@
     Structure* slowPutStructure;
     slowPutStructure = createRegExpMatchesArraySlowPutStructure(vm, this);
     m_regExpMatchesArrayStructure.set(vm, this, slowPutStructure);
-    slowPutStructure = createRegExpMatchesArrayWithGroupsSlowPutStructure(vm, this);
-    m_regExpMatchesArrayWithGroupsStructure.set(vm, this, slowPutStructure);
     slowPutStructure = ClonedArguments::createSlowPutStructure(vm, this, m_objectPrototype.get());
     m_clonedArgumentsStructure.set(vm, this, slowPutStructure);
 
@@ -1818,7 +1815,6 @@
     visitor.append(thisObject->m_asyncGeneratorStructure);
     thisObject->m_iteratorResultObjectStructure.visit(visitor);
     visitor.append(thisObject->m_regExpMatchesArrayStructure);
-    visitor.append(thisObject->m_regExpMatchesArrayWithGroupsStructure);
     thisObject->m_moduleRecordStructure.visit(visitor);
     thisObject->m_moduleNamespaceObjectStructure.visit(visitor);
     thisObject->m_proxyObjectStructure.visit(visitor);

Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObject.h (252373 => 252374)


--- trunk/Source/_javascript_Core/runtime/JSGlobalObject.h	2019-11-12 19:49:29 UTC (rev 252373)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObject.h	2019-11-12 22:09:37 UTC (rev 252374)
@@ -376,7 +376,6 @@
     WriteBarrier<Structure> m_asyncGeneratorStructure;
     LazyProperty<JSGlobalObject, Structure> m_iteratorResultObjectStructure;
     WriteBarrier<Structure> m_regExpMatchesArrayStructure;
-    WriteBarrier<Structure> m_regExpMatchesArrayWithGroupsStructure;
     LazyProperty<JSGlobalObject, Structure> m_moduleRecordStructure;
     LazyProperty<JSGlobalObject, Structure> m_moduleNamespaceObjectStructure;
     LazyProperty<JSGlobalObject, Structure> m_proxyObjectStructure;
@@ -754,7 +753,6 @@
     Structure* stringObjectStructure() const { return m_stringObjectStructure.get(); }
     Structure* iteratorResultObjectStructure() const { return m_iteratorResultObjectStructure.get(this); }
     Structure* regExpMatchesArrayStructure() const { return m_regExpMatchesArrayStructure.get(); }
-    Structure* regExpMatchesArrayWithGroupsStructure() const { return m_regExpMatchesArrayWithGroupsStructure.get(); }
     Structure* moduleRecordStructure() const { return m_moduleRecordStructure.get(this); }
     Structure* moduleNamespaceObjectStructure() const { return m_moduleNamespaceObjectStructure.get(this); }
     Structure* proxyObjectStructure() const { return m_proxyObjectStructure.get(this); }

Modified: trunk/Source/_javascript_Core/runtime/RegExpMatchesArray.cpp (252373 => 252374)


--- trunk/Source/_javascript_Core/runtime/RegExpMatchesArray.cpp	2019-11-12 19:49:29 UTC (rev 252373)
+++ trunk/Source/_javascript_Core/runtime/RegExpMatchesArray.cpp	2019-11-12 22:09:37 UTC (rev 252374)
@@ -70,9 +70,7 @@
     return array;
 }
 
-enum class ShouldCreateGroups { No, Yes };
-
-static Structure* createStructureImpl(VM& vm, JSGlobalObject* globalObject, IndexingType indexingType, ShouldCreateGroups shouldCreateGroups = ShouldCreateGroups::No)
+static Structure* createStructureImpl(VM& vm, JSGlobalObject* globalObject, IndexingType indexingType)
 {
     Structure* structure = globalObject->arrayStructureForIndexingTypeDuringAllocation(indexingType);
     PropertyOffset offset;
@@ -80,14 +78,8 @@
     ASSERT(offset == RegExpMatchesArrayIndexPropertyOffset);
     structure = Structure::addPropertyTransition(vm, structure, vm.propertyNames->input, 0, offset);
     ASSERT(offset == RegExpMatchesArrayInputPropertyOffset);
-    switch (shouldCreateGroups) {
-    case ShouldCreateGroups::Yes:
-        structure = Structure::addPropertyTransition(vm, structure, vm.propertyNames->groups, 0, offset);
-        ASSERT(offset == RegExpMatchesArrayGroupsPropertyOffset);
-        break;
-    case ShouldCreateGroups::No:
-        break;
-    }
+    structure = Structure::addPropertyTransition(vm, structure, vm.propertyNames->groups, 0, offset);
+    ASSERT(offset == RegExpMatchesArrayGroupsPropertyOffset);
     return structure;
 }
 
@@ -101,14 +93,4 @@
     return createStructureImpl(vm, globalObject, ArrayWithSlowPutArrayStorage);
 }
 
-Structure* createRegExpMatchesArrayWithGroupsStructure(VM& vm, JSGlobalObject* globalObject)
-{
-    return createStructureImpl(vm, globalObject, ArrayWithContiguous, ShouldCreateGroups::Yes);
-}
-
-Structure* createRegExpMatchesArrayWithGroupsSlowPutStructure(VM& vm, JSGlobalObject* globalObject)
-{
-    return createStructureImpl(vm, globalObject, ArrayWithSlowPutArrayStorage, ShouldCreateGroups::Yes);
-}
-
 } // namespace JSC

Modified: trunk/Source/_javascript_Core/runtime/RegExpMatchesArray.h (252373 => 252374)


--- trunk/Source/_javascript_Core/runtime/RegExpMatchesArray.h	2019-11-12 19:49:29 UTC (rev 252373)
+++ trunk/Source/_javascript_Core/runtime/RegExpMatchesArray.h	2019-11-12 22:09:37 UTC (rev 252374)
@@ -24,6 +24,7 @@
 #include "JSArray.h"
 #include "JSCInlines.h"
 #include "JSGlobalObject.h"
+#include "ObjectConstructor.h"
 #include "RegExpInlines.h"
 #include "RegExpObject.h"
 
@@ -82,18 +83,13 @@
     
     unsigned numSubpatterns = regExp->numSubpatterns();
     bool hasNamedCaptures = regExp->hasNamedCaptures();
-    JSObject* groups = nullptr;
+    JSObject* groups = hasNamedCaptures ? constructEmptyObject(vm, globalObject->nullPrototypeObjectStructure()) : nullptr;
     Structure* matchStructure = globalObject->regExpMatchesArrayStructure();
-    if (hasNamedCaptures) {
-        groups = JSFinalObject::create(vm, JSFinalObject::createStructure(vm, globalObject, globalObject->objectPrototype(), 0));
-        matchStructure = globalObject->regExpMatchesArrayWithGroupsStructure();
-    }
 
     auto setProperties = [&] () {
         array->putDirect(vm, RegExpMatchesArrayIndexPropertyOffset, jsNumber(result.start));
         array->putDirect(vm, RegExpMatchesArrayInputPropertyOffset, input);
-        if (hasNamedCaptures)
-            array->putDirect(vm, RegExpMatchesArrayGroupsPropertyOffset, groups);
+        array->putDirect(vm, RegExpMatchesArrayGroupsPropertyOffset, hasNamedCaptures ? groups : jsUndefined());
 
         ASSERT(!array->butterfly()->indexingHeader()->preCapacity(matchStructure));
         auto capacity = matchStructure->outOfLineCapacity();
@@ -155,7 +151,7 @@
 
     // We initialize the groups object late as it could allocate, which with the current API could cause
     // allocations.
-    if (groups) {
+    if (hasNamedCaptures) {
         for (unsigned i = 1; i <= numSubpatterns; ++i) {
             String groupName = regExp->getCaptureGroupName(i);
             if (!groupName.isEmpty())
@@ -179,7 +175,5 @@
 JSArray* createEmptyRegExpMatchesArray(JSGlobalObject*, JSString*, RegExp*);
 Structure* createRegExpMatchesArrayStructure(VM&, JSGlobalObject*);
 Structure* createRegExpMatchesArraySlowPutStructure(VM&, JSGlobalObject*);
-Structure* createRegExpMatchesArrayWithGroupsStructure(VM&, JSGlobalObject*);
-Structure* createRegExpMatchesArrayWithGroupsSlowPutStructure(VM&, JSGlobalObject*);
 
 } // namespace JSC

Modified: trunk/Source/_javascript_Core/runtime/StringPrototype.cpp (252373 => 252374)


--- trunk/Source/_javascript_Core/runtime/StringPrototype.cpp	2019-11-12 19:49:29 UTC (rev 252373)
+++ trunk/Source/_javascript_Core/runtime/StringPrototype.cpp	2019-11-12 22:09:37 UTC (rev 252374)
@@ -39,6 +39,7 @@
 #include "JSGlobalObjectFunctions.h"
 #include "JSStringIterator.h"
 #include "Lookup.h"
+#include "ObjectConstructor.h"
 #include "ObjectPrototype.h"
 #include "ParseInt.h"
 #include "PropertyNameArray.h"
@@ -571,12 +572,8 @@
                 OUT_OF_MEMORY(globalObject, scope);
 
             cachedCall.clearArguments();
+            JSObject* groups = hasNamedCaptures ? constructEmptyObject(vm, globalObject->nullPrototypeObjectStructure()) : nullptr;
 
-            JSObject* groups = nullptr;
-
-            if (hasNamedCaptures)
-                groups = JSFinalObject::create(vm, JSFinalObject::createStructure(vm, globalObject, globalObject->objectPrototype(), 0));
-
             for (unsigned i = 0; i < regExp->numSubpatterns() + 1; ++i) {
                 int matchStart = ovector[i * 2];
                 int matchLen = ovector[i * 2 + 1] - matchStart;
@@ -636,11 +633,8 @@
                     OUT_OF_MEMORY(globalObject, scope);
 
                 MarkedArgumentBuffer args;
-                JSObject* groups = nullptr;
+                JSObject* groups = hasNamedCaptures ? constructEmptyObject(vm, globalObject->nullPrototypeObjectStructure()) : nullptr;
 
-                if (hasNamedCaptures)
-                    groups = JSFinalObject::create(vm, JSFinalObject::createStructure(vm, globalObject, globalObject->objectPrototype(), 0));
-
                 for (unsigned i = 0; i < regExp->numSubpatterns() + 1; ++i) {
                     int matchStart = ovector[i * 2];
                     int matchLen = ovector[i * 2 + 1] - matchStart;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to