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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to