[Clang][CoverageMapping] Fix compile time explosions by adjusting only appropriated skipped ranges

[bruno]

Summary

D83592 added comments to be part of skipped regions, and as part of that, it
also shrinks a skipped range if it spans a line that contains a non-comment
token. This is done by adjustSkippedRange.

The adjustSkippedRange currently runs on skipped regions that are not
comments, causing a 5min regression while building a big C++ files without any
comments.

Fix the compile time introduced in D83592 by tagging SkippedRange with kind
information and use that to decide what needs additional processing. There are
no good way to add a testcase for this that I'm aware.

[zequanwu]

LGTM, thanks.

[zequanwu]

Just noticed that 3 test cases failed. Please fix them before landing.

[bruno]

Just noticed that 3 test cases failed. Please fix them before landing.

Sure thing, thanks!

[bruno]

Empty lines handling should also make it skipped, fix the 3 failures.

clang/lib/CodeGen/CoverageMappingGen.h

 class LangOptions;
 class SourceManager;
 class FileEntry;
 class Preprocessor;
 class Decl;
 class Stmt;
 
 struct SkippedRange {
+  enum Kind {
+    PPIfElse, // Preprocessor #if/#else ...
+    EmptyLine,
+    Comment,
+  };
+
   SourceRange Range;
   // The location of token before the skipped source range.
   SourceLocation PrevTokLoc;
   // The location of token after the skipped source range.
   SourceLocation NextTokLoc;
+  // The nature of this skipped range
+  Kind RangeKind;
+
+  bool isComment() { return RangeKind == Comment; }
+  bool isEmptyLine() { return RangeKind == EmptyLine; }
+  bool isPPIfElse() { return RangeKind == PPIfElse; }
 
-  SkippedRange(SourceRange Range, SourceLocation PrevTokLoc = SourceLocation(),
+  SkippedRange(SourceRange Range, Kind K,
+               SourceLocation PrevTokLoc = SourceLocation(),
                SourceLocation NextTokLoc = SourceLocation())
-      : Range(Range), PrevTokLoc(PrevTokLoc), NextTokLoc(NextTokLoc) {}
+      : Range(Range), PrevTokLoc(PrevTokLoc), NextTokLoc(NextTokLoc),
+        RangeKind(K) {}
 };
 
 /// Stores additional source code information like skipped ranges which
 /// is required by the coverage mapping generator and is obtained from
 /// the preprocessor.
 class CoverageSourceInfo : public PPCallbacks,
                            public CommentHandler,
                            public EmptylineHandler {
   // A vector of skipped source ranges and PrevTokLoc with NextTokLoc.
   std::vector<SkippedRange> SkippedRanges;
 
   SourceManager &SourceMgr;
 
 public:
   // Location of the token parsed before HandleComment is called. This is
   // updated every time Preprocessor::Lex lexes a new token.
   SourceLocation PrevTokLoc;
 
   CoverageSourceInfo(SourceManager &SourceMgr) : SourceMgr(SourceMgr) {}
 
   std::vector<SkippedRange> &getSkippedRanges() { return SkippedRanges; }
 
-  void AddSkippedRange(SourceRange Range);
+  void AddSkippedRange(SourceRange Range, SkippedRange::Kind RangeKind);
 
   void SourceRangeSkipped(SourceRange Range, SourceLocation EndifLoc) override;
 
   void HandleEmptyline(SourceRange Range) override;
 
   bool HandleComment(Preprocessor &PP, SourceRange Range) override;
 
   void updateNextTokLoc(SourceLocation Loc);

clang/lib/CodeGen/CoverageMappingGen.cpp

PP.setTokenWatcher([CoverageInfo](clang::Token Tok) {
       CoverageInfo->PrevTokLoc = Tok.getLocation();
       if (Tok.getKind() != clang::tok::eod)
         CoverageInfo->updateNextTokLoc(Tok.getLocation());
     });
   }
   return CoverageInfo;
 }
 
-void CoverageSourceInfo::AddSkippedRange(SourceRange Range) {
+void CoverageSourceInfo::AddSkippedRange(SourceRange Range,
+                                         SkippedRange::Kind RangeKind) {
   if (EmptyLineCommentCoverage && !SkippedRanges.empty() &&
       PrevTokLoc == SkippedRanges.back().PrevTokLoc &&
       SourceMgr.isWrittenInSameFile(SkippedRanges.back().Range.getEnd(),
                                     Range.getBegin()))
     SkippedRanges.back().Range.setEnd(Range.getEnd());
   else
-    SkippedRanges.push_back({Range, PrevTokLoc});
+    SkippedRanges.push_back({Range, RangeKind, PrevTokLoc});
 }
 
 void CoverageSourceInfo::SourceRangeSkipped(SourceRange Range, SourceLocation) {
-  AddSkippedRange(Range);
+  AddSkippedRange(Range, SkippedRange::PPIfElse);
 }
 
 void CoverageSourceInfo::HandleEmptyline(SourceRange Range) {
-  AddSkippedRange(Range);
+  AddSkippedRange(Range, SkippedRange::EmptyLine);
 }
 
 bool CoverageSourceInfo::HandleComment(Preprocessor &PP, SourceRange Range) {
-  AddSkippedRange(Range);
+  AddSkippedRange(Range, SkippedRange::Comment);
   return false;
 }
 
 void CoverageSourceInfo::updateNextTokLoc(SourceLocation Loc) {
   if (!SkippedRanges.empty() && SkippedRanges.back().NextTokLoc.isInvalid())
     SkippedRanges.back().NextTokLoc = Loc;
 }
 
Optional<unsigned> getCoverageFileID(SourceLocation Loc) {
     if (Mapping != FileIDMapping.end())
       return Mapping->second.first;
     return None;
   }
 
   /// This shrinks the skipped range if it spans a line that contains a
   /// non-comment token. If shrinking the skipped range would make it empty,
   /// this returns None.
+  /// Note this function can potentially be expensive because
+  /// getSpellingLineNumber uses getLineNumber, which is expensive.
   Optional<SpellingRegion> adjustSkippedRange(SourceManager &SM,
                                               SourceLocation LocStart,
                                               SourceLocation LocEnd,
                                               SourceLocation PrevTokLoc,
                                               SourceLocation NextTokLoc) {
     SpellingRegion SR{SM, LocStart, LocEnd};
     SR.ColumnStart = 1;
     if (PrevTokLoc.isValid() && SM.isWrittenInSameFile(LocStart, PrevTokLoc) &&
for (auto &I : SkippedRanges) {
       auto LocStart = Range.getBegin();
       auto LocEnd = Range.getEnd();
       assert(SM.isWrittenInSameFile(LocStart, LocEnd) &&
              "region spans multiple files");
 
       auto CovFileID = getCoverageFileID(LocStart);
       if (!CovFileID)
         continue;
-      Optional<SpellingRegion> SR =
-          adjustSkippedRange(SM, LocStart, LocEnd, I.PrevTokLoc, I.NextTokLoc);
-      if (!SR.hasValue())
+      Optional<SpellingRegion> SR;
+      if (I.isComment())
+        SR = adjustSkippedRange(SM, LocStart, LocEnd, I.PrevTokLoc,
+                                I.NextTokLoc);
+      else if (I.isPPIfElse())
+        SR = {SM, LocStart, LocEnd};
+
+      if (I.isEmptyLine() || !SR.hasValue())
         continue;
       auto Region = CounterMappingRegion::makeSkipped(
           *CovFileID, SR->LineStart, SR->ColumnStart, SR->LineEnd,
           SR->ColumnEnd);
       // Make sure that we only collect the regions that are inside
       // the source code of this function.
       if (Region.LineStart >= FileLineRanges[*CovFileID].first &&
           Region.LineEnd <= FileLineRanges[*CovFileID].second)