[coverage] Extend SourceRangeAstVisitor for throw statements

bcoe · Commit Bot · commit 2d08967d4a4b · 2019-02-28T10:45:29.000Z
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. R=jgruber@chromium.org, neis@chromium.org, yangguo@chromium.org Bug: v8:8691 Change-Id: Ibcbab79fbe54719a8993045040349c863b139011 Reviewed-on: https://chromium-review.googlesource.com/c/1480632 Commit-Queue: Georg Neis <neis@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Reviewed-by: Jakob Gruber <jgruber@chromium.org> Cr-Commit-Position: refs/heads/master@{#59936}
diff --git a/AUTHORS b/AUTHORS
@@ -55,7 +55,7 @@ Anna Henningsen <anna@addaleax.net>
 Anton Bikineev <ant.bikineev@gmail.com>
 Bangfu Tao <bangfu.tao@samsung.com>
 Daniel Shelton <d1.shelton@samsung.com>
-Ben Coe <ben@npmjs.com>
+Ben Coe <bencoe@gmail.com>
 Ben Newman <ben@meteor.com>
 Ben Noordhuis <info@bnoordhuis.nl>
 Benjamin Tan <demoneaux@gmail.com>
diff --git a/src/ast/source-range-ast-visitor.cc b/src/ast/source-range-ast-visitor.cc
@@ -56,7 +56,18 @@ void SourceRangeAstVisitor::MaybeRemoveLastContinuationRange(
   if (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/test/mjsunit/code-coverage-block.js b/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}]
 );
 
@@ -1008,13 +1008,37 @@ TestCoverage(
 "https://crbug.com/927464",
 `
 !function f() {                           // 0000
-  function unused() { nop(); }            // 0100
-  nop();                                  // 0150
-}();                                      // 0200
+  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);

-Original file line number
+Diff line change
 Anton Bikineev <[email protected]>
 Bangfu Tao <[email protected]>
 Daniel Shelton <[email protected]>
 -Ben Coe <ben@npmjs.com>
 +Ben Coe <bencoe@gmail.com>
 Ben Newman <[email protected]>
 Ben Noordhuis <[email protected]>
 Benjamin Tan <[email protected]>