[PATCH] D97101: [Coverage] Emit zero count gap region between statements if first statements contains return, throw, goto, co_return

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added inline comments.


================
Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:593
       MostRecentLocation = *StartLoc;
-      completeDeferred(Count, MostRecentLocation);
     }
 
----------------
vsk wrote:
> Does this mean a deferred gap region following a `break;` statement won't be completed before the next region is pushed?
I reverted this change.


================
Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:942
     pushRegion(Counter::getZero());
-    auto &ZeroRegion = getRegion();
-    ZeroRegion.setDeferred(true);
-    LastTerminatedRegion = {EndLoc, RegionStack.size()};
+    if (!HasTerminateStmt) {
+      auto &ZeroRegion = getRegion();
----------------
vsk wrote:
> What's supposed to be the difference between the zero region pushed after a `return;` vs. after a `break;`?
What I think is `DeferredRegion` is only used for `break;` and `continue`. Other terminate statements like `return;`, `throw` etc will use the logic in `VisitStmt` to emit gap region. So, I added this condition to separate the two cases.


================
Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1057
+    if (SaveTerminateStmt)
+      HasTerminateStmt = true;
     handleFileExit(getEnd(S));
----------------
vsk wrote:
> Can this replace all of the machinery around DeferredRegions? It'd be nice to just have one way to set up gaps between statements.
See above comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97101/new/

https://reviews.llvm.org/D97101