Updates to regexp-match-indices tests based on d-flag #2934
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Data point: I have prototyped an implementation of regexp-match-indices with the /d flag in SpiderMonkey, but we are waiting on the updated test262 tests before shipping. It looks like V8 has already landed their implementation with /d (guarded behind a flag). |
|
WebKit/WebKit@7e63c11 JSC change is also ready. |
msaboff
previously approved these changes
Feb 19, 2021
There was a problem hiding this comment.
Looks mostly good. I think there are a couple of copy-paste errors as I commented below for tests test/built-ins/RegExp/match-indices/indices-groups-object-undefined.js and test/built-ins/RegExp/prototype/flags/coercion-hasIndices.js.
It also looks like there is some existing dead code in test/built-ins/RegExp/match-indices/indices-groups-properties.js
msaboff
reviewed
Feb 19, 2021
msaboff
reviewed
Feb 19, 2021
msaboff
reviewed
Feb 19, 2021
test/built-ins/RegExp/match-indices/indices-groups-properties.js
Outdated
Show resolved
Hide resolved
msaboff
reviewed
Feb 19, 2021
test/built-ins/RegExp/match-indices/indices-groups-properties.js
Outdated
Show resolved
Hide resolved
webkit-commit-queue
pushed a commit
to WebKit/WebKit
that referenced
this pull request
Feb 19, 2021
https://bugs.webkit.org/show_bug.cgi?id=222157 Reviewed by Yusuke Suzuki. JSTests: Added a new test to verify that all flag RegExp flag combinations work round tripping from flags to flag proerties and back. Added standalone versions of the updated regexp-match-indices from the PR tc39/test262#2934 as JSTest/stress test. Disabled the regexp-match-indices feature test in test262 until the pull request with updated tests land and we update the WebKit version. This is tracked in https://bugs.webkit.org/show_bug.cgi?id=222142. * stress/regexp-all-flags.js: Added. (flagsFromVariation): (setPropertiesForVariation): (missingPropertiesForVariation): (test.let.flagsSet.get call): (test): * stress/test262-indices-array-element.js: Added. (assertSameValue): * stress/test262-indices-array-matched.js: Added. (assertSameValue): (assertCompareArray): * stress/test262-indices-array-non-unicode-match.js: Added. (assertSameValue): (assertCompareArray): (assertDeepEqual): (verifyProperty): * stress/test262-indices-array-properties.js: Added. (verifyProperty): * stress/test262-indices-array-unicode-match.js: Added. (assertSameValue): (assertCompareArray): (assertDeepEqual): (verifyProperty): * stress/test262-indices-array-unicode-property-names.js: Added. (assertCompareArray): * stress/test262-indices-array-unmatched.js: Added. (assertSameValue): * stress/test262-indices-array.js: Added. (assert): (assertSameValue): * stress/test262-indices-groups-object-undefined.js: Added. (verifyProperty): * stress/test262-indices-groups-object-unmatched.js: Added. (assertSameValue): (assertCompareArray): * stress/test262-indices-groups-object.js: Added. (assertSameValue): (assertCompareArray): (verifyProperty): * stress/test262-indices-groups-properties.js: Added. (assertCompareArray): (verifyProperty): * stress/test262-indices-property.js: Added. (assertSameValue): (verifyProperty): * test262/config.yaml: Source/JavaScriptCore: When hasIndices is true, but there aren't any named groups, the spec says that we should create the indices.groups property is the value undefined. Increased the size of FlagsString to 7 plus terminater to account for the new 'd' flags. * runtime/RegExpMatchesArray.h: (JSC::createRegExpMatchesArray): * runtime/RegExpPrototype.cpp: Canonical link: https://commits.webkit.org/234355@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@273160 268f45cc-cd09-0410-ab3c-d52691b4dbfc
philn
pushed a commit
to philn/old-webkit
that referenced
this pull request
Feb 21, 2021
https://bugs.webkit.org/show_bug.cgi?id=222157 Reviewed by Yusuke Suzuki. JSTests: Added a new test to verify that all flag RegExp flag combinations work round tripping from flags to flag proerties and back. Added standalone versions of the updated regexp-match-indices from the PR tc39/test262#2934 as JSTest/stress test. Disabled the regexp-match-indices feature test in test262 until the pull request with updated tests land and we update the WebKit version. This is tracked in https://bugs.webkit.org/show_bug.cgi?id=222142. * stress/regexp-all-flags.js: Added. (flagsFromVariation): (setPropertiesForVariation): (missingPropertiesForVariation): (test.let.flagsSet.get call): (test): * stress/test262-indices-array-element.js: Added. (assertSameValue): * stress/test262-indices-array-matched.js: Added. (assertSameValue): (assertCompareArray): * stress/test262-indices-array-non-unicode-match.js: Added. (assertSameValue): (assertCompareArray): (assertDeepEqual): (verifyProperty): * stress/test262-indices-array-properties.js: Added. (verifyProperty): * stress/test262-indices-array-unicode-match.js: Added. (assertSameValue): (assertCompareArray): (assertDeepEqual): (verifyProperty): * stress/test262-indices-array-unicode-property-names.js: Added. (assertCompareArray): * stress/test262-indices-array-unmatched.js: Added. (assertSameValue): * stress/test262-indices-array.js: Added. (assert): (assertSameValue): * stress/test262-indices-groups-object-undefined.js: Added. (verifyProperty): * stress/test262-indices-groups-object-unmatched.js: Added. (assertSameValue): (assertCompareArray): * stress/test262-indices-groups-object.js: Added. (assertSameValue): (assertCompareArray): (verifyProperty): * stress/test262-indices-groups-properties.js: Added. (assertCompareArray): (verifyProperty): * stress/test262-indices-property.js: Added. (assertSameValue): (verifyProperty): * test262/config.yaml: Source/JavaScriptCore: When hasIndices is true, but there aren't any named groups, the spec says that we should create the indices.groups property is the value undefined. Increased the size of FlagsString to 7 plus terminater to account for the new 'd' flags. * runtime/RegExpMatchesArray.h: (JSC::createRegExpMatchesArray): * runtime/RegExpPrototype.cpp: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@273160 268f45cc-cd09-0410-ab3c-d52691b4dbfc
msaboff
previously approved these changes
Feb 22, 2021
|
FYI: I'll be putting up a PR against |
|
@rwaldron can you review and (hopefully) merge? |
|
ping: @rwaldron, @leobalter I'd appreciate a review, thanks. |
rwaldron
approved these changes
Feb 25, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updates based on the changes from tc39/proposal-regexp-match-indices#49
NOTE: Its not yet clear if there are any public implementations that have adopted this change as of yet.