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

Mon Jul 13 13:34:40 PDT 2020

vsk added a comment.

@zequanwu thanks for working on this. Instead of threading CommentSkipped through PPCallbacks, wdyt of 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.

================
Comment at: llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp:152
+static MutableArrayRef<CounterMappingRegion>
+mergeSkippedRegions(MutableArrayRef<CounterMappingRegion> MappingRegions) {
+  SmallVector<CounterMappingRegion, 32> MergedRegions;
----------------
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.

================
Comment at: llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp:183
+
+  return MergedRegions;
+}
----------------
This is stack use after free, right? This needs to release ownership of the 'MergedRegions' container.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83592