Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2540153002:
[regexp] Refactor RegExp.prototype.exec (Closed)
|
Created:
4 years ago by jgruber Modified:
4 years ago Reviewers:
Igor Sheludko CC:
v8-reviews_googlegroups.com Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[regexp] Refactor RegExp.prototype.exec
This refactors portions of exec into a new function without RegExpResult
construction, which will be used in the future by test, @@match, and @@search
fast paths.
Unnecessary ToString and ToLength calls as well as repeated map checks were
removed.
BUG=v8:5339
Committed: https://crrev.com/65b2ab90ff1cbc90f73568c7ec9d72cb760ac6c8
Cr-Commit-Position: refs/heads/master@{#41414}
Patch Set 1 #Patch Set 2 : Assert String/RegExp #
Total comments: 4
Patch Set 3 : Address comments #
Dependent Patchsets: Messages
Total messages: 20 (13 generated)
The CQ bit was checked by jgruber@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by jgruber@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== [regexp] Refactor RegExp.prototype.exec This refactors portions of exec into a new function without RegExpResult construction, which will be used in the future by test, @@match, and @@search fast paths. Useless ToString and ToLength calls were removed. BUG=v8:5339 ========== to ========== [regexp] Refactor RegExp.prototype.exec This refactors portions of exec into a new function without RegExpResult construction, which will be used in the future by test, @@match, and @@search fast paths. Unnecessary ToString and ToLength calls as well as repeated map checks were removed. BUG=v8:5339 ==========
jgruber@chromium.org changed reviewers: + ishell@chromium.org
Nice cleanup! lgtm https://codereview.chromium.org/2540153002/diff/20001/src/builtins/builtins-r... File src/builtins/builtins-regexp.cc (right): https://codereview.chromium.org/2540153002/diff/20001/src/builtins/builtins-r... src/builtins/builtins-regexp.cc:300: Node* Generate_RegExpPrototypeExecBodyWithoutResult( Please add a comment (especially about what does this function return).
https://codereview.chromium.org/2540153002/diff/20001/src/builtins/builtins-r... File src/builtins/builtins-regexp.cc (right): https://codereview.chromium.org/2540153002/diff/20001/src/builtins/builtins-r... src/builtins/builtins-regexp.cc:427: Node* Generate_RegExpPrototypeExecBody(CodeStubAssembler* a, I think you can drop the Generate_ prefix for readability here and above.
https://codereview.chromium.org/2540153002/diff/20001/src/builtins/builtins-r... File src/builtins/builtins-regexp.cc (right): https://codereview.chromium.org/2540153002/diff/20001/src/builtins/builtins-r... src/builtins/builtins-regexp.cc:300: Node* Generate_RegExpPrototypeExecBodyWithoutResult( On 2016/11/30 22:04:21, Igor Sheludko wrote: > Please add a comment (especially about what does this function return). Done. https://codereview.chromium.org/2540153002/diff/20001/src/builtins/builtins-r... src/builtins/builtins-regexp.cc:427: Node* Generate_RegExpPrototypeExecBody(CodeStubAssembler* a, On 2016/11/30 22:12:08, Igor Sheludko wrote: > I think you can drop the Generate_ prefix for readability here and above. Done.
The CQ bit was checked by jgruber@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from ishell@chromium.org Link to the patchset: https://codereview.chromium.org/2540153002/#ps40001 (title: "Address comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1480583352485670,
"parent_rev": "324a514eb31605585c7900b6d25fe8fbb45f7a10", "commit_rev":
"b05c11193e8a7faf37b39712ce95d5fb6c440698"}
Message was sent while issue was closed.
Description was changed from ========== [regexp] Refactor RegExp.prototype.exec This refactors portions of exec into a new function without RegExpResult construction, which will be used in the future by test, @@match, and @@search fast paths. Unnecessary ToString and ToLength calls as well as repeated map checks were removed. BUG=v8:5339 ========== to ========== [regexp] Refactor RegExp.prototype.exec This refactors portions of exec into a new function without RegExpResult construction, which will be used in the future by test, @@match, and @@search fast paths. Unnecessary ToString and ToLength calls as well as repeated map checks were removed. BUG=v8:5339 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== [regexp] Refactor RegExp.prototype.exec This refactors portions of exec into a new function without RegExpResult construction, which will be used in the future by test, @@match, and @@search fast paths. Unnecessary ToString and ToLength calls as well as repeated map checks were removed. BUG=v8:5339 ========== to ========== [regexp] Refactor RegExp.prototype.exec This refactors portions of exec into a new function without RegExpResult construction, which will be used in the future by test, @@match, and @@search fast paths. Unnecessary ToString and ToLength calls as well as repeated map checks were removed. BUG=v8:5339 Committed: https://crrev.com/65b2ab90ff1cbc90f73568c7ec9d72cb760ac6c8 Cr-Commit-Position: refs/heads/master@{#41414} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/65b2ab90ff1cbc90f73568c7ec9d72cb760ac6c8 Cr-Commit-Position: refs/heads/master@{#41414} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
