[PATCH] D83592: [Parser] Add comment to skipped regions

[Zequan Wu via Phabricator via cfe-commits]

zequanwu marked an inline comment as done.
zequanwu added a comment.

In D83592#2148376 <https://reviews.llvm.org/D83592#2148376>, @vsk wrote:

> taking advantage of the existing CommentHandler interface? To simplify things, we can add a static method like 'CoverageMappingGen::setupPreprocessorCallbacks(Preprocessor &)' and call that from CodeGenAction::CreateASTConsumer.


Sorry, I don't quite understand this. I am already using `ActionCommentHandler` to handle this one inside `HandleComment`.



================
Comment at: llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp:152
+static MutableArrayRef<CounterMappingRegion>
+mergeSkippedRegions(MutableArrayRef<CounterMappingRegion> MappingRegions) {
+  SmallVector<CounterMappingRegion, 32> MergedRegions;
----------------
vsk wrote:
> This seems like a lot of complexity to add to handle a narrow case. Is it necessary to merge skipped regions early in the process? Note that llvm's SegmentBuilder takes care of merging regions at the very end.
Merging at here is to avoid duplicate output for option `-dump-coverage-mapping`. Like the following example to avoid `Skipped,File 0, 2:3 -> 4:9 = 0` and `Skipped,File 0, 2:15 -> 2:25 = 0` to be outputted at the same time. They should be merged into 1 region `Skipped,File 0, 2:3 -> 4:9 = 0`
```
File 0, 1:12 -> 6:2 = #0
 Skipped,File 0, 2:3 -> 4:9 = 0
 Skipped,File 0, 2:15 -> 2:25 = 0
   1|      1|int main() {
   2|       |  #ifdef TEST // comment
   3|       |  int x = 1;
   4|       |  #endif
   5|      1|  return 0;
   6|      1|}
```
So, we need to do it early.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83592/new/

https://reviews.llvm.org/D83592