[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

This revision was automatically updated to reflect the committed changes. Closed by commit rG9caa3fbe03f4: [Coverage] Add empty line regions to SkippedRegions (authored by zequanwu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84988/new/ https://r

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

zequanwu added a comment. In D84988#2282849 , @vsk wrote: > @zequanwu FTR, I don't have any outstanding concerns (I understand you might > be asking for a second reviewer to chime in though). Thanks for reviewing. Then, I think it might be ready to land.

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

vsk added a comment. @zequanwu FTR, I don't have any outstanding concerns (I understand you might be asking for a second reviewer to chime in though). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84988/new/ https://reviews.llvm.org/D84988 __

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

zequanwu added a comment. Friendly ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84988/new/ https://reviews.llvm.org/D84988 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

zequanwu updated this revision to Diff 291630. zequanwu added a comment. Rename option EmptyLineCoverage to EmptyLineCommentCoverage and mark it cl::Hidden. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84988/new/ https://reviews.llvm.org/D84988

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

vsk added inline comments. Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:38 + "disable it on test)"), +llvm::cl::init(true)); + We might consider marking this as cl::Hidden. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

zequanwu updated this revision to Diff 291118. zequanwu added a comment. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. - Address comment. - Add cl::opt to disable emitting skipped regions for empty lines and comments (used on test case only), and update test cases under

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

vsk added a comment. In D84988#2264199 , @zequanwu wrote: > Thanks for reviewing. One last thing I need to resolve is to handle the > coverage test case as skipped regions are emitted for empty lines, and > haven't thought a good way other than adding ch

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

zequanwu added a comment. Thanks for reviewing. One last thing I need to resolve is to handle the coverage test case as skipped regions are emitted for empty lines, and haven't thought a good way other than adding checks for those new skipped regions. Repository: rG LLVM Github Monorepo CHA

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

vsk accepted this revision. vsk added a comment. This revision is now accepted and ready to land. I took a closer look at the lexer changes, and think that these look fine. Thanks again for working on this. LGTM with a minor change, described inline. Comment at: clang/lib/Lex/

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

zequanwu added inline comments. Comment at: llvm/lib/ProfileData/Coverage/CoverageMapping.cpp:597 const auto &R = Segments[I]; - if (!(L.Line < R.Line) && !(L.Line == R.Line && L.Col < R.Col)) { + if (!(L.Line < R.Line) && !(L.Line == R.Line && L.Col <= R.Col)) {

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

zequanwu updated this revision to Diff 289617. zequanwu added a comment. address comment. emit second segment when first segment is skipped and activeregions is not empty. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84988/new/ https://reviews.ll

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

vsk added a comment. I'm not as familiar with the preprocessor bits, but this looks like it's in good shape. Comment at: llvm/lib/ProfileData/Coverage/CoverageMapping.cpp:597 const auto &R = Segments[I]; - if (!(L.Line < R.Line) && !(L.Line == R.Line && L.Col < R.C

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

zequanwu added inline comments. Comment at: llvm/lib/ProfileData/Coverage/CoverageMapping.cpp:483 bool GapRegion = CR.value().Kind == CounterMappingRegion::GapRegion; if (CR.index() + 1 == Regions.size() || vsk wrote: > zequanwu wrote: > > vsk wro

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

zequanwu updated this revision to Diff 289582. zequanwu added a comment. Emit second segment as wrapped segment only when first segment is RegionEntry Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84988/new/ https://reviews.llvm.org/D84988 Files:

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

vsk added inline comments. Comment at: llvm/lib/ProfileData/Coverage/CoverageMapping.cpp:483 bool GapRegion = CR.value().Kind == CounterMappingRegion::GapRegion; if (CR.index() + 1 == Regions.size() || zequanwu wrote: > vsk wrote: > > zequanwu wro

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

zequanwu updated this revision to Diff 289047. zequanwu added a comment. update. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84988/new/ https://reviews.llvm.org/D84988 Files: clang/include/clang/Lex/Lexer.h clang/include/clang/Lex/Preprocess

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

zequanwu added a comment. Friendly ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84988/new/ https://reviews.llvm.org/D84988 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llv

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

zequanwu updated this revision to Diff 286688. zequanwu added a comment. minor fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84988/new/ https://reviews.llvm.org/D84988 Files: clang/include/clang/Lex/Lexer.h clang/include/clang/Lex/Preproc

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

zequanwu added inline comments. Comment at: llvm/lib/ProfileData/Coverage/CoverageMapping.cpp:483 bool GapRegion = CR.value().Kind == CounterMappingRegion::GapRegion; if (CR.index() + 1 == Regions.size() || vsk wrote: > zequanwu wrote: > > vsk wro

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

zequanwu updated this revision to Diff 286622. zequanwu added a comment. Add a wrapped segment at location of zero-length segment with last pushed region count. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84988/new/ https://reviews.llvm.org/D849

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

vsk added inline comments. Comment at: llvm/lib/ProfileData/Coverage/CoverageMapping.cpp:483 bool GapRegion = CR.value().Kind == CounterMappingRegion::GapRegion; if (CR.index() + 1 == Regions.size() || zequanwu wrote: > vsk wrote: > > Why is this

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

zequanwu added inline comments. Comment at: llvm/lib/ProfileData/Coverage/CoverageMapping.cpp:580 const auto &R = Segments[I]; - if (!(L.Line < R.Line) && !(L.Line == R.Line && L.Col < R.Col)) { + if (!(L.Line <= R.Line) && !(L.Line == R.Line && L.Col <= R.Col))

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

zequanwu updated this revision to Diff 286123. zequanwu marked 3 inline comments as done. zequanwu edited the summary of this revision. zequanwu added a comment. Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84988/new/ https://rev

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

zequanwu added inline comments. Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:326 +if (PrevTokLoc.isValid()) { + unsigned PrevTokLine = SM.getSpellingLineNumber(PrevTokLoc); + if (SR.LineStart == PrevTokLine) { vsk wrote: > It looks like this

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

vsk added inline comments. Comment at: clang/include/clang/Lex/Lexer.h:131 + const char *NewLinePtr; + Could you leave a comment describing what this is? Comment at: clang/include/clang/Lex/Preprocessor.h:960 + void setParsingFunctionBody(b

[PATCH] D84988: [Coverage] Add empty line regions to SkippedRegions

zequanwu added a comment. In D84988#2221805 , @vsk wrote: > Hi @zequanwu, are you looking for review for this patch? I wasn't sure > because of the WIP label. Yes, I removed the label. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION