[llvm-cov] Make report metrics agree with line exec counts, fixes PR34615

Use the same logic as the line-oriented coverage view to determine the
number of covered lines in a function.

Fixes llvm.org/PR34615.

llvm-svn: 313604

test/tools/llvm-cov/Inputs/highlightedRanges.json

@@ -12,7 +12,7 @@
 
 // Verify the Summary Section for the first file
 // CHECK-SAME: "summary":{
-// CHECK-SAME: "lines":{"count":40,"covered":26,"percent":65},
+// CHECK-SAME: "lines":{"count":40,"covered":27,"percent":67},
 // CHECK-SAME: "functions":{"count":4,"covered":4,"percent":100},
 // CHECK-SAME: "regions":{"count":19,"covered":11,"notcovered":8,"percent":57}}}
 
@@ -45,7 +45,7 @@
 
 // Full Export Summary
 // CHECK-SAME: "totals":{
-// CHECK-SAME: "lines":{"count":40,"covered":26,"percent":65},
+// CHECK-SAME: "lines":{"count":40,"covered":27,"percent":67},
 // CHECK-SAME: "functions":{"count":4,"covered":4,"percent":100},
 // CHECK-SAME: "instantiations":{"count":4,"covered":4,"percent":100},
 // CHECK-SAME: "regions":{"count":19,"covered":11,"notcovered":8,"percent":57}}

test/tools/llvm-cov/Inputs/lineExecutionCounts.json

@@ -12,7 +12,7 @@
 
 // Verify the Summary Section for the first file
 // CHECK-SAME: "summary":{
-// CHECK-SAME: "lines":{"count":20,"covered":16,"percent":80},
+// CHECK-SAME: "lines":{"count":20,"covered":18,"percent":90},
 // CHECK-SAME: "functions":{"count":1,"covered":1,"percent":100},
 // CHECK-SAME: "regions":{"count":11,"covered":8,"notcovered":3,"percent":72}}}
 
@@ -30,7 +30,7 @@
 
 // Full Export Summary
 // CHECK-SAME: "totals":{
-// CHECK-SAME: "lines":{"count":20,"covered":16,"percent":80},
+// CHECK-SAME: "lines":{"count":20,"covered":18,"percent":90},
 // CHECK-SAME: "functions":{"count":1,"covered":1,"percent":100},
 // CHECK-SAME: "instantiations":{"count":1,"covered":1,"percent":100},
 // CHECK-SAME: "regions":{"count":11,"covered":8,"notcovered":3,"percent":72}}

test/tools/llvm-cov/Inputs/regionMarkers.json

@@ -12,7 +12,7 @@
 
 // Verify the Summary Section for the first file
 // CHECK-SAME: "summary":{
-// CHECK-SAME: "lines":{"count":21,"covered":17,"percent":80},
+// CHECK-SAME: "lines":{"count":21,"covered":18,"percent":85},
 // CHECK-SAME: "functions":{"count":1,"covered":1,"percent":100},
 // CHECK-SAME: "regions":{"count":10,"covered":7,"notcovered":3,"percent":70}}
 
@@ -29,7 +29,7 @@
 
 // Full Export Summary
 // CHECK-SAME: "totals":{
-// CHECK-SAME: "lines":{"count":21,"covered":17,"percent":80},
+// CHECK-SAME: "lines":{"count":21,"covered":18,"percent":85},
 // CHECK-SAME: "functions":{"count":1,"covered":1,"percent":100},
 // CHECK-SAME: "instantiations":{"count":1,"covered":1,"percent":100},
 // CHECK-SAME: "regions":{"count":10,"covered":7,"notcovered":3,"percent":70}}

test/tools/llvm-cov/Inputs/showExpansions.json

@@ -22,7 +22,7 @@
 
 // Verify the Summary Section for the first file
 // CHECK-SAME: "summary":{
-// CHECK-SAME: "lines":{"count":17,"covered":15,"percent":88},
+// CHECK-SAME: "lines":{"count":5,"covered":5,"percent":100},
 // CHECK-SAME: "functions":{"count":1,"covered":1,"percent":100},
 // CHECK-SAME: "regions":{"count":13,"covered":12,"notcovered":1,"percent":92}}
 
@@ -43,7 +43,7 @@
 
 // Full Export Summary
 // CHECK-SAME: "totals":{
-// CHECK-SAME: "lines":{"count":17,"covered":15,"percent":88},
+// CHECK-SAME: "lines":{"count":5,"covered":5,"percent":100},
 // CHECK-SAME: "functions":{"count":1,"covered":1,"percent":100},
 // CHECK-SAME: "instantiations":{"count":1,"covered":1,"percent":100},
 // CHECK-SAME: "regions":{"count":13,"covered":12,"notcovered":1,"percent":92}}

test/tools/llvm-cov/showLineExecutionCounts.cpp

@@ -86,7 +86,7 @@ int main() {                              // TEXT: [[@LINE]]|   161|int main(
 // HTML-INDEX: <td class='column-entry-green'>
 // HTML-INDEX: 100.00% (1/1)
 // HTML-INDEX: <td class='column-entry-yellow'>
-// HTML-INDEX: 80.00% (16/20)
+// HTML-INDEX: 90.00% (18/20)
 // HTML-INDEX: <td class='column-entry-red'>
 // HTML-INDEX: 72.73% (8/11)
 // HTML-INDEX: TOTALS

tools/llvm-cov/CodeCoverage.cpp

@@ -841,7 +841,7 @@ int CodeCoverageTool::show(int argc, const char **argv,
 
     // Show functions.
     for (const auto &Function : Coverage->getCoveredFunctions()) {
-      if (!Filters.matches(Function))
+      if (!Filters.matches(*Coverage.get(), Function))
         continue;
 
       auto mainView = createFunctionView(Function, *Coverage);

tools/llvm-cov/CoverageFilters.cpp

@@ -17,47 +17,54 @@
 
 using namespace llvm;
 
-bool NameCoverageFilter::matches(const coverage::FunctionRecord &Function) {
+bool NameCoverageFilter::matches(const coverage::CoverageMapping &,
+                                 const coverage::FunctionRecord &Function) {
   StringRef FuncName = Function.Name;
   return FuncName.find(Name) != StringRef::npos;
 }
 
-bool
-NameRegexCoverageFilter::matches(const coverage::FunctionRecord &Function) {
+bool NameRegexCoverageFilter::matches(
+    const coverage::CoverageMapping &,
+    const coverage::FunctionRecord &Function) {
   return llvm::Regex(Regex).match(Function.Name);
 }
 
 bool NameWhitelistCoverageFilter::matches(
+    const coverage::CoverageMapping &,
     const coverage::FunctionRecord &Function) {
   return Whitelist.inSection("whitelist_fun", Function.Name);
 }
 
-bool RegionCoverageFilter::matches(const coverage::FunctionRecord &Function) {
-  return PassesThreshold(FunctionCoverageSummary::get(Function)
+bool RegionCoverageFilter::matches(const coverage::CoverageMapping &CM,
+                                   const coverage::FunctionRecord &Function) {
+  return PassesThreshold(FunctionCoverageSummary::get(CM, Function)
                              .RegionCoverage.getPercentCovered());
 }
 
-bool LineCoverageFilter::matches(const coverage::FunctionRecord &Function) {
-  return PassesThreshold(
-      FunctionCoverageSummary::get(Function).LineCoverage.getPercentCovered());
+bool LineCoverageFilter::matches(const coverage::CoverageMapping &CM,
+                                 const coverage::FunctionRecord &Function) {
+  return PassesThreshold(FunctionCoverageSummary::get(CM, Function)
+                             .LineCoverage.getPercentCovered());
 }
 
 void CoverageFilters::push_back(std::unique_ptr<CoverageFilter> Filter) {
   Filters.push_back(std::move(Filter));
 }
 
-bool CoverageFilters::matches(const coverage::FunctionRecord &Function) {
+bool CoverageFilters::matches(const coverage::CoverageMapping &CM,
+                              const coverage::FunctionRecord &Function) {
   for (const auto &Filter : Filters) {
-    if (Filter->matches(Function))
+    if (Filter->matches(CM, Function))
       return true;
   }
   return false;
 }
 
-bool
-CoverageFiltersMatchAll::matches(const coverage::FunctionRecord &Function) {
+bool CoverageFiltersMatchAll::matches(
+    const coverage::CoverageMapping &CM,
+    const coverage::FunctionRecord &Function) {
   for (const auto &Filter : Filters) {
-    if (!Filter->matches(Function))
+    if (!Filter->matches(CM, Function))
       return false;
   }
   return true;

tools/llvm-cov/CoverageFilters.h

@@ -14,6 +14,7 @@
 #ifndef LLVM_COV_COVERAGEFILTERS_H
 #define LLVM_COV_COVERAGEFILTERS_H
 
+#include "CoverageSummaryInfo.h"
 #include "llvm/ProfileData/Coverage/CoverageMapping.h"
 #include "llvm/Support/SpecialCaseList.h"
 #include <memory>
@@ -27,7 +28,8 @@ public:
   virtual ~CoverageFilter() {}
 
   /// \brief Return true if the function passes the requirements of this filter.
-  virtual bool matches(const coverage::FunctionRecord &Function) {
+  virtual bool matches(const coverage::CoverageMapping &CM,
+                       const coverage::FunctionRecord &Function) {
     return true;
   }
 };
@@ -39,7 +41,8 @@ class NameCoverageFilter : public CoverageFilter {
 public:
   NameCoverageFilter(StringRef Name) : Name(Name) {}
 
-  bool matches(const coverage::FunctionRecord &Function) override;
+  bool matches(const coverage::CoverageMapping &CM,
+               const coverage::FunctionRecord &Function) override;
 };
 
 /// \brief Matches functions whose name matches a certain regular expression.
@@ -49,7 +52,8 @@ class NameRegexCoverageFilter : public CoverageFilter {
 public:
   NameRegexCoverageFilter(StringRef Regex) : Regex(Regex) {}
 
-  bool matches(const coverage::FunctionRecord &Function) override;
+  bool matches(const coverage::CoverageMapping &CM,
+               const coverage::FunctionRecord &Function) override;
 };
 
 /// \brief Matches functions whose name appears in a SpecialCaseList in the
@@ -61,7 +65,8 @@ public:
   NameWhitelistCoverageFilter(const SpecialCaseList &Whitelist)
       : Whitelist(Whitelist) {}
 
-  bool matches(const coverage::FunctionRecord &Function) override;
+  bool matches(const coverage::CoverageMapping &CM,
+               const coverage::FunctionRecord &Function) override;
 };
 
 /// \brief Matches numbers that pass a certain threshold.
@@ -97,7 +102,8 @@ public:
   RegionCoverageFilter(Operation Op, double Threshold)
       : StatisticThresholdFilter(Op, Threshold) {}
 
-  bool matches(const coverage::FunctionRecord &Function) override;
+  bool matches(const coverage::CoverageMapping &CM,
+               const coverage::FunctionRecord &Function) override;
 };
 
 /// \brief Matches functions whose line coverage percentage
@@ -108,7 +114,8 @@ public:
   LineCoverageFilter(Operation Op, double Threshold)
       : StatisticThresholdFilter(Op, Threshold) {}
 
-  bool matches(const coverage::FunctionRecord &Function) override;
+  bool matches(const coverage::CoverageMapping &CM,
+               const coverage::FunctionRecord &Function) override;
 };
 
 /// \brief A collection of filters.
@@ -124,7 +131,8 @@ public:
 
   bool empty() const { return Filters.empty(); }
 
-  bool matches(const coverage::FunctionRecord &Function) override;
+  bool matches(const coverage::CoverageMapping &CM,
+               const coverage::FunctionRecord &Function) override;
 };
 
 /// \brief A collection of filters.
@@ -132,7 +140,8 @@ public:
 /// in an instance of this class.
 class CoverageFiltersMatchAll : public CoverageFilters {
 public:
-  bool matches(const coverage::FunctionRecord &Function) override;
+  bool matches(const coverage::CoverageMapping &CM,
+               const coverage::FunctionRecord &Function) override;
 };
 
 } // namespace llvm

tools/llvm-cov/CoverageReport.cpp

@@ -306,7 +306,7 @@ void CoverageReport::renderFunctionReports(ArrayRef<std::string> Files,
     OS << "\n";
     FunctionCoverageSummary Totals("TOTAL");
     for (const auto &F : Functions) {
-      FunctionCoverageSummary Function = FunctionCoverageSummary::get(F);
+      auto Function = FunctionCoverageSummary::get(Coverage, F);
       ++Totals.ExecutionCount;
       Totals.RegionCoverage += Function.RegionCoverage;
       Totals.LineCoverage += Function.LineCoverage;
@@ -332,7 +332,7 @@ std::vector<FileCoverageSummary> CoverageReport::prepareFileReports(
     for (const auto &Group : Coverage.getInstantiationGroups(Filename)) {
       std::vector<FunctionCoverageSummary> InstantiationSummaries;
       for (const coverage::FunctionRecord *F : Group.getInstantiations()) {
-        auto InstantiationSummary = FunctionCoverageSummary::get(*F);
+        auto InstantiationSummary = FunctionCoverageSummary::get(Coverage, *F);
         Summary.addInstantiation(InstantiationSummary);
         Totals.addInstantiation(InstantiationSummary);
         InstantiationSummaries.push_back(InstantiationSummary);

tools/llvm-cov/CoverageSummaryInfo.cpp

@@ -17,8 +17,53 @@
 using namespace llvm;
 using namespace coverage;
 
+LineCoverageStats::LineCoverageStats(
+    ArrayRef<const coverage::CoverageSegment *> LineSegments,
+    const coverage::CoverageSegment *WrappedSegment) {
+  // Find the minimum number of regions which start in this line.
+  unsigned MinRegionCount = 0;
+  auto isStartOfRegion = [](const coverage::CoverageSegment *S) {
+    return !S->IsGapRegion && S->HasCount && S->IsRegionEntry;
+  };
+  for (unsigned I = 0; I < LineSegments.size() && MinRegionCount < 2; ++I)
+    if (isStartOfRegion(LineSegments[I]))
+      ++MinRegionCount;
+
+  bool StartOfSkippedRegion = !LineSegments.empty() &&
+                              !LineSegments.front()->HasCount &&
+                              LineSegments.front()->IsRegionEntry;
+
+  ExecutionCount = 0;
+  HasMultipleRegions = MinRegionCount > 1;
+  Mapped =
+      !StartOfSkippedRegion &&
+      ((WrappedSegment && WrappedSegment->HasCount) || (MinRegionCount > 0));
+
+  if (!Mapped)
+    return;
+
+  // Pick the max count among regions which start and end on this line, to
+  // avoid erroneously using the wrapped count, and to avoid picking region
+  // counts which come from deferred regions.
+  if (LineSegments.size() > 1) {
+    for (unsigned I = 0; I < LineSegments.size() - 1; ++I) {
+      if (!LineSegments[I]->IsGapRegion)
+        ExecutionCount = std::max(ExecutionCount, LineSegments[I]->Count);
+    }
+    return;
+  }
+
+  // If a non-gap region starts here, use its count. Otherwise use the wrapped
+  // count.
+  if (MinRegionCount == 1)
+    ExecutionCount = LineSegments[0]->Count;
+  else
+    ExecutionCount = WrappedSegment->Count;
+}
+
 FunctionCoverageSummary
-FunctionCoverageSummary::get(const coverage::FunctionRecord &Function) {
+FunctionCoverageSummary::get(const CoverageMapping &CM,
+                             const coverage::FunctionRecord &Function) {
   // Compute the region coverage.
   size_t NumCodeRegions = 0, CoveredRegions = 0;
   for (auto &CR : Function.CountedRegions) {
@@ -29,51 +74,32 @@ FunctionCoverageSummary::get(const coverage::FunctionRecord &Function) {
       ++CoveredRegions;
   }
 
-  // TODO: This logic is incorrect and needs to be removed (PR34615). We need
-  // to use the segment builder to get accurate line execution counts.
-  //
   // Compute the line coverage
   size_t NumLines = 0, CoveredLines = 0;
-  for (unsigned FileID = 0, E = Function.Filenames.size(); FileID < E;
-       ++FileID) {
-    // Find the line start and end of the function's source code
-    // in that particular file
-    unsigned LineStart = std::numeric_limits<unsigned>::max();
-    unsigned LineEnd = 0;
-    for (auto &CR : Function.CountedRegions) {
-      if (CR.FileID != FileID)
-        continue;
-      LineStart = std::min(LineStart, CR.LineStart);
-      LineEnd = std::max(LineEnd, CR.LineEnd);
-    }
-    assert(LineStart <= LineEnd && "Function contains spurious file");
-    unsigned LineCount = LineEnd - LineStart + 1;
+  CoverageData CD = CM.getCoverageForFunction(Function);
+  auto NextSegment = CD.begin();
+  auto EndSegment = CD.end();
+  const coverage::CoverageSegment *WrappedSegment = nullptr;
+  SmallVector<const coverage::CoverageSegment *, 4> LineSegments;
+  unsigned Line = NextSegment->Line;
+  while (NextSegment != EndSegment) {
+    // Gather the segments on this line and the wrapped segment.
+    if (LineSegments.size())
+      WrappedSegment = LineSegments.back();
+    LineSegments.clear();
+    while (NextSegment != EndSegment && NextSegment->Line == Line)
+      LineSegments.push_back(&*NextSegment++);
 
-    // Get counters
-    llvm::SmallVector<uint64_t, 16> ExecutionCounts;
-    ExecutionCounts.resize(LineCount, 0);
-    unsigned LinesNotSkipped = LineCount;
-    for (auto &CR : Function.CountedRegions) {
-      if (CR.FileID != FileID)
-        continue;
-      // Ignore the lines that were skipped by the preprocessor.
-      auto ExecutionCount = CR.ExecutionCount;
-      if (CR.Kind == CounterMappingRegion::SkippedRegion) {
-        unsigned SkippedLines = CR.LineEnd - CR.LineStart + 1;
-        assert((SkippedLines <= LinesNotSkipped) &&
-               "Skipped region larger than file containing it");
-        LinesNotSkipped -= SkippedLines;
-        ExecutionCount = 1;
+    LineCoverageStats LCS{LineSegments, WrappedSegment};
+    if (LCS.isMapped()) {
+      ++NumLines;
+      if (LCS.ExecutionCount)
+        ++CoveredLines;
     }
-      for (unsigned I = CR.LineStart; I <= CR.LineEnd; ++I)
-        ExecutionCounts[I - LineStart] = ExecutionCount;
-    }
-    unsigned UncoveredLines = std::min(
-        (unsigned)std::count(ExecutionCounts.begin(), ExecutionCounts.end(), 0),
-        (unsigned)LinesNotSkipped);
-    CoveredLines += LinesNotSkipped - UncoveredLines;
-    NumLines += LinesNotSkipped;
+
+    ++Line;
   }
+
   return FunctionCoverageSummary(
       Function.Name, Function.ExecutionCount,
       RegionCoverageInfo(CoveredRegions, NumCodeRegions),

tools/llvm-cov/CoverageSummaryInfo.h

@@ -136,6 +136,20 @@ public:
   }
 };
 
+/// \brief Coverage statistics for a single line.
+struct LineCoverageStats {
+  uint64_t ExecutionCount;
+  bool HasMultipleRegions;
+  bool Mapped;
+
+  LineCoverageStats(ArrayRef<const coverage::CoverageSegment *> LineSegments,
+                    const coverage::CoverageSegment *WrappedSegment);
+
+  bool isMapped() const { return Mapped; }
+
+  bool hasMultipleRegions() const { return HasMultipleRegions; }
+};
+
 /// \brief A summary of function's code coverage.
 struct FunctionCoverageSummary {
   std::string Name;
@@ -154,8 +168,8 @@ struct FunctionCoverageSummary {
 
   /// \brief Compute the code coverage summary for the given function coverage
   /// mapping record.
-  static FunctionCoverageSummary
-  get(const coverage::FunctionRecord &Function);
+  static FunctionCoverageSummary get(const coverage::CoverageMapping &CM,
+                                     const coverage::FunctionRecord &Function);
 
   /// Compute the code coverage summary for an instantiation group \p Group,
   /// given a list of summaries for each instantiation in \p Summaries.

tools/llvm-cov/SourceCoverageView.cpp

@@ -83,50 +83,6 @@ CoveragePrinter::create(const CoverageViewOptions &Opts) {
   llvm_unreachable("Unknown coverage output format!");
 }
 
-LineCoverageStats::LineCoverageStats(
-    ArrayRef<const coverage::CoverageSegment *> LineSegments,
-    const coverage::CoverageSegment *WrappedSegment) {
-  // Find the minimum number of regions which start in this line.
-  unsigned MinRegionCount = 0;
-  auto isStartOfRegion = [](const coverage::CoverageSegment *S) {
-    return !S->IsGapRegion && S->HasCount && S->IsRegionEntry;
-  };
-  for (unsigned I = 0; I < LineSegments.size() && MinRegionCount < 2; ++I)
-    if (isStartOfRegion(LineSegments[I]))
-      ++MinRegionCount;
-
-  bool StartOfSkippedRegion = !LineSegments.empty() &&
-                              !LineSegments.front()->HasCount &&
-                              LineSegments.front()->IsRegionEntry;
-
-  ExecutionCount = 0;
-  HasMultipleRegions = MinRegionCount > 1;
-  Mapped =
-      !StartOfSkippedRegion &&
-      ((WrappedSegment && WrappedSegment->HasCount) || (MinRegionCount > 0));
-
-  if (!Mapped)
-    return;
-
-  // Pick the max count among regions which start and end on this line, to
-  // avoid erroneously using the wrapped count, and to avoid picking region
-  // counts which come from deferred regions.
-  if (LineSegments.size() > 1) {
-    for (unsigned I = 0; I < LineSegments.size() - 1; ++I) {
-      if (!LineSegments[I]->IsGapRegion)
-        ExecutionCount = std::max(ExecutionCount, LineSegments[I]->Count);
-    }
-    return;
-  }
-
-  // If a non-gap region starts here, use its count. Otherwise use the wrapped
-  // count.
-  if (MinRegionCount == 1)
-    ExecutionCount = LineSegments[0]->Count;
-  else
-    ExecutionCount = WrappedSegment->Count;
-}
-
 unsigned SourceCoverageView::getFirstUncoveredLineNo() {
   const auto MinSegIt =
       find_if(CoverageInfo, [](const coverage::CoverageSegment &S) {

tools/llvm-cov/SourceCoverageView.h

@@ -15,6 +15,7 @@
 #define LLVM_COV_SOURCECOVERAGEVIEW_H
 
 #include "CoverageViewOptions.h"
+#include "CoverageSummaryInfo.h"
 #include "llvm/ProfileData/Coverage/CoverageMapping.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include <vector>
@@ -64,20 +65,6 @@ struct InstantiationView {
   }
 };
 
-/// \brief Coverage statistics for a single line.
-struct LineCoverageStats {
-  uint64_t ExecutionCount;
-  bool HasMultipleRegions;
-  bool Mapped;
-
-  LineCoverageStats(ArrayRef<const coverage::CoverageSegment *> LineSegments,
-                    const coverage::CoverageSegment *WrappedSegment);
-
-  bool isMapped() const { return Mapped; }
-
-  bool hasMultipleRegions() const { return HasMultipleRegions; }
-};
-
 /// \brief A file manager that handles format-aware file creation.
 class CoveragePrinter {
 public: