[PATCH] D85176: [Coverage] Emit gap region after conditions when macro is present.

2021-02-19 Thread Zequan Wu via Phabricator via cfe-commits 
zequanwu added a comment.

In D85176#2572661 , @saugustine wrote:

> This commit introduced an unused variable warning when built without asserts. 
> (CoverageMappingGen.cpp:984) I have fixed it with  rG4544a63b7705 
> .

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85176

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85176: [Coverage] Emit gap region after conditions when macro is present.

2021-02-18 Thread Sterling Augustine via Phabricator via cfe-commits 
saugustine added a comment.

This commit introduced an unused variable warning when built without asserts. 
(CoverageMappingGen.cpp:984) I have fixed it with  rG4544a63b7705 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85176

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85176: [Coverage] Emit gap region after conditions when macro is present.

2021-02-18 Thread Zequan Wu via Phabricator via cfe-commits 
zequanwu added a comment.

Landed at 
https://github.com/llvm/llvm-project/commit/d83511dd26ca8d0dd5be6302ad7b55de05cedab2
 (forgot to add the differential revision line).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85176

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85176: [Coverage] Emit gap region after conditions when macro is present.

2021-02-18 Thread Vedant Kumar via Phabricator via cfe-commits 
vsk accepted this revision.
vsk added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85176

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85176: [Coverage] Emit gap region after conditions when macro is present.

2021-02-18 Thread Zequan Wu via Phabricator via cfe-commits 
zequanwu added inline comments.



Comment at: clang/test/CoverageMapping/if.cpp:10
+void foo() {// CHECK-NEXT: Gap,File 0, [[@LINE+1]]:21 -> 
[[@LINE+1]]:22 = #2
   if (int j = true ? nop()  // CHECK-NEXT: [[@LINE]]:22 -> [[@LINE]]:27 = 
#2
: nop(); // CHECK-NEXT: [[@LINE]]:22 -> [[@LINE]]:27 = 
(#0 - #2)

vsk wrote:
> Just to double-check: this is now starting the gap after the '?', instead of 
> including the '?' - if so, that looks right.
Yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85176

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85176: [Coverage] Emit gap region after conditions when macro is present.

2021-02-18 Thread Zequan Wu via Phabricator via cfe-commits 
zequanwu updated this revision to Diff 324710.
zequanwu marked 2 inline comments as done.
zequanwu added a comment.

- Added regression test.
- Assert `getIncludeOrExpansionLoc(AfterLoc)` is valid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85176

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/test/CoverageMapping/if.cpp
  clang/test/CoverageMapping/macro-expressions.cpp
  clang/test/CoverageMapping/macroparams2.c
  clang/test/CoverageMapping/macros.c
  clang/test/CoverageMapping/macroscopes.cpp
  clang/test/CoverageMapping/moremacros.c

Index: clang/test/CoverageMapping/moremacros.c
===
--- clang/test/CoverageMapping/moremacros.c
+++ clang/test/CoverageMapping/moremacros.c
@@ -9,18 +9,20 @@
   // CHECK-NEXT: File 0, [[@LINE+1]]:7 -> [[@LINE+1]]:12 = #0
   if (!argc) {} // CHECK: File 0, [[@LINE]]:14 -> [[@LINE]]:16 = #1
 
-  // CHECK-NEXT: File 0, [[@LINE+4]]:7 -> [[@LINE+4]]:12 = #0
-  // CHECK-NEXT: Branch,File 0, [[@LINE+3]]:7 -> [[@LINE+3]]:12 = #2, (#0 - #2)
+  // CHECK-NEXT: File 0, [[@LINE+5]]:7 -> [[@LINE+5]]:12 = #0
+  // CHECK-NEXT: Branch,File 0, [[@LINE+4]]:7 -> [[@LINE+4]]:12 = #2, (#0 - #2)
+  // CHECK-NEXT: Gap,File 0, [[@LINE+3]]:13 -> [[@LINE+3]]:14 = #2
   // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:14 -> [[@LINE+2]]:19 = #2
   // CHECK-NEXT: File 0, [[@LINE+1]]:19 -> [[@LINE+4]]:8 = #2
   if (!argc) LBRAC
 return 0;
   // CHECK-NEXT: Expansion,File 0, [[@LINE+1]]:3 -> [[@LINE+1]]:8 = #2
-  RBRAC // CHECK-NEXT: [[@LINE]]:8 -> [[@LINE+7]]:3 = (#0 - #2)
+  RBRAC // CHECK-NEXT: [[@LINE]]:8 -> [[@LINE+8]]:3 = (#0 - #2)
 
-  // CHECK-NEXT: File 0, [[@LINE+5]]:3 -> [[@LINE+16]]:2 = (#0 - #2)
-  // CHECK-NEXT: File 0, [[@LINE+4]]:7 -> [[@LINE+4]]:12 = (#0 - #2)
-  // CHECK-NEXT: Branch,File 0, [[@LINE+3]]:7 -> [[@LINE+3]]:12 = #3, ((#0 - #2) - #3)
+  // CHECK-NEXT: File 0, [[@LINE+6]]:3 -> [[@LINE+17]]:2 = (#0 - #2)
+  // CHECK-NEXT: File 0, [[@LINE+5]]:7 -> [[@LINE+5]]:12 = (#0 - #2)
+  // CHECK-NEXT: Branch,File 0, [[@LINE+4]]:7 -> [[@LINE+4]]:12 = #3, ((#0 - #2) - #3)
+  // CHECK-NEXT: Gap,File 0, [[@LINE+3]]:13 -> [[@LINE+3]]:14 = #3
   // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:14 -> [[@LINE+2]]:19 = #3
   // CHECK-NEXT: File 0, [[@LINE+1]]:19 -> [[@LINE+3]]:4 = #3
   if (!argc) LBRAC
Index: clang/test/CoverageMapping/macroscopes.cpp
===
--- clang/test/CoverageMapping/macroscopes.cpp
+++ clang/test/CoverageMapping/macroscopes.cpp
@@ -61,13 +61,15 @@
   starts_a_scope
   ends_a_scope
 
-  // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:3 -> [[@LINE+2]]:17 = #0
+  // CHECK-NEXT: Expansion,File 0, [[@LINE+3]]:3 -> [[@LINE+3]]:17 = #0
+  // CHECK-NEXT: Gap,File 0, [[@LINE+2]]:17 -> [[@LINE+3]]:5 = #8
   // CHECK-NEXT: Expansion,File 0, [[@LINE+2]]:5 -> [[@LINE+2]]:16 = #8
   starts_a_while
 simple_stmt;
 
   x = 0;
-  // CHECK-NEXT: Expansion,File 0, [[@LINE+4]]:3 -> [[@LINE+4]]:17 = #0
+  // CHECK-NEXT: Expansion,File 0, [[@LINE+5]]:3 -> [[@LINE+5]]:17 = #0
+  // CHECK-NEXT: Gap,File 0, [[@LINE+4]]:17 -> [[@LINE+4]]:18 = #9
   // CHECK-NEXT: File 0, [[@LINE+3]]:18 -> [[@LINE+5]]:15 = #9
   // CHECK-NEXT: Expansion,File 0, [[@LINE+3]]:5 -> [[@LINE+3]]:16 = #9
   // CHECK-NEXT: Expansion,File 0, [[@LINE+3]]:3 -> [[@LINE+3]]:15 = #9
Index: clang/test/CoverageMapping/macros.c
===
--- clang/test/CoverageMapping/macros.c
+++ clang/test/CoverageMapping/macros.c
@@ -38,37 +38,55 @@
 // CHECK-NEXT: File 2, 4:17 -> 4:22 = #0
 
 // CHECK-NEXT: func4
-void func4() { // CHECK-NEXT: File 0, [[@LINE]]:14 -> [[@LINE+7]]:2 = #0
+void func4() { // CHECK-NEXT: File 0, [[@LINE]]:14 -> [[@LINE+8]]:2 = #0
   int i = 0;
   while (i++ < 10) // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE]]:18 = (#0 + #1)
-if (i < 5) // CHECK: File 0, [[@LINE]]:5 -> [[@LINE+3]]:14 = #1
+if (i < 5) // CHECK: File 0, [[@LINE]]:5 -> [[@LINE+4]]:14 = #1
// CHECK-NEXT: File 0, [[@LINE-1]]:9 -> [[@LINE-1]]:14 = #1
// CHECK-NEXT: Branch,File 0, [[@LINE-2]]:9 -> [[@LINE-2]]:14 = #2, (#1 - #2)
+   // CHECK-NEXT: Gap,File 0, [[@LINE-3]]:15 -> [[@LINE+1]]:7 = #2
   MACRO_2; // CHECK-NEXT: Expansion,File 0, [[@LINE]]:7 -> [[@LINE]]:14 = #2
 }
 // CHECK-NEXT: File 1, 4:17 -> 4:22 = #2
 // CHECK-NOT: File 1
 
 // CHECK-NEXT: func5
-void func5() { // CHECK-NEXT: File 0, [[@LINE]]:14 -> [[@LINE+5]]:2 = #0
+void func5() { // CHECK-NEXT: File 0, [[@LINE]]:14 -> [[@LINE+6]]:2 = #0
   int i = 0;
   if (i > 5) // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:12 = #0
  // CHECK-NEXT: Branch,File 0, [[@LINE-1]]:7 -> [[@LINE-1]]:12 = #1, (#0 - #1)
+ // CHECK-NEXT: Gap,File 0, [[@LINE-2]]:13 -> [[@LINE+1]]:5 = #1
 MACRO_3; // CHECK-NEXT: Expansion,File 0, [[@LINE]]:5 ->

[PATCH] D85176: [Coverage] Emit gap region after conditions when macro is present.

2021-02-18 Thread Vedant Kumar via Phabricator via cfe-commits 
vsk added a comment.

Could you check in the reproducer program (`void p`) as a regression test?




Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:995
+
+AfterLoc = getPreciseTokenLocEnd(getIncludeOrExpansionLoc(AfterLoc));
+assert(AfterLoc.isValid());

It could aid debugging to assert that the result of 
`getIncludeOrExpansionLoc(AfterLoc)` is valid.



Comment at: clang/test/CoverageMapping/if.cpp:10
+void foo() {// CHECK-NEXT: Gap,File 0, [[@LINE+1]]:21 -> 
[[@LINE+1]]:22 = #2
   if (int j = true ? nop()  // CHECK-NEXT: [[@LINE]]:22 -> [[@LINE]]:27 = 
#2
: nop(); // CHECK-NEXT: [[@LINE]]:22 -> [[@LINE]]:27 = 
(#0 - #2)

Just to double-check: this is now starting the gap after the '?', instead of 
including the '?' - if so, that looks right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85176

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits