diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog index 136f55566f829..e1f3bf7a7ac76 100644 --- a/JSTests/ChangeLog +++ b/JSTests/ChangeLog @@ -1,3 +1,12 @@ +2019-11-12 Alexey Shvayka + + 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 UTC offset for Samoa is miscalculated when !HAVE(TIMEGM) diff --git a/JSTests/test262/expectations.yaml b/JSTests/test262/expectations.yaml index 359ec8ed615df..96844d148b85a 100644 --- a/JSTests/test262/expectations.yaml +++ b/JSTests/test262/expectations.yaml @@ -1183,12 +1183,6 @@ test/built-ins/RegExp/named-groups/groups-object-subclass-sans.js: test/built-ins/RegExp/named-groups/groups-object-subclass.js: default: 'Test262Error: Expected SameValue(«b», «$») to be true' strict mode: 'Test262Error: Expected SameValue(«b», «$») 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(«$$cd», «$<$cd») to be true' strict mode: 'Test262Error: Expected SameValue(«$$cd», «$<$cd») to be true' diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog index 51ecf442e7b04..d9e6a4a17a794 100644 --- a/Source/JavaScriptCore/ChangeLog +++ b/Source/JavaScriptCore/ChangeLog @@ -1,3 +1,38 @@ +2019-11-12 Alexey Shvayka + + 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::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 Unreviewed, fix incorrect assertion diff --git a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h index 41ca366385301..32f801adc482d 100644 --- a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h +++ b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h @@ -2356,7 +2356,6 @@ bool AbstractInterpreter::executeEffects(unsigned clobberLimi 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; diff --git a/Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp b/Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp index 1a8f72d7f773f..143eb4624495b 100644 --- a/Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp +++ b/Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp @@ -606,12 +606,7 @@ class StrengthReductionPhase : public Phase { 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) diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp index 29bc62d6d9f63..bc83b71c3bcb3 100644 --- a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp +++ b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp @@ -708,7 +708,6 @@ void JSGlobalObject::init(VM& vm) 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& init) { @@ -1557,8 +1556,6 @@ void JSGlobalObject::fireWatchpointAndMakeAllArrayStructuresSlowPut(VM& vm) 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 @@ void JSGlobalObject::visitChildren(JSCell* cell, SlotVisitor& visitor) 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); diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.h b/Source/JavaScriptCore/runtime/JSGlobalObject.h index 1efdf7f5410c8..779e3e953c643 100644 --- a/Source/JavaScriptCore/runtime/JSGlobalObject.h +++ b/Source/JavaScriptCore/runtime/JSGlobalObject.h @@ -376,7 +376,6 @@ class JSGlobalObject : public JSSegmentedVariableObject { WriteBarrier m_asyncGeneratorStructure; LazyProperty m_iteratorResultObjectStructure; WriteBarrier m_regExpMatchesArrayStructure; - WriteBarrier m_regExpMatchesArrayWithGroupsStructure; LazyProperty m_moduleRecordStructure; LazyProperty m_moduleNamespaceObjectStructure; LazyProperty m_proxyObjectStructure; @@ -754,7 +753,6 @@ class JSGlobalObject : public JSSegmentedVariableObject { 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); } diff --git a/Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp b/Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp index c42be7f2a98da..fe76c00b99991 100644 --- a/Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp +++ b/Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp @@ -70,9 +70,7 @@ JSArray* createEmptyRegExpMatchesArray(JSGlobalObject* globalObject, JSString* i 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 @@ static Structure* createStructureImpl(VM& vm, JSGlobalObject* globalObject, Inde 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 @@ Structure* createRegExpMatchesArraySlowPutStructure(VM& vm, JSGlobalObject* glob 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 diff --git a/Source/JavaScriptCore/runtime/RegExpMatchesArray.h b/Source/JavaScriptCore/runtime/RegExpMatchesArray.h index e9f61313c5241..9581c14de1dcb 100644 --- a/Source/JavaScriptCore/runtime/RegExpMatchesArray.h +++ b/Source/JavaScriptCore/runtime/RegExpMatchesArray.h @@ -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 @@ ALWAYS_INLINE JSArray* createRegExpMatchesArray( 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 @@ ALWAYS_INLINE JSArray* createRegExpMatchesArray( // 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 @@ inline JSArray* createRegExpMatchesArray(JSGlobalObject* globalObject, JSString* JSArray* createEmptyRegExpMatchesArray(JSGlobalObject*, JSString*, RegExp*); Structure* createRegExpMatchesArrayStructure(VM&, JSGlobalObject*); Structure* createRegExpMatchesArraySlowPutStructure(VM&, JSGlobalObject*); -Structure* createRegExpMatchesArrayWithGroupsStructure(VM&, JSGlobalObject*); -Structure* createRegExpMatchesArrayWithGroupsSlowPutStructure(VM&, JSGlobalObject*); } // namespace JSC diff --git a/Source/JavaScriptCore/runtime/StringPrototype.cpp b/Source/JavaScriptCore/runtime/StringPrototype.cpp index ed7f5c8cf992b..372fd3c5b9697 100644 --- a/Source/JavaScriptCore/runtime/StringPrototype.cpp +++ b/Source/JavaScriptCore/runtime/StringPrototype.cpp @@ -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,11 +572,7 @@ static ALWAYS_INLINE JSString* replaceUsingRegExpSearch( OUT_OF_MEMORY(globalObject, scope); cachedCall.clearArguments(); - - JSObject* groups = nullptr; - - if (hasNamedCaptures) - groups = JSFinalObject::create(vm, JSFinalObject::createStructure(vm, globalObject, globalObject->objectPrototype(), 0)); + JSObject* groups = hasNamedCaptures ? constructEmptyObject(vm, globalObject->nullPrototypeObjectStructure()) : nullptr; for (unsigned i = 0; i < regExp->numSubpatterns() + 1; ++i) { int matchStart = ovector[i * 2]; @@ -636,10 +633,7 @@ static ALWAYS_INLINE JSString* replaceUsingRegExpSearch( OUT_OF_MEMORY(globalObject, scope); MarkedArgumentBuffer args; - JSObject* groups = nullptr; - - if (hasNamedCaptures) - groups = JSFinalObject::create(vm, JSFinalObject::createStructure(vm, globalObject, globalObject->objectPrototype(), 0)); + JSObject* groups = hasNamedCaptures ? constructEmptyObject(vm, globalObject->nullPrototypeObjectStructure()) : nullptr; for (unsigned i = 0; i < regExp->numSubpatterns() + 1; ++i) { int matchStart = ovector[i * 2];