[PATCH] D138847: MC/DC in LLVM Source-Based Code Coverage: llvm-cov visualization

[Gulfem Savrun Yeniceri via Phabricator via llvm-commits]

gulfem added a comment.

Overall, it looks good to me. I added a few minor things.
Please add `--show-mcdc` to the `llvm-cov` command guide.
https://github.com/llvm/llvm-project/blob/main/llvm/docs/CommandGuide/llvm-cov.rst



================
Comment at: llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h:238
+    /// A Branch Region can be extended to include IDs to facilitate MC/DC.
+    MCDCBranchRegion
   };
----------------
What is the purpose of `MCDCBranchRegion`?  We already have branch regions, and why adding `MCDCDecisionRegion` is not enough? 


================
Comment at: llvm/tools/llvm-cov/CoverageSummaryInfo.h:147
+class MCDCCoverageInfo {
+  /// The number of Independence Pairs that were covered
+  size_t CoveredPairs;
----------------
This comment misses a dot at the end.


================
Comment at: llvm/tools/llvm-cov/CoverageSummaryInfo.h:158
+      : CoveredPairs(CoveredPairs), NumPairs(NumPairs) {
+    assert(CoveredPairs <= NumPairs && "Covered Pairs over-counted");
+  }
----------------
"Covered pairs over-counted"


================
Comment at: llvm/tools/llvm-cov/CoverageSummaryInfo.h:179
+  double getPercentCovered() const {
+    assert(CoveredPairs <= NumPairs && "Covered Pairs over-counted");
+    if (NumPairs == 0)
----------------
"Covered pairs over-counted"


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

https://reviews.llvm.org/D138847