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