https://github.com/cor3ntin closed
https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/hanickadot updated
https://github.com/llvm/llvm-project/pull/77214
From 766df92436569a924ef1c3860b3e1019c2048b53 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hana=20Dusi=CC=81kova=CC=81?=
Date: Wed, 10 Jan 2024 10:00:31 +0100
Subject: [PATCH] [coverage] fix incorrect coverage
Hana =?utf-8?q?Dusi=CC=81kova=CC=81?=
Message-ID:
In-Reply-To:
cor3ntin wrote:
Oups, can you rebase?
https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
hanickadot wrote:
yes please, I don't have merge rights
https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Hana =?utf-8?q?Dusi=CC=81kova=CC=81?=
Message-ID:
In-Reply-To:
https://github.com/cor3ntin approved this pull request.
LGTM
@hanickadot do you need me to merge that for you?
https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing
Hana =?utf-8?q?Dusi=CC=81kova=CC=81?=
Message-ID:
In-Reply-To:
https://github.com/ornata approved this pull request.
https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
Hana =?utf-8?q?Dusi=CC=81kova=CC=81?=
Message-ID:
In-Reply-To:
ornata wrote:
LGTM
https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -7732,7 +7732,11 @@ TreeTransform::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
https://github.com/hanickadot updated
https://github.com/llvm/llvm-project/pull/77214
From 413517b2a1d4e45b6c58ab282c7990e83f429ab9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hana=20Dusi=CC=81kova=CC=81?=
Date: Mon, 8 Jan 2024 11:54:45 +0100
Subject: [PATCH 1/2] [coverage] fix incorrect coverage
@@ -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
+//
@@ -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
+//
@@ -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
-//
@@ -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
+//
@@ -7741,6 +7745,10 @@ TreeTransform::TransformIfStmt(IfStmt *S) {
Else = getDerived().TransformStmt(S->getElse());
if (Else.isInvalid())
return StmtError();
+ } else if (S->getElse() && ConstexprConditionValue &&
ornata wrote:
Could you add a
@@ -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
-//
@@ -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
+//
https://github.com/ornata commented:
Added some comments, mostly nits on the test.
https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -7732,7 +7732,11 @@ TreeTransform::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
@@ -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
+//
https://github.com/ornata edited https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hanickadot wrote:
Added some tests. And also adding @ornata for review.
https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/hanickadot updated
https://github.com/llvm/llvm-project/pull/77214
From 413517b2a1d4e45b6c58ab282c7990e83f429ab9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hana=20Dusi=CC=81kova=CC=81?=
Date: Mon, 8 Jan 2024 11:54:45 +0100
Subject: [PATCH] [coverage] fix incorrect coverage
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?=
Date: Sun, 7 Jan 2024 00:13:08 +0100
Subject: [PATCH 1/2] [coverage] fix incorrect coverage
https://github.com/hanickadot updated
https://github.com/llvm/llvm-project/pull/77214
From 5aa041529997724e19109c45cf1483eb72d7dab6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hana=20Dusi=CC=81kova=CC=81?=
Date: Sun, 7 Jan 2024 00:13:08 +0100
Subject: [PATCH] [coverage] fix incorrect coverage
hanickadot wrote:
Yes, and it will be a bit bigger change. This is currently my biggest change
yet :) But I want to do it as next. This fix's intention is to make the source
location of regions properly done.
https://github.com/llvm/llvm-project/pull/77214
chfast wrote:
> But in future it would be useful to mark whole gap there as skipped instead.
> If there is interest I would do it in another PR.
This would be very useful not to treat compile-time unreachable code as
uncovered. Can the skipped code be marked separately in the reports?
https://github.com/hanickadot updated
https://github.com/llvm/llvm-project/pull/77214
From fb2efdecce3a629c801d1f179297b1189e1ec287 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hana=20Dusi=CC=81kova=CC=81?=
Date: Sun, 7 Jan 2024 00:13:08 +0100
Subject: [PATCH] [coverage] fix incorrect coverage
@@ -7732,7 +7732,8 @@ TreeTransform::TransformIfStmt(IfStmt *S) {
if (Then.isInvalid())
return StmtError();
} else {
-Then = new (getSema().Context) NullStmt(S->getThen()->getBeginLoc());
+Then = new (getSema().Context)
+
cor3ntin wrote:
Adding a few folks to the review (We do not seem to update the coverage mapping
very often)
https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://github.com/hanickadot updated
https://github.com/llvm/llvm-project/pull/77214
From 1891847f50311c4ffbaa40577900eca2245c0a7c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hana=20Dusi=CC=81kova=CC=81?=
Date: Sun, 7 Jan 2024 00:13:08 +0100
Subject: [PATCH] [coverage] fix incorrect coverage
@@ -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) {
+
hanickadot wrote:
Fixes https://github.com/llvm/llvm-project/issues/54419.
https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -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) {
+
https://github.com/cor3ntin edited
https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/cor3ntin commented:
Thanks for the patch!
I think this might need a release note (referencing the couple open issues
about this bug), and probably some tests.
https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing
@@ -7732,7 +7732,8 @@ TreeTransform::TransformIfStmt(IfStmt *S) {
if (Then.isInvalid())
return StmtError();
} else {
-Then = new (getSema().Context) NullStmt(S->getThen()->getBeginLoc());
+Then = new (getSema().Context)
+
https://github.com/hanickadot updated
https://github.com/llvm/llvm-project/pull/77214
From bde3362e609a129f410890745b1dcfd635ba2ec0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hana=20Dusi=CC=81kova=CC=81?=
Date: Sun, 7 Jan 2024 00:13:08 +0100
Subject: [PATCH] [coverage] fix incorrect coverage
https://github.com/hanickadot updated
https://github.com/llvm/llvm-project/pull/77214
From 8f1370aae4db2048c35516a85fb72c742557942b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hana=20Dusi=CC=81kova=CC=81?=
Date: Sun, 7 Jan 2024 00:13:08 +0100
Subject: [PATCH] [coverage] fix incorrect coverage
https://github.com/hanickadot edited
https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/hanickadot updated
https://github.com/llvm/llvm-project/pull/77214
From 8f1370aae4db2048c35516a85fb72c742557942b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hana=20Dusi=CC=81kova=CC=81?=
Date: Sun, 7 Jan 2024 00:13:08 +0100
Subject: [PATCH] [coverage] fix incorrect coverage
hanickadot wrote:
https://github.com/llvm/llvm-project/assets/6557263/47ff77c0-9101-44cf-b2d5-ffea514bfc0c;>
(notice wrong coverage "if constexpr" for positive one, and completely missing
for negative one, also notice "if consteval" marking always the same branch as
uncovered)
https://github.com/hanickadot edited
https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Hana Dusíková (hanickadot)
Changes
It was a while since I noticed coverage report is broken for "if constexpr" and
"if consteval" (as shown on first picture).
img width="453" alt="Screenshot 2024-01-07 at 00 29 17"
https://github.com/hanickadot created
https://github.com/llvm/llvm-project/pull/77214
It was a while since I noticed coverage report is broken for "if constexpr" and
"if consteval" (as shown on first picture).
44 matches
Mail list logo
