[clang] [clang][coverage] fixing "if constexpr" and "if consteval" coverage report (PR #77214)

Hana Dusíková via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 8 02:52:44 PST 2024


https://github.com/hanickadot updated https://github.com/llvm/llvm-project/pull/77214

>From 2dc9d821aa3d100f99ac603a0880498226c94323 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hana=20Dusi=CC=81kova=CC=81?= <hanicka at hanicka.net>
Date: Sun, 7 Jan 2024 00:13:08 +0100
Subject: [PATCH 1/2] [coverage] fix incorrect coverage reporting for "if
 constexpr" and "if consteval"

---
 clang/docs/ReleaseNotes.rst              |  3 +++
 clang/include/clang/AST/Stmt.h           |  6 ++++--
 clang/lib/CodeGen/CoverageMappingGen.cpp | 13 +++++++++++--
 clang/lib/Sema/TreeTransform.h           | 10 +++++++++-
 4 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c9b577bd549b1e..c36e051d055b72 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -696,6 +696,9 @@ Bug Fixes in This Version
 - Clang now accepts recursive non-dependent calls to functions with deduced
   return type.
   Fixes (`#71015 <https://github.com/llvm/llvm-project/issues/71015>`_)
+- Clang now emits correct source location for code-coverage regions in `if constexpr`
+  and `if consteval` branches.
+  Fixes (`#54419 <https://github.com/llvm/llvm-project/issues/54419>`_)
 
 
 Bug Fixes to Compiler Builtins
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index da7b37ce0e1211..e1fde24e647789 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -1631,8 +1631,10 @@ class CompoundStmt final
                               SourceLocation RB);
 
   // Build an empty compound statement with a location.
-  explicit CompoundStmt(SourceLocation Loc)
-      : Stmt(CompoundStmtClass), LBraceLoc(Loc), RBraceLoc(Loc) {
+  explicit CompoundStmt(SourceLocation Loc) : CompoundStmt(Loc, Loc) {}
+
+  CompoundStmt(SourceLocation Loc, SourceLocation EndLoc)
+      : Stmt(CompoundStmtClass), LBraceLoc(Loc), RBraceLoc(EndLoc) {
     CompoundStmtBits.NumStmts = 0;
     CompoundStmtBits.HasFPFeatures = 0;
   }
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
index bf227386a71b78..b245abd16c3f4a 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1712,7 +1712,11 @@ struct CounterCoverageMappingBuilder
       extendRegion(S->getCond());
 
     Counter ParentCount = getRegion().getCounter();
-    Counter ThenCount = getRegionCounter(S);
+
+    // If this is "if !consteval" the then-branch will never be taken, we don't
+    // need to change counter
+    Counter ThenCount =
+        S->isNegatedConsteval() ? ParentCount : getRegionCounter(S);
 
     if (!S->isConsteval()) {
       // Emitting a counter for the condition makes it easier to interpret the
@@ -1729,7 +1733,12 @@ struct CounterCoverageMappingBuilder
     extendRegion(S->getThen());
     Counter OutCount = propagateCounts(ThenCount, S->getThen());
 
-    Counter ElseCount = subtractCounters(ParentCount, ThenCount);
+    // If this is "if consteval" the else-branch will never be taken, we don't
+    // need to change counter
+    Counter ElseCount = S->isNonNegatedConsteval()
+                            ? ParentCount
+                            : subtractCounters(ParentCount, ThenCount);
+
     if (const Stmt *Else = S->getElse()) {
       bool ThenHasTerminateStmt = HasTerminateStmt;
       HasTerminateStmt = false;
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index c8c5a51bf9f94e..19266a7ffa2fe0 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -7732,7 +7732,11 @@ TreeTransform<Derived>::TransformIfStmt(IfStmt *S) {
     if (Then.isInvalid())
       return StmtError();
   } else {
-    Then = new (getSema().Context) NullStmt(S->getThen()->getBeginLoc());
+    // Discarded branch is replaced with empty CompoundStmt so we can keep
+    // proper source location for start and end of original branch, so
+    // subsequent transformations like CoverageMapping work properly
+    Then = new (getSema().Context)
+        CompoundStmt(S->getThen()->getBeginLoc(), S->getThen()->getEndLoc());
   }
 
   // Transform the "else" branch.
@@ -7741,6 +7745,10 @@ TreeTransform<Derived>::TransformIfStmt(IfStmt *S) {
     Else = getDerived().TransformStmt(S->getElse());
     if (Else.isInvalid())
       return StmtError();
+  } else if (S->getElse() && ConstexprConditionValue &&
+             *ConstexprConditionValue) {
+    Else = new (getSema().Context)
+        CompoundStmt(S->getElse()->getBeginLoc(), S->getElse()->getEndLoc());
   }
 
   if (!getDerived().AlwaysRebuild() &&

>From 37879a5b7ec3f98ce423672d2bf43efe826ab5af Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hana=20Dusi=CC=81kova=CC=81?= <hanicka at hanicka.net>
Date: Sun, 7 Jan 2024 00:13:08 +0100
Subject: [PATCH 2/2] [coverage] fix incorrect coverage reporting for "if
 constexpr" and "if consteval"

---
 clang/test/CoverageMapping/if.cpp | 86 +++++++++++++++++++++++++------
 1 file changed, 69 insertions(+), 17 deletions(-)

diff --git a/clang/test/CoverageMapping/if.cpp b/clang/test/CoverageMapping/if.cpp
index 65e3d62df79db4..cc18f791c2f79e 100644
--- a/clang/test/CoverageMapping/if.cpp
+++ b/clang/test/CoverageMapping/if.cpp
@@ -23,19 +23,29 @@ void foo() {                    // CHECK-NEXT: Gap,File 0, [[@LINE+1]]:21 -> [[@
 }                               // CHECK-NEXT: [[@LINE-2]]:9 -> [[@LINE-1]]:5 = #1
                                 // CHECK-NEXT: [[@LINE-2]]:5 -> [[@LINE-2]]:8 = #1
 
-// FIXME: Do not generate coverage for discarded branches in if consteval and if constexpr statements
-constexpr int check_consteval(int i) {
-    if consteval {
-      i++;
-    }
-    if !consteval {
-      i++;
-    }
-    if consteval {
-        return 42;
-    } else {
-        return i;
-    }
+// FIXME: Do not generate coverage for discarded branches in if constexpr
+// CHECK-LABEL: _Z16check_constexprAi:
+int check_constexprA(int i) {   // CHECK-NEXT: [[@LINE]]:29 -> {{[0-9]+}}:2 = #0
+                                // CHECK-NEXT: [[@LINE+2]]:16 -> [[@LINE+2]]:20 = #0
+                                // CHECK-NEXT: Branch,File 0, [[@LINE+1]]:16 -> [[@LINE+1]]:20 = 0, 0
+  if constexpr(true) {          // CHECK-NEXT: Gap,File 0, [[@LINE]]:21 -> [[@LINE]]:22 = #1
+    i *= 3;                     // CHECK-NEXT: [[@LINE-1]]:22 -> [[@LINE+1]]:4 = #1
+  } else {                      // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 -> [[@LINE]]:10 = (#0 - #1)
+    i *= 5;                     // CHECK-NEXT: [[@LINE-1]]:10 -> [[@LINE+1]]:4 = (#0 - #1)
+  }
+  return i;
+}
+
+// CHECK-LABEL: _Z16check_constexprBi:
+int check_constexprB(int i) {   // CHECK-NEXT: [[@LINE]]:29 -> {{[0-9]+}}:2 = #0
+                                // CHECK-NEXT: [[@LINE+2]]:16 -> [[@LINE+2]]:21 = #0
+                                // CHECK-NEXT: Branch,File 0, [[@LINE+1]]:16 -> [[@LINE+1]]:21 = 0, 0
+  if constexpr(false) {         // CHECK-NEXT: Gap,File 0, [[@LINE]]:22 -> [[@LINE]]:23 = #1
+    i *= 3;                     // CHECK-NEXT: File 0, [[@LINE-1]]:23 -> [[@LINE+1]]:4 = #1
+  } else {                      // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 -> [[@LINE]]:10 = (#0 - #1)
+    i *= 5;                     // CHECK-NEXT: File 0, [[@LINE-1]]:10 -> [[@LINE+1]]:4 = (#0 - #1)
+  }
+  return i;
 }
 
 // CHECK-LABEL: main:
@@ -75,10 +85,6 @@ int main() {                    // CHECK: File 0, [[@LINE]]:12 -> {{[0-9]+}}:2 =
                                 // CHECK-NEXT: File 0, [[@LINE+1]]:14 -> [[@LINE+1]]:20 = #6
   i = i == 0?i + 12:i + 10;     // CHECK-NEXT: File 0, [[@LINE]]:21 -> [[@LINE]]:27 = (#0 - #6)
 
-  // GH-57377
-  constexpr int c_i = check_consteval(0);
-  check_consteval(i);
-
   // GH-45481
   S s;                    
   s.the_prop = 0? 1 : 2;        // CHECK-NEXT: File 0, [[@LINE]]:16 -> [[@LINE]]:17 = #0
@@ -98,3 +104,49 @@ int main() {                    // CHECK: File 0, [[@LINE]]:12 -> {{[0-9]+}}:2 =
 void ternary() {
   true ? FOO : FOO; // CHECK-NOT: Gap,{{.*}}, [[@LINE]]:8 ->
 }
+
+// FIXME: Do not generate coverage for discarded branches in if consteval
+// GH-57377
+// CHECK-LABEL: _Z16check_constevalAi:
+constexpr int check_constevalA(int i) { // CHECK-NEXT: [[@LINE]]:39 -> {{[0-9]+}}:2 = #0
+  if consteval {                        
+    i *= 3;                             // CHECK-NEXT: [[@LINE-1]]:16 -> [[@LINE+1]]:4 = #1
+  } else {                              // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 -> [[@LINE]]:10 = #0
+    i *= 5;                             // CHECK-NEXT: [[@LINE-1]]:10 -> [[@LINE+1]]:4 = #0
+  }
+  return i;                             // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE]]:11 = (#0 + #1)
+}
+
+// CHECK-LABEL: _Z16check_constevalBi:
+constexpr int check_constevalB(int i) { // CHECK-NEXT: [[@LINE]]:39 -> {{[0-9]+}}:2 = #0
+  if !consteval {                        
+    i *= 3;                             // CHECK-NEXT: [[@LINE-1]]:17 -> [[@LINE+1]]:4 = #0
+  } else {                              // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 -> [[@LINE]]:10 = 0
+    i *= 5;                             // CHECK-NEXT: [[@LINE-1]]:10 -> [[@LINE+1]]:4 = 0
+  }
+  return i;
+}
+
+// CHECK-LABEL: _Z16check_constevalCi:
+constexpr int check_constevalC(int i) { // CHECK-NEXT: [[@LINE]]:39 -> {{[0-9]+}}:2 = #0
+  if consteval {                        
+    i *= 3;                             // CHECK-NEXT: [[@LINE-1]]:16 -> [[@LINE+1]]:4 = #1
+  }
+  return i;                             // CHECK-NEXT: [[@LINE]]:3 -> [[@LINE]]:11 = (#0 + #1)
+}
+
+// CHECK-LABEL: _Z16check_constevalDi:
+constexpr int check_constevalD(int i) { // CHECK-NEXT: [[@LINE]]:39 -> {{[0-9]+}}:2 = #0
+  if !consteval {                        
+    i *= 3;                             // CHECK-NEXT: [[@LINE-1]]:17 -> [[@LINE+1]]:4 = #0
+  }
+  return i;
+}
+
+int instantiate_consteval(int i) {
+  i *= check_constevalA(i);
+  i *= check_constevalB(i);
+  i *= check_constevalC(i);
+  i *= check_constevalD(i);
+  return i;
+}



More information about the cfe-commits mailing list