This repository has been archived by the owner on Jan 25, 2022. It is now read-only.
Normative: Create a groups property unconditionally #40
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
If the RegExp does not have named groups, set a groups property of the result to undefined. This change is made to facilitate optimizing RegExp implementations, which often take a "fast path" for built-in, unmodified to not incur the overhead of the full subclassable semantics. If the "groups" property may be read from higher up on the prototype chain, the fast path for RegExp.prototype.replace would become more complex or slower, or alternatively, more conditions about the environment might need to be checked as a precondition to apply the fast path. An alternate approach would be to only read an own groups property based on a HasOwnProperty test followed by a Get. I don't see big advantages or disadvantages to that approach vs this one, and I'd be fine to revisit this patch if more differentiating factors are raised before Stage 4. Closes #34
|
cc @anba @schuay @bterlson @hashseed @mathiasbynens @tschneidereit @ljharb @getify @msaboff This patch still test262 needs tests and, ideally, implementations before Stage 4. Is anyone interested in picking that up? Otherwise, I could work on it in the coming weeks. |
|
LGTM. V8 tracking bug: https://bugs.chromium.org/p/v8/issues/detail?id=7192 |
kisg
pushed a commit
to paul99/v8mips
that referenced
this pull request
Dec 18, 2017
See tc39/proposal-regexp-named-groups#40. The spec is being changed to always create a 'groups' property on regexp result objects. Its value is undefined if no named captures exist, and the object containing named captures otherwise. Bug: v8:7192, v8:5437 Change-Id: I1fb00ffc186c7effd84b5692dcbed420581855c3 Reviewed-on: https://chromium-review.googlesource.com/829137 Reviewed-by: Yang Guo <[email protected]> Commit-Queue: Jakob Gruber <[email protected]> Cr-Commit-Position: refs/heads/master@{#50154}
mathiasbynens
added a commit
to mathiasbynens/test262
that referenced
this pull request
Dec 18, 2017
The `groups` property must be created unconditionally. tc39/proposal-regexp-named-groups#40
mathiasbynens
added a commit
to mathiasbynens/test262
that referenced
this pull request
Dec 19, 2017
The `groups` property must be created unconditionally. tc39/proposal-regexp-named-groups#40
mathiasbynens
added a commit
to mathiasbynens/test262
that referenced
this pull request
Dec 19, 2017
The `groups` property must be created unconditionally. tc39/proposal-regexp-named-groups#40
mathiasbynens
added a commit
to mathiasbynens/test262
that referenced
this pull request
Dec 20, 2017
The `groups` property must be created unconditionally. tc39/proposal-regexp-named-groups#40
rwaldron
pushed a commit
to tc39/test262
that referenced
this pull request
Dec 20, 2017
The `groups` property must be created unconditionally. tc39/proposal-regexp-named-groups#40
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
If the RegExp does not have named groups, set a groups property
of the result to undefined. This change is made to facilitate
optimizing RegExp implementations, which often take a "fast path"
for built-in, unmodified to not incur the overhead of the full
subclassable semantics. If the "groups" property may be read
from higher up on the prototype chain, the fast path for
RegExp.prototype.replace would become more complex or slower, or
alternatively, more conditions about the environment might need
to be checked as a precondition to apply the fast path.
An alternate approach would be to only read an own groups property
based on a HasOwnProperty test followed by a Get. I don't see big
advantages or disadvantages to that approach vs this one, and I'd
be fine to revisit this patch if more differentiating factors
are raised before Stage 4.
Closes #34