[PATCH] D20997: [Coverage] Fix an assertion failure if the definition of an unused function spans multiple files.

[Vedant Kumar via cfe-commits]

vsk added a comment.

Thanks, this looks good overall. I just have a few minor comments.


================
Comment at: lib/CodeGen/CoverageMappingGen.cpp:329
@@ +328,3 @@
+      FileID ParentFile = SM.getFileID(Start);
+      while (ParentFile != SM.getFileID(End) && !isNestedIn(End, ParentFile)) {
+        Start = getIncludeOrExpansionLoc(Start);
----------------
SM.getFileID() can be expensive if there's a miss in the SourceManager's cache. Seeing as `End` isn't updated in this loop, it might be profitable/cleaner to maintain dedicated `StartFileID` and `EndFileID` variables. Wdyt?

================
Comment at: lib/CodeGen/CoverageMappingGen.cpp:331
@@ +330,3 @@
+        Start = getIncludeOrExpansionLoc(Start);
+        assert(Start.isValid());
+        ParentFile = SM.getFileID(Start);
----------------
Please add a string here, e.g "Declaration start loc not nested within a known region".

================
Comment at: lib/CodeGen/CoverageMappingGen.cpp:336
@@ +335,3 @@
+        End = getPreciseTokenLocEnd(getIncludeOrExpansionLoc(End));
+        assert(End.isValid());
+      }
----------------
^ ditto.


Repository:
  rL LLVM

http://reviews.llvm.org/D20997