-
Notifications
You must be signed in to change notification settings - Fork 31.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[v12.x backport] deps: V8: cherry-pick 0dfd9ea51241 #31412
Conversation
Original commit message: [coverage] Fix coverage with default arguments In the presence of default arguments, the body of the function gets wrapped into another block. This caused our trailing-range-after-return optimization to not apply, because the wrapper block had no source range assigned. This CL correctly assignes a source range to that block, which allows already present code to handle it correctly. Note that this is not a real coverage bug; we've just been reporting whitespace as uncovered. We're fixing it for consistency. Originally reported on github.com/bcoe/c8/issues/66 Bug: v8:9952 Change-Id: Iab3905f558eb99126e0dad8072d03d0a312fdcd3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1903430 Commit-Queue: Sigurd Schneider <[email protected]> Reviewed-by: Toon Verwaest <[email protected]> Reviewed-by: Jakob Gruber <[email protected]> Cr-Commit-Position: refs/heads/master@{#64836} Refs: v8/v8@0dfd9ea PR-URL: nodejs#30713 Backport-PR-URL: nodejs#31412 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rich Trott <[email protected]>
38d273c
to
dd8c833
Compare
@targos I believe this should be ready to go. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but @bcoe wondering why the change to code-coverage-block-async.js is not included?
@mhdawson in the rebase, it seems like something in the |
Original commit message: [coverage] Fix coverage with default arguments In the presence of default arguments, the body of the function gets wrapped into another block. This caused our trailing-range-after-return optimization to not apply, because the wrapper block had no source range assigned. This CL correctly assignes a source range to that block, which allows already present code to handle it correctly. Note that this is not a real coverage bug; we've just been reporting whitespace as uncovered. We're fixing it for consistency. Originally reported on github.com/bcoe/c8/issues/66 Bug: v8:9952 Change-Id: Iab3905f558eb99126e0dad8072d03d0a312fdcd3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1903430 Commit-Queue: Sigurd Schneider <[email protected]> Reviewed-by: Toon Verwaest <[email protected]> Reviewed-by: Jakob Gruber <[email protected]> Cr-Commit-Position: refs/heads/master@{#64836} Refs: v8/v8@0dfd9ea Backport-PR-URL: #31412 PR-URL: #30713 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rich Trott <[email protected]>
landed in 02d7283 |
Original commit message: [coverage] Fix coverage with default arguments In the presence of default arguments, the body of the function gets wrapped into another block. This caused our trailing-range-after-return optimization to not apply, because the wrapper block had no source range assigned. This CL correctly assignes a source range to that block, which allows already present code to handle it correctly. Note that this is not a real coverage bug; we've just been reporting whitespace as uncovered. We're fixing it for consistency. Originally reported on github.com/bcoe/c8/issues/66 Bug: v8:9952 Change-Id: Iab3905f558eb99126e0dad8072d03d0a312fdcd3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1903430 Commit-Queue: Sigurd Schneider <[email protected]> Reviewed-by: Toon Verwaest <[email protected]> Reviewed-by: Jakob Gruber <[email protected]> Cr-Commit-Position: refs/heads/master@{#64836} Refs: v8/v8@0dfd9ea Backport-PR-URL: #31412 PR-URL: #30713 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rich Trott <[email protected]>
Original commit message:
Refs: v8/v8@0dfd9ea
PR-URL: #30713
Reviewed-By: Michaël Zasso [email protected]
Reviewed-By: Colin Ihrig [email protected]
Reviewed-By: Jiawen Geng [email protected]
Reviewed-By: Rich Trott [email protected]
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes