[PATCH] D70571: [Coverage] Emit a gap region to cover switch bodies
This revision was automatically updated to reflect the committed changes. Closed by commit rG859bf4d2bea2: [Coverage] Emit a gap region to cover switch bodies (authored by vsk). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D70571?vs=231724&id=231964#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70571/new/ https://reviews.llvm.org/D70571 Files: clang/docs/SourceBasedCodeCoverage.rst clang/lib/CodeGen/CoverageMappingGen.cpp clang/test/CoverageMapping/switch.cpp clang/test/CoverageMapping/switchmacro.c Index: clang/test/CoverageMapping/switchmacro.c === --- clang/test/CoverageMapping/switchmacro.c +++ clang/test/CoverageMapping/switchmacro.c @@ -4,7 +4,7 @@ // CHECK: foo int foo(int i) { // CHECK-NEXT: File 0, [[@LINE]]:16 -> {{[0-9]+}}:2 = #0 - switch (i) { + switch (i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:14 -> {{[0-9]+}}:11 = 0 default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> {{[0-9]+}}:11 = #2 if (i == 1) // CHECK-NEXT: File 0, [[@LINE]]:9 -> [[@LINE]]:15 = #2 return 0; // CHECK: File 0, [[@LINE]]:7 -> [[@LINE]]:15 = #3 Index: clang/test/CoverageMapping/switch.cpp === --- clang/test/CoverageMapping/switch.cpp +++ clang/test/CoverageMapping/switch.cpp @@ -2,11 +2,11 @@ // CHECK: foo void foo(int i) { // CHECK-NEXT: File 0, [[@LINE]]:17 -> [[@LINE+8]]:2 = #0 - switch(i) { + switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+4]]:10 = 0 case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:11 = #2 return; case 2: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #3 -break; // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = #1 +break; // CHECK-NEXT: Gap,File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = #1 } int x = 0;// CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:2 = #1 } @@ -29,7 +29,7 @@ nop(); switch (i) { // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+6]]:2 = #4 -nop(); // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE+2]]:10 = 0 +nop(); // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE+2]]:10 = 0 case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #7 nop(); } @@ -47,7 +47,7 @@ // CHECK-NEXT: main int main() {// CHECK-NEXT: File 0, [[@LINE]]:12 -> [[@LINE+35]]:2 = #0 int i = 0; - switch(i) { + switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+8]]:10 = 0 case 0: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = #2 i = 1; break; @@ -58,16 +58,16 @@ break; // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = #1 } switch(i) { // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+23]]:2 = #1 - case 0: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = #6 -i = 1; + case 0: // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:13 -> [[@LINE+6]]:10 = 0 +i = 1; // CHECK-NEXT: File 0, [[@LINE-1]]:3 -> [[@LINE+1]]:10 = #6 break; case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+3]]:10 = #7 i = 2; default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = (#7 + #8) break; // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+3]]:3 = #5 } - - switch(i) { // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+13]]:2 = #5 +// CHECK-NEXT: File 0, [[@LINE+1]]:3 -> [[@LINE+14]]:2 = #5 + switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+6]]:11 = 0 case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+5]]:11 = #10 case 2: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+4]]:11 = (#10 + #11) i = 11; @@ -82,10 +82,23 @@ return 0; } + // CHECK: pr44011 +int pr44011(int i) { // CHECK-NEXT: File 0, [[@LINE]]:20 -> {{.*}}:2 = #0 + switch (i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:14 -> [[@LINE+6]]:13 = 0 + + case 1:// CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:13 = #2 +return 0; + + default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:13 = #3 +return 1; + } +} // A region for counter #1 is missing due to the missing return. + + // FIXME: End location for "case 1" shouldn't point at the end of the switch. // CHECK: fallthrough int fallthrough(int i) { // CHECK-NEXT: File 0, [[@LINE]]:24 -> [[@LINE+12]]:2 = #0 - switch(i) { + switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+9]]:10 = 0 case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+8]]:10 = #2 i = 23; case 2: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = (#2 + #3) @@ -101,7 +114,7 @@ void abort(void) __attribute((noretur
[PATCH] D70571: [Coverage] Emit a gap region to cover switch bodies
vsk marked an inline comment as done. vsk added inline comments. Comment at: clang/docs/SourceBasedCodeCoverage.rst:330 +last case ends). This gap region has a zero count: this causes "gap" areas in +between case statements, which contain no executable code, to appear uncovered. + hans wrote: > I thought the point is that it //doesn't// appear uncovered in coverage > reports? I.e. it will have a zero count, but that's expected. Yes, I'll make it clear that a zero count is expected here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70571/new/ https://reviews.llvm.org/D70571 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70571: [Coverage] Emit a gap region to cover switch bodies
hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. Looks fine to me. Comment at: clang/docs/SourceBasedCodeCoverage.rst:330 +last case ends). This gap region has a zero count: this causes "gap" areas in +between case statements, which contain no executable code, to appear uncovered. + I thought the point is that it //doesn't// appear uncovered in coverage reports? I.e. it will have a zero count, but that's expected. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70571/new/ https://reviews.llvm.org/D70571 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70571: [Coverage] Emit a gap region to cover switch bodies
vsk updated this revision to Diff 231724. vsk added a comment. Add test case from PR44011. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70571/new/ https://reviews.llvm.org/D70571 Files: clang/docs/SourceBasedCodeCoverage.rst clang/lib/CodeGen/CoverageMappingGen.cpp clang/test/CoverageMapping/switch.cpp clang/test/CoverageMapping/switchmacro.c Index: clang/test/CoverageMapping/switchmacro.c === --- clang/test/CoverageMapping/switchmacro.c +++ clang/test/CoverageMapping/switchmacro.c @@ -4,7 +4,7 @@ // CHECK: foo int foo(int i) { // CHECK-NEXT: File 0, [[@LINE]]:16 -> {{[0-9]+}}:2 = #0 - switch (i) { + switch (i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:14 -> {{[0-9]+}}:11 = 0 default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> {{[0-9]+}}:11 = #2 if (i == 1) // CHECK-NEXT: File 0, [[@LINE]]:9 -> [[@LINE]]:15 = #2 return 0; // CHECK: File 0, [[@LINE]]:7 -> [[@LINE]]:15 = #3 Index: clang/test/CoverageMapping/switch.cpp === --- clang/test/CoverageMapping/switch.cpp +++ clang/test/CoverageMapping/switch.cpp @@ -2,11 +2,11 @@ // CHECK: foo void foo(int i) { // CHECK-NEXT: File 0, [[@LINE]]:17 -> [[@LINE+8]]:2 = #0 - switch(i) { + switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+4]]:10 = 0 case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:11 = #2 return; case 2: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #3 -break; // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = #1 +break; // CHECK-NEXT: Gap,File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = #1 } int x = 0;// CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:2 = #1 } @@ -29,7 +29,7 @@ nop(); switch (i) { // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+6]]:2 = #4 -nop(); // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE+2]]:10 = 0 +nop(); // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE+2]]:10 = 0 case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #7 nop(); } @@ -47,7 +47,7 @@ // CHECK-NEXT: main int main() {// CHECK-NEXT: File 0, [[@LINE]]:12 -> [[@LINE+35]]:2 = #0 int i = 0; - switch(i) { + switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+8]]:10 = 0 case 0: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = #2 i = 1; break; @@ -58,16 +58,16 @@ break; // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = #1 } switch(i) { // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+23]]:2 = #1 - case 0: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = #6 -i = 1; + case 0: // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:13 -> [[@LINE+6]]:10 = 0 +i = 1; // CHECK-NEXT: File 0, [[@LINE-1]]:3 -> [[@LINE+1]]:10 = #6 break; case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+3]]:10 = #7 i = 2; default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = (#7 + #8) break; // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+3]]:3 = #5 } - - switch(i) { // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+13]]:2 = #5 +// CHECK-NEXT: File 0, [[@LINE+1]]:3 -> [[@LINE+14]]:2 = #5 + switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+6]]:11 = 0 case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+5]]:11 = #10 case 2: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+4]]:11 = (#10 + #11) i = 11; @@ -82,10 +82,23 @@ return 0; } + // CHECK: pr44011 +int pr44011(int i) { // CHECK-NEXT: File 0, [[@LINE]]:20 -> {{.*}}:2 = #0 + switch (i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:14 -> [[@LINE+6]]:13 = 0 + + case 1:// CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:13 = #2 +return 0; + + default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:13 = #3 +return 1; + } +} // A region for counter #1 is missing due to the missing return. + + // FIXME: End location for "case 1" shouldn't point at the end of the switch. // CHECK: fallthrough int fallthrough(int i) { // CHECK-NEXT: File 0, [[@LINE]]:24 -> [[@LINE+12]]:2 = #0 - switch(i) { + switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+9]]:10 = 0 case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+8]]:10 = #2 i = 23; case 2: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = (#2 + #3) @@ -101,7 +114,7 @@ void abort(void) __attribute((noreturn)); // CHECK: noret int noret(int x) { // CHECK-NEXT: File 0, [[@LINE]]:18 -> [[@LINE+9]]:2 - switch (x) { + switch (x) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:14 -> [[@LINE+6]]:14 = 0 default: // CHECK
[PATCH] D70571: [Coverage] Emit a gap region to cover switch bodies
hans added a comment. Thanks! I'm not that familiar with the coverage counters, but this seems good as far as I can tell. Maybe add the test case from PR44011? I didn't see any test cases with that kind of gaps between cases. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70571/new/ https://reviews.llvm.org/D70571 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70571: [Coverage] Emit a gap region to cover switch bodies
vsk updated this revision to Diff 230714. vsk added a comment. - Add some documentation about clang internals. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70571/new/ https://reviews.llvm.org/D70571 Files: clang/docs/SourceBasedCodeCoverage.rst clang/lib/CodeGen/CoverageMappingGen.cpp clang/test/CoverageMapping/switch.cpp clang/test/CoverageMapping/switchmacro.c Index: clang/test/CoverageMapping/switchmacro.c === --- clang/test/CoverageMapping/switchmacro.c +++ clang/test/CoverageMapping/switchmacro.c @@ -4,7 +4,7 @@ // CHECK: foo int foo(int i) { // CHECK-NEXT: File 0, [[@LINE]]:16 -> {{[0-9]+}}:2 = #0 - switch (i) { + switch (i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:14 -> {{[0-9]+}}:11 = 0 default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> {{[0-9]+}}:11 = #2 if (i == 1) // CHECK-NEXT: File 0, [[@LINE]]:9 -> [[@LINE]]:15 = #2 return 0; // CHECK: File 0, [[@LINE]]:7 -> [[@LINE]]:15 = #3 Index: clang/test/CoverageMapping/switch.cpp === --- clang/test/CoverageMapping/switch.cpp +++ clang/test/CoverageMapping/switch.cpp @@ -2,11 +2,11 @@ // CHECK: foo void foo(int i) { // CHECK-NEXT: File 0, [[@LINE]]:17 -> [[@LINE+8]]:2 = #0 - switch(i) { + switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+4]]:10 = 0 case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:11 = #2 return; case 2: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #3 -break; // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = #1 +break; // CHECK-NEXT: Gap,File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = #1 } int x = 0;// CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:2 = #1 } @@ -29,7 +29,7 @@ nop(); switch (i) { // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+6]]:2 = #4 -nop(); // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE+2]]:10 = 0 +nop(); // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE+2]]:10 = 0 case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #7 nop(); } @@ -47,7 +47,7 @@ // CHECK-NEXT: main int main() {// CHECK-NEXT: File 0, [[@LINE]]:12 -> [[@LINE+35]]:2 = #0 int i = 0; - switch(i) { + switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+8]]:10 = 0 case 0: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = #2 i = 1; break; @@ -58,16 +58,16 @@ break; // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = #1 } switch(i) { // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+23]]:2 = #1 - case 0: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = #6 -i = 1; + case 0: // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:13 -> [[@LINE+6]]:10 = 0 +i = 1; // CHECK-NEXT: File 0, [[@LINE-1]]:3 -> [[@LINE+1]]:10 = #6 break; case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+3]]:10 = #7 i = 2; default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = (#7 + #8) break; // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+3]]:3 = #5 } - - switch(i) { // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+13]]:2 = #5 +// CHECK-NEXT: File 0, [[@LINE+1]]:3 -> [[@LINE+14]]:2 = #5 + switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+6]]:11 = 0 case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+5]]:11 = #10 case 2: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+4]]:11 = (#10 + #11) i = 11; @@ -85,7 +85,7 @@ // FIXME: End location for "case 1" shouldn't point at the end of the switch. // CHECK: fallthrough int fallthrough(int i) { // CHECK-NEXT: File 0, [[@LINE]]:24 -> [[@LINE+12]]:2 = #0 - switch(i) { + switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+9]]:10 = 0 case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+8]]:10 = #2 i = 23; case 2: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = (#2 + #3) @@ -101,7 +101,7 @@ void abort(void) __attribute((noreturn)); // CHECK: noret int noret(int x) { // CHECK-NEXT: File 0, [[@LINE]]:18 -> [[@LINE+9]]:2 - switch (x) { + switch (x) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:14 -> [[@LINE+6]]:14 = 0 default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:12 abort(); case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:13 Index: clang/lib/CodeGen/CoverageMappingGen.cpp === --- clang/lib/CodeGen/CoverageMappingGen.cpp +++ clang/lib/CodeGen/CoverageMappingGen.cpp @@ -1114,8 +1114,8 @@ // Make a region for the body of the switch. If the body starts with // a case, that case wi
[PATCH] D70571: [Coverage] Emit a gap region to cover switch bodies
vsk planned changes to this revision. vsk marked an inline comment as done. vsk added a comment. I'll write something up in the coverage mapping docs. Briefly, though, with this change there should be a gap region that covers the entire switch body (starting from the '{' in 'switch {', and terminating where the last case ends). Comment at: clang/test/CoverageMapping/switch.cpp:32 switch (i) { // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+6]]:2 = #4 -nop(); // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE+2]]:10 = 0 +nop(); // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE+2]]:10 = 0 case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #7 efriedma wrote: > I'm not sure I understand the effect here. Will we show that nop() never > executes, or will we not show any coverage information for it? The report will show an execution count of 0 for that call to nop(). In general a gap region's count is selected as the line count when it's the only region present in a line. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70571/new/ https://reviews.llvm.org/D70571 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70571: [Coverage] Emit a gap region to cover switch bodies
efriedma added a comment. Could you write a description somewhere of what the overall region tree should look like for a switch? Comment at: clang/test/CoverageMapping/switch.cpp:32 switch (i) { // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+6]]:2 = #4 -nop(); // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE+2]]:10 = 0 +nop(); // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE+2]]:10 = 0 case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #7 I'm not sure I understand the effect here. Will we show that nop() never executes, or will we not show any coverage information for it? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70571/new/ https://reviews.llvm.org/D70571 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70571: [Coverage] Emit a gap region to cover switch bodies
vsk created this revision. vsk added reviewers: hans, rnk, efriedma. vsk edited the summary of this revision. Emit a gap region beginning where the switch body begins. This sets line execution counts in the areas between non-overlapping cases to 0. This does not resolve an outstanding issue with case statement regions that do not end when a region is terminated. But it should address llvm.org/PR44011. https://reviews.llvm.org/D70571 Files: clang/lib/CodeGen/CoverageMappingGen.cpp clang/test/CoverageMapping/switch.cpp clang/test/CoverageMapping/switchmacro.c Index: clang/test/CoverageMapping/switchmacro.c === --- clang/test/CoverageMapping/switchmacro.c +++ clang/test/CoverageMapping/switchmacro.c @@ -4,7 +4,7 @@ // CHECK: foo int foo(int i) { // CHECK-NEXT: File 0, [[@LINE]]:16 -> {{[0-9]+}}:2 = #0 - switch (i) { + switch (i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:14 -> {{[0-9]+}}:11 = 0 default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> {{[0-9]+}}:11 = #2 if (i == 1) // CHECK-NEXT: File 0, [[@LINE]]:9 -> [[@LINE]]:15 = #2 return 0; // CHECK: File 0, [[@LINE]]:7 -> [[@LINE]]:15 = #3 Index: clang/test/CoverageMapping/switch.cpp === --- clang/test/CoverageMapping/switch.cpp +++ clang/test/CoverageMapping/switch.cpp @@ -2,11 +2,11 @@ // CHECK: foo void foo(int i) { // CHECK-NEXT: File 0, [[@LINE]]:17 -> [[@LINE+8]]:2 = #0 - switch(i) { + switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+4]]:10 = 0 case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:11 = #2 return; case 2: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #3 -break; // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = #1 +break; // CHECK-NEXT: Gap,File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = #1 } int x = 0;// CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:2 = #1 } @@ -29,7 +29,7 @@ nop(); switch (i) { // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+6]]:2 = #4 -nop(); // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE+2]]:10 = 0 +nop(); // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> [[@LINE+2]]:10 = 0 case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = #7 nop(); } @@ -47,7 +47,7 @@ // CHECK-NEXT: main int main() {// CHECK-NEXT: File 0, [[@LINE]]:12 -> [[@LINE+35]]:2 = #0 int i = 0; - switch(i) { + switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+8]]:10 = 0 case 0: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = #2 i = 1; break; @@ -58,16 +58,16 @@ break; // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+2]]:3 = #1 } switch(i) { // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+23]]:2 = #1 - case 0: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = #6 -i = 1; + case 0: // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:13 -> [[@LINE+6]]:10 = 0 +i = 1; // CHECK-NEXT: File 0, [[@LINE-1]]:3 -> [[@LINE+1]]:10 = #6 break; case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+3]]:10 = #7 i = 2; default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:10 = (#7 + #8) break; // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE+3]]:3 = #5 } - - switch(i) { // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+13]]:2 = #5 +// CHECK-NEXT: File 0, [[@LINE+1]]:3 -> [[@LINE+14]]:2 = #5 + switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+6]]:11 = 0 case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+5]]:11 = #10 case 2: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+4]]:11 = (#10 + #11) i = 11; @@ -85,7 +85,7 @@ // FIXME: End location for "case 1" shouldn't point at the end of the switch. // CHECK: fallthrough int fallthrough(int i) { // CHECK-NEXT: File 0, [[@LINE]]:24 -> [[@LINE+12]]:2 = #0 - switch(i) { + switch(i) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:13 -> [[@LINE+9]]:10 = 0 case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+8]]:10 = #2 i = 23; case 2: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+2]]:10 = (#2 + #3) @@ -101,7 +101,7 @@ void abort(void) __attribute((noreturn)); // CHECK: noret int noret(int x) { // CHECK-NEXT: File 0, [[@LINE]]:18 -> [[@LINE+9]]:2 - switch (x) { + switch (x) { // CHECK-NEXT: Gap,File 0, [[@LINE]]:14 -> [[@LINE+6]]:14 = 0 default: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:12 abort(); case 1: // CHECK-NEXT: File 0, [[@LINE]]:3 -> [[@LINE+1]]:13 Index: clang/lib/CodeGen/CoverageMappingGen.cpp === --- clang/lib/CodeGen/CoverageMappi