deps: v8, backport 2d08967

Original commit message: [coverage] Extend SourceRangeAstVisitor for throw statements The SourceRangeAstVisitor has custom logic for blocks ending with a statement that has a continuation range. In these cases, the trailing continuation is removed which makes the reported coverage ranges a bit nicer. throw Error('foo') consists of an ExpressionStatement, with a Throw expression stored within the statement. The source range itself is stored with the Throw, not the statement. We now properly extract the correct AST node for trailing throw statements. [email protected], [email protected], [email protected] Bug: v8:8691 Change-Id: Ibcbab79fbe54719a8993045040349c863b139011 Reviewed-on: https://chromium-review.googlesource.com/c/1480632 Commit-Queue: Georg Neis <[email protected]> Reviewed-by: Georg Neis <[email protected]> Reviewed-by: Jakob Gruber <[email protected]> Cr-Commit-Position: refs/heads/master@{#59936} Refs: v8/v8@2d08967 PR-URL: #26413 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Mar 5, 2019 · c78788a · c78788a
1 parent d4fdec6
commit c78788a
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 9 deletions.
diff --git a/deps/v8/AUTHORS b/deps/v8/AUTHORS
@@ -50,7 +50,7 @@ Andrew Paprocki <[email protected]>
 Andrei Kashcha <[email protected]>
 Anna Henningsen <[email protected]>
 Bangfu Tao <[email protected]>
-Ben Coe <ben@npmjs.com>
+Ben Coe <bencoe@gmail.com>
 Ben Newman <[email protected]>
 Ben Noordhuis <[email protected]>
 Benjamin Tan <[email protected]>

diff --git a/deps/v8/src/ast/source-range-ast-visitor.cc b/deps/v8/src/ast/source-range-ast-visitor.cc
@@ -56,7 +56,18 @@ void SourceRangeAstVisitor::MaybeRemoveLastContinuationRange(
   if (statements == nullptr || statements->is_empty()) return;
 
   Statement* last_statement = statements->last();
-  AstNodeSourceRanges* last_range = source_range_map_->Find(last_statement);
+  AstNodeSourceRanges* last_range = nullptr;
+
+  if (last_statement->IsExpressionStatement() &&
+      last_statement->AsExpressionStatement()->expression()->IsThrow()) {
+    // For ThrowStatement, source range is tied to Throw expression not
+    // ExpressionStatement.
+    last_range = source_range_map_->Find(
+        last_statement->AsExpressionStatement()->expression());
+  } else {
+    last_range = source_range_map_->Find(last_statement);
+  }
+
   if (last_range == nullptr) return;
 
   if (last_range->HasRange(SourceRangeKind::kContinuation)) {

diff --git a/deps/v8/test/mjsunit/code-coverage-block.js b/deps/v8/test/mjsunit/code-coverage-block.js
@@ -353,11 +353,11 @@ TestCoverage(
 [{"start":0,"end":849,"count":1},
  {"start":1,"end":801,"count":1},
  {"start":67,"end":87,"count":0},
- {"start":219,"end":222,"count":0},
+ {"start":221,"end":222,"count":0},
  {"start":254,"end":274,"count":0},
- {"start":369,"end":372,"count":0},
+ {"start":371,"end":372,"count":0},
  {"start":403,"end":404,"count":0},
- {"start":513,"end":554,"count":0}]
+ {"start":553,"end":554,"count":0}]
 );
 
 TestCoverage("try/catch/finally statements with early return",
@@ -374,10 +374,10 @@ TestCoverage("try/catch/finally statements with early return",
 `,
 [{"start":0,"end":449,"count":1},
  {"start":1,"end":151,"count":1},
- {"start":67,"end":70,"count":0},
+ {"start":69,"end":70,"count":0},
  {"start":91,"end":150,"count":0},
  {"start":201,"end":401,"count":1},
- {"start":267,"end":270,"count":0},
+ {"start":269,"end":270,"count":0},
  {"start":321,"end":400,"count":0}]
 );
 
@@ -409,15 +409,15 @@ TestCoverage(
 `,
 [{"start":0,"end":1099,"count":1},
  {"start":1,"end":151,"count":1},
- {"start":67,"end":70,"count":0},
+ {"start":69,"end":70,"count":0},
  {"start":91,"end":150,"count":0},
  {"start":201,"end":351,"count":1},
  {"start":286,"end":350,"count":0},
  {"start":401,"end":701,"count":1},
  {"start":603,"end":700,"count":0},
  {"start":561,"end":568,"count":0},  // TODO(jgruber): Sorting.
  {"start":751,"end":1051,"count":1},
- {"start":817,"end":820,"count":0},
+ {"start":819,"end":820,"count":0},
  {"start":861,"end":1050,"count":0}]
 );
 
@@ -1004,4 +1004,41 @@ c(true); d(true);                         // 1650
  {"start":1403,"end":1503,"count":0}]
 );
 
+TestCoverage(
+"https://crbug.com/927464",
+`
+!function f() {                           // 0000
+  function unused() { nop(); }            // 0050
+  nop();                                  // 0100
+}();                                      // 0150
+`,
+[{"start":0,"end":199,"count":1},
+ {"start":1,"end":151,"count":1},
+ {"start":52,"end":80,"count":0}]
+);
+
+TestCoverage(
+"https://crbug.com/v8/8691",
+`
+function f(shouldThrow) {                 // 0000
+  if (shouldThrow) {                      // 0050
+    throw Error('threw')                  // 0100
+  }                                       // 0150
+}                                         // 0200
+try {                                     // 0250
+  f(true)                                 // 0300
+} catch (err) {                           // 0350
+                                          // 0400
+}                                         // 0450
+try {                                     // 0500
+  f(false)                                // 0550
+} catch (err) {}                          // 0600
+`,
+[{"start":0,"end":649,"count":1},
+ {"start":351,"end":352,"count":0},
+ {"start":602,"end":616,"count":0},
+ {"start":0,"end":201,"count":2},
+ {"start":69,"end":153,"count":1}]
+);
+
 %DebugToggleBlockCoverage(false);