Skip to content

Commit

Permalink
deps: V8: cherry-pick 0dfd9ea51241
Browse files Browse the repository at this point in the history
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 <sigurds@chromium.org>
    Reviewed-by: Toon Verwaest <verwaest@chromium.org>
    Reviewed-by: Jakob Gruber <jgruber@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#64836}

Refs: v8/v8@0dfd9ea

PR-URL: #30713
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
bcoe authored and targos committed Dec 1, 2019
1 parent 93707c4 commit 0ba877a
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 1 deletion.
2 changes: 1 addition & 1 deletion common.gypi
Expand Up @@ -39,7 +39,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.22',
'v8_embedder_string': '-node.23',

##### V8 defaults for Node.js #####

Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/parsing/parser-base.h
Expand Up @@ -4108,6 +4108,7 @@ void ParserBase<Impl>::ParseFunctionBody(
inner_body.Rewind();
inner_body.Add(inner_block);
inner_block->set_scope(inner_scope);
impl()->RecordBlockSourceRange(inner_block, scope()->end_position());
if (!impl()->HasCheckedSyntax()) {
const AstRawString* conflict = inner_scope->FindVariableDeclaredIn(
function_scope, VariableMode::kLastLexicalVariableMode);
Expand Down
17 changes: 17 additions & 0 deletions deps/v8/test/mjsunit/code-coverage-block-async.js
Expand Up @@ -119,4 +119,21 @@ new Foo().timeout().next(); // 0400
{"start":184,"end":302,"count":0},
{"start":158,"end":182,"count":1}] );

TestCoverage(
"https://crbug.com/v8/9952",
`
async function test(foo) { // 0000
return {bar}; // 0050
// 0100
function bar() { // 0150
console.log("test"); // 0200
} // 0250
} // 0300
test().then(r => r.bar()); // 0350
%PerformMicrotaskCheckpoint(); // 0400`,
[{"start":0,"end":449,"count":1},
{"start":0,"end":301,"count":1},
{"start":152,"end":253,"count":1},
{"start":362,"end":374,"count":1}]);

%DebugToggleBlockCoverage(false);
29 changes: 29 additions & 0 deletions deps/v8/test/mjsunit/code-coverage-block.js
Expand Up @@ -1083,4 +1083,33 @@ TestCoverage(
{"start":16,"end":33,"count":0}]
);

TestCoverage(
"https://crbug.com/v8/9952",
`
function test(foo = "foodef") { // 0000
return {bar}; // 0050
// 0100
function bar() { // 0150
console.log("test"); // 0200
} // 0250
} // 0300
test().bar(); // 0350`,
[{"start":0,"end":399,"count":1},
{"start":0,"end":301,"count":1},
{"start":152,"end":253,"count":1}]);

TestCoverage(
"https://crbug.com/v8/9952",
`
function test(foo = (()=>{})) { // 0000
return {foo}; // 0050
} // 0100
// 0150
test(()=>{}).foo(); // 0200`,
[{"start":0,"end":249,"count":1},
{"start":0,"end":101,"count":1},
{"start":21,"end":27,"count":0},
{"start":205,"end":211,"count":1}]
);

%DebugToggleBlockCoverage(false);

0 comments on commit 0ba877a

Please sign in to comment.