[coverage] remove the last continuation range before synthetic return

bcoe · Commit Bot · commit 2d5017a0fc02 · 2020-07-07T15:53:21.000Z
Rather than only removing the continuation range for the last return statement prior to a synthetic return statement, remove the continuation tracking for whatever statement occurs prior to the synthetic return. Bug: v8:10628 Change-Id: Ieb8e393479c9811cf1b9756840bbfdbe7f44a1b8 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2280585 Commit-Queue: Benjamin Coe <bencoe@google.com> Reviewed-by: Toon Verwaest <verwaest@chromium.org> Reviewed-by: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Sigurd Schneider <sigurds@chromium.org> Cr-Commit-Position: refs/heads/master@{#68719}
diff --git a/src/ast/source-range-ast-visitor.cc b/src/ast/source-range-ast-visitor.cc
@@ -99,11 +99,13 @@ void SourceRangeAstVisitor::MaybeRemoveLastContinuationRange(
 }
 
 namespace {
-Statement* FindLastNonSyntheticReturn(ZonePtrList<Statement>* statements) {
+Statement* FindLastNonSyntheticStatement(ZonePtrList<Statement>* statements) {
   for (int i = statements->length() - 1; i >= 0; --i) {
     Statement* stmt = statements->at(i);
-    if (!stmt->IsReturnStatement()) break;
-    if (stmt->AsReturnStatement()->is_synthetic_async_return()) continue;
+    if (stmt->IsReturnStatement() &&
+        stmt->AsReturnStatement()->is_synthetic_async_return()) {
+      continue;
+    }
     return stmt;
   }
   return nullptr;
@@ -114,11 +116,11 @@ void SourceRangeAstVisitor::MaybeRemoveContinuationRangeOfAsyncReturn(
     TryCatchStatement* try_catch_stmt) {
   // Detect try-catch inserted by NewTryCatchStatementForAsyncAwait in the
   // parser (issued for async functions, including async generators), and
-  // remove the continuation ranges of return statements corresponding to
-  // returns at function end in the untransformed source.
+  // remove the continuation range of the last statement, such that the
+  // range of the enclosing function body is used.
   if (try_catch_stmt->is_try_catch_for_async()) {
     Statement* last_non_synthetic =
-        FindLastNonSyntheticReturn(try_catch_stmt->try_block()->statements());
+      FindLastNonSyntheticStatement(try_catch_stmt->try_block()->statements());
     if (last_non_synthetic) {
       MaybeRemoveContinuationRange(last_non_synthetic);
     }
diff --git a/test/mjsunit/code-coverage-block-async.js b/test/mjsunit/code-coverage-block-async.js
@@ -136,8 +136,6 @@ test().then(r => r.bar());                // 0350
  {"start":152,"end":253,"count":1},
  {"start":362,"end":374,"count":1}]);
 
-// Documents the open bug that causes trailing space after `finally`
-// not to be removed by SourceRangeAstVisitor.
 TestCoverage(
 "https://crbug.com/v8/10628",
 `
@@ -152,7 +150,52 @@ abc();                                    // 0350
 %PerformMicrotaskCheckpoint();            // 0400
 `,
 [{"start":0,"end":449,"count":1},
- {"start":0,"end":301,"count":1},
- {"start":252,"end":300,"count":0}]);
+ {"start":0,"end":301,"count":1}]);
+
+TestCoverage(
+"try/catch/finally statements async",
+`
+!async function() {                       // 0000
+  try { nop(); } catch (e) { nop(); }     // 0050
+  try { nop(); } finally { nop(); }       // 0100
+  try {                                   // 0150
+    try { throw 42; } catch (e) { nop(); }// 0200
+  } catch (e) { nop(); }                  // 0250
+  try {                                   // 0300
+    try { throw 42; } finally { nop(); }  // 0350
+  } catch (e) { nop(); }                  // 0400
+  try {                                   // 0450
+    throw 42;                             // 0500
+  } catch (e) {                           // 0550
+    nop();                                // 0600
+  } finally {                             // 0650
+    nop();                                // 0700
+  }                                       // 0750
+}();                                      // 0800
+`,
+[{"start":0,"end":849,"count":1},
+  {"start":1,"end":801,"count":1},
+  {"start":67,"end":87,"count":0},
+  {"start":254,"end":274,"count":0}]
+);
+
+TestCoverage("try/catch/finally statements with early return async",
+`
+!async function() {                       // 0000
+  try { throw 42; } catch (e) { return; } // 0050
+  nop();                                  // 0100
+}();                                      // 0150
+!async function() {                       // 0200
+  try { throw 42; } catch (e) {}          // 0250
+  finally { return; }                     // 0300
+  nop();                                  // 0350
+}();                                      // 0400
+`,
+[{"start":0,"end":449,"count":1},
+  {"start":1,"end":151,"count":1},
+  {"start":91,"end":150,"count":0},
+  {"start":201,"end":401,"count":1},
+  {"start":321,"end":400,"count":0}]
+);
 
 %DebugToggleBlockCoverage(false);
