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

Zequan Wu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 13 16:57:21 PDT 2020


zequanwu added a comment.

For those test cases, I could either moving the comments inside a function to outside or change `CHECK-NEXT` to `CHECK`

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

> Before updating any tests, maybe it's worth doing a quick experiment with comments placed in different contexts, to see whether adding these skipped regions is really sufficient. For example, given:
>
>   1|  for (auto x : collection) {
>   2|    // Explain the loop.
>   3|  }
>
>
> The loop region covers lines 1-3. If we skip the comment range on line 2, does an execution count from the loop region still get picked up? I'd expect it to. It's possible that we need more information from the preprocessor about whether the line is fully comment/whitespace-only.


Here is what I got:

  $ clang -fcoverage-mapping -fprofile-instr-generate /tmp/a.c -Xclang -dump-coverage-mapping && ./a.out && llvm-profdata merge -sparse default.profraw -o a.profdata && llvm-cov show ./a.out -instr-profile=a.profdata
  main:
    File 0, 1:12 -> 6:2 = #0
    File 0, 2:19 -> 2:25 = (#0 + #1)
    File 0, 2:27 -> 2:30 = #1
    Gap,File 0, 2:31 -> 2:32 = #1
    File 0, 2:32 -> 4:4 = #1
    Skipped,File 0, 3:5 -> 3:15 = 0
      1|      1|int main() {
      2|     11|  for (int i = 0; i < 10; i++) {
      3|       |    // comment
      4|     10|  }
      5|      1|  return 0;
      6|      1|}

For those failed test cases, they are caused by extra `Skipped, File ...` lines which invalidate some `CHECK-NEXT`. 
I could either change `CHECK-NEXT` to `CHECK` or moving those checks inside function to above the function, since comments outside functions will not be tracked.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83592





More information about the llvm-commits mailing list