[llvm-branch-commits] [clang] [MC/DC] Enable nested expressions (PR #125413)
https://github.com/chapuni updated
https://github.com/llvm/llvm-project/pull/125413
>From c56ecc30e9fd1a674073e362fbfcc6b43f2f52e2 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi
Date: Sun, 2 Feb 2025 22:06:32 +0900
Subject: [PATCH 1/5] [MC/DC] Enable nested expressions
A warning "contains an operation with a nested boolean expression." is
no longer emitter. At the moment, split expressions are treated as
individual Decisions.
---
clang/lib/CodeGen/CodeGenPGO.cpp | 150 ++
.../test/CoverageMapping/mcdc-nested-expr.cpp | 30 +++-
.../Frontend/custom-diag-werror-interaction.c | 4 +-
3 files changed, 109 insertions(+), 75 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 7d16f673ada41..0c3973aa4dccf 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -228,10 +228,17 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
/// The stacks are also used to find error cases and notify the user. A
/// standard logical operator nest for a boolean expression could be in a
form
/// similar to this: "x = a && b && c && (d || f)"
- unsigned NumCond = 0;
- bool SplitNestedLogicalOp = false;
- SmallVector NonLogOpStack;
- SmallVector LogOpStack;
+ struct DecisionState {
+llvm::DenseSet Leaves; // Not BinOp
+const Expr *DecisionExpr;// Root
+bool Split;
+
+DecisionState() = delete;
+DecisionState(const Expr *E, bool Split = false)
+: DecisionExpr(E), Split(Split) {}
+ };
+
+ SmallVector DecisionStack;
// Hook: dataTraverseStmtPre() is invoked prior to visiting an AST Stmt node.
bool dataTraverseStmtPre(Stmt *S) {
@@ -239,34 +246,28 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
if (MCDCMaxCond == 0)
return true;
-/// At the top of the logical operator nest, reset the number of
conditions,
-/// also forget previously seen split nesting cases.
-if (LogOpStack.empty()) {
- NumCond = 0;
- SplitNestedLogicalOp = false;
-}
-
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-/// Check for "split-nested" logical operators. This happens when a new
-/// boolean expression logical-op nest is encountered within an
existing
-/// boolean expression, separated by a non-logical operator. For
-/// example, in "x = (a && b && c && foo(d && f))", the "d && f" case
-/// starts a new boolean expression that is separated from the other
-/// conditions by the operator foo(). Split-nested cases are not
-/// supported by MC/DC.
-SplitNestedLogicalOp = SplitNestedLogicalOp || !NonLogOpStack.empty();
-
-LogOpStack.push_back(BinOp);
+/// Mark "in splitting" when a leaf is met.
+if (!DecisionStack.empty()) {
+ auto &StackTop = DecisionStack.back();
+ if (!StackTop.Split) {
+if (StackTop.Leaves.contains(S)) {
+ assert(!StackTop.Split);
+ StackTop.Split = true;
+}
return true;
}
+
+ // Split
+ assert(StackTop.Split);
+ assert(!StackTop.Leaves.contains(S));
}
-/// Keep track of non-logical operators. These are OK as long as we don't
-/// encounter a new logical operator after seeing one.
-if (!LogOpStack.empty())
- NonLogOpStack.push_back(S);
+if (const auto *E = dyn_cast(S)) {
+ if (const auto *BinOp =
+ dyn_cast(CodeGenFunction::stripCond(E));
+ BinOp && BinOp->isLogicalOp())
+DecisionStack.emplace_back(E);
+}
return true;
}
@@ -275,49 +276,57 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
// an AST Stmt node. MC/DC will use it to to signal when the top of a
// logical operation (boolean expression) nest is encountered.
bool dataTraverseStmtPost(Stmt *S) {
-/// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
-if (MCDCMaxCond == 0)
+if (DecisionStack.empty())
return true;
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-assert(LogOpStack.back() == BinOp);
-LogOpStack.pop_back();
-
-/// At the top of logical operator nest:
-if (LogOpStack.empty()) {
- /// Was the "split-nested" logical operator case encountered?
- if (SplitNestedLogicalOp) {
-unsigned DiagID = Diag.getCustomDiagID(
-DiagnosticsEngine::Warning,
-"unsupported MC/DC boolean expression; "
-"contains an operation with a nested boolean expression. "
-"Expression will not be covered");
-Diag.Report(S->getBeginLoc(), DiagID);
-return true;
- }
-
- /// Was the maximum number of conditions encoun
[llvm-branch-commits] [clang] [MC/DC] Enable nested expressions (PR #125413)
https://github.com/chapuni updated
https://github.com/llvm/llvm-project/pull/125413
>From c56ecc30e9fd1a674073e362fbfcc6b43f2f52e2 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi
Date: Sun, 2 Feb 2025 22:06:32 +0900
Subject: [PATCH 1/5] [MC/DC] Enable nested expressions
A warning "contains an operation with a nested boolean expression." is
no longer emitter. At the moment, split expressions are treated as
individual Decisions.
---
clang/lib/CodeGen/CodeGenPGO.cpp | 150 ++
.../test/CoverageMapping/mcdc-nested-expr.cpp | 30 +++-
.../Frontend/custom-diag-werror-interaction.c | 4 +-
3 files changed, 109 insertions(+), 75 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 7d16f673ada41..0c3973aa4dccf 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -228,10 +228,17 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
/// The stacks are also used to find error cases and notify the user. A
/// standard logical operator nest for a boolean expression could be in a
form
/// similar to this: "x = a && b && c && (d || f)"
- unsigned NumCond = 0;
- bool SplitNestedLogicalOp = false;
- SmallVector NonLogOpStack;
- SmallVector LogOpStack;
+ struct DecisionState {
+llvm::DenseSet Leaves; // Not BinOp
+const Expr *DecisionExpr;// Root
+bool Split;
+
+DecisionState() = delete;
+DecisionState(const Expr *E, bool Split = false)
+: DecisionExpr(E), Split(Split) {}
+ };
+
+ SmallVector DecisionStack;
// Hook: dataTraverseStmtPre() is invoked prior to visiting an AST Stmt node.
bool dataTraverseStmtPre(Stmt *S) {
@@ -239,34 +246,28 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
if (MCDCMaxCond == 0)
return true;
-/// At the top of the logical operator nest, reset the number of
conditions,
-/// also forget previously seen split nesting cases.
-if (LogOpStack.empty()) {
- NumCond = 0;
- SplitNestedLogicalOp = false;
-}
-
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-/// Check for "split-nested" logical operators. This happens when a new
-/// boolean expression logical-op nest is encountered within an
existing
-/// boolean expression, separated by a non-logical operator. For
-/// example, in "x = (a && b && c && foo(d && f))", the "d && f" case
-/// starts a new boolean expression that is separated from the other
-/// conditions by the operator foo(). Split-nested cases are not
-/// supported by MC/DC.
-SplitNestedLogicalOp = SplitNestedLogicalOp || !NonLogOpStack.empty();
-
-LogOpStack.push_back(BinOp);
+/// Mark "in splitting" when a leaf is met.
+if (!DecisionStack.empty()) {
+ auto &StackTop = DecisionStack.back();
+ if (!StackTop.Split) {
+if (StackTop.Leaves.contains(S)) {
+ assert(!StackTop.Split);
+ StackTop.Split = true;
+}
return true;
}
+
+ // Split
+ assert(StackTop.Split);
+ assert(!StackTop.Leaves.contains(S));
}
-/// Keep track of non-logical operators. These are OK as long as we don't
-/// encounter a new logical operator after seeing one.
-if (!LogOpStack.empty())
- NonLogOpStack.push_back(S);
+if (const auto *E = dyn_cast(S)) {
+ if (const auto *BinOp =
+ dyn_cast(CodeGenFunction::stripCond(E));
+ BinOp && BinOp->isLogicalOp())
+DecisionStack.emplace_back(E);
+}
return true;
}
@@ -275,49 +276,57 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
// an AST Stmt node. MC/DC will use it to to signal when the top of a
// logical operation (boolean expression) nest is encountered.
bool dataTraverseStmtPost(Stmt *S) {
-/// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
-if (MCDCMaxCond == 0)
+if (DecisionStack.empty())
return true;
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-assert(LogOpStack.back() == BinOp);
-LogOpStack.pop_back();
-
-/// At the top of logical operator nest:
-if (LogOpStack.empty()) {
- /// Was the "split-nested" logical operator case encountered?
- if (SplitNestedLogicalOp) {
-unsigned DiagID = Diag.getCustomDiagID(
-DiagnosticsEngine::Warning,
-"unsupported MC/DC boolean expression; "
-"contains an operation with a nested boolean expression. "
-"Expression will not be covered");
-Diag.Report(S->getBeginLoc(), DiagID);
-return true;
- }
-
- /// Was the maximum number of conditions encoun
[llvm-branch-commits] [clang] [MC/DC] Enable nested expressions (PR #125413)
https://github.com/chapuni edited https://github.com/llvm/llvm-project/pull/125413 ___ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [MC/DC] Enable nested expressions (PR #125413)
https://github.com/chapuni updated
https://github.com/llvm/llvm-project/pull/125413
>From c56ecc30e9fd1a674073e362fbfcc6b43f2f52e2 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi
Date: Sun, 2 Feb 2025 22:06:32 +0900
Subject: [PATCH 1/5] [MC/DC] Enable nested expressions
A warning "contains an operation with a nested boolean expression." is
no longer emitter. At the moment, split expressions are treated as
individual Decisions.
---
clang/lib/CodeGen/CodeGenPGO.cpp | 150 ++
.../test/CoverageMapping/mcdc-nested-expr.cpp | 30 +++-
.../Frontend/custom-diag-werror-interaction.c | 4 +-
3 files changed, 109 insertions(+), 75 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 7d16f673ada41..0c3973aa4dccf 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -228,10 +228,17 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
/// The stacks are also used to find error cases and notify the user. A
/// standard logical operator nest for a boolean expression could be in a
form
/// similar to this: "x = a && b && c && (d || f)"
- unsigned NumCond = 0;
- bool SplitNestedLogicalOp = false;
- SmallVector NonLogOpStack;
- SmallVector LogOpStack;
+ struct DecisionState {
+llvm::DenseSet Leaves; // Not BinOp
+const Expr *DecisionExpr;// Root
+bool Split;
+
+DecisionState() = delete;
+DecisionState(const Expr *E, bool Split = false)
+: DecisionExpr(E), Split(Split) {}
+ };
+
+ SmallVector DecisionStack;
// Hook: dataTraverseStmtPre() is invoked prior to visiting an AST Stmt node.
bool dataTraverseStmtPre(Stmt *S) {
@@ -239,34 +246,28 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
if (MCDCMaxCond == 0)
return true;
-/// At the top of the logical operator nest, reset the number of
conditions,
-/// also forget previously seen split nesting cases.
-if (LogOpStack.empty()) {
- NumCond = 0;
- SplitNestedLogicalOp = false;
-}
-
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-/// Check for "split-nested" logical operators. This happens when a new
-/// boolean expression logical-op nest is encountered within an
existing
-/// boolean expression, separated by a non-logical operator. For
-/// example, in "x = (a && b && c && foo(d && f))", the "d && f" case
-/// starts a new boolean expression that is separated from the other
-/// conditions by the operator foo(). Split-nested cases are not
-/// supported by MC/DC.
-SplitNestedLogicalOp = SplitNestedLogicalOp || !NonLogOpStack.empty();
-
-LogOpStack.push_back(BinOp);
+/// Mark "in splitting" when a leaf is met.
+if (!DecisionStack.empty()) {
+ auto &StackTop = DecisionStack.back();
+ if (!StackTop.Split) {
+if (StackTop.Leaves.contains(S)) {
+ assert(!StackTop.Split);
+ StackTop.Split = true;
+}
return true;
}
+
+ // Split
+ assert(StackTop.Split);
+ assert(!StackTop.Leaves.contains(S));
}
-/// Keep track of non-logical operators. These are OK as long as we don't
-/// encounter a new logical operator after seeing one.
-if (!LogOpStack.empty())
- NonLogOpStack.push_back(S);
+if (const auto *E = dyn_cast(S)) {
+ if (const auto *BinOp =
+ dyn_cast(CodeGenFunction::stripCond(E));
+ BinOp && BinOp->isLogicalOp())
+DecisionStack.emplace_back(E);
+}
return true;
}
@@ -275,49 +276,57 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
// an AST Stmt node. MC/DC will use it to to signal when the top of a
// logical operation (boolean expression) nest is encountered.
bool dataTraverseStmtPost(Stmt *S) {
-/// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
-if (MCDCMaxCond == 0)
+if (DecisionStack.empty())
return true;
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-assert(LogOpStack.back() == BinOp);
-LogOpStack.pop_back();
-
-/// At the top of logical operator nest:
-if (LogOpStack.empty()) {
- /// Was the "split-nested" logical operator case encountered?
- if (SplitNestedLogicalOp) {
-unsigned DiagID = Diag.getCustomDiagID(
-DiagnosticsEngine::Warning,
-"unsupported MC/DC boolean expression; "
-"contains an operation with a nested boolean expression. "
-"Expression will not be covered");
-Diag.Report(S->getBeginLoc(), DiagID);
-return true;
- }
-
- /// Was the maximum number of conditions encoun
[llvm-branch-commits] [clang] [MC/DC] Enable nested expressions (PR #125413)
https://github.com/chapuni updated
https://github.com/llvm/llvm-project/pull/125413
>From c56ecc30e9fd1a674073e362fbfcc6b43f2f52e2 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi
Date: Sun, 2 Feb 2025 22:06:32 +0900
Subject: [PATCH 1/5] [MC/DC] Enable nested expressions
A warning "contains an operation with a nested boolean expression." is
no longer emitter. At the moment, split expressions are treated as
individual Decisions.
---
clang/lib/CodeGen/CodeGenPGO.cpp | 150 ++
.../test/CoverageMapping/mcdc-nested-expr.cpp | 30 +++-
.../Frontend/custom-diag-werror-interaction.c | 4 +-
3 files changed, 109 insertions(+), 75 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 7d16f673ada41..0c3973aa4dccf 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -228,10 +228,17 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
/// The stacks are also used to find error cases and notify the user. A
/// standard logical operator nest for a boolean expression could be in a
form
/// similar to this: "x = a && b && c && (d || f)"
- unsigned NumCond = 0;
- bool SplitNestedLogicalOp = false;
- SmallVector NonLogOpStack;
- SmallVector LogOpStack;
+ struct DecisionState {
+llvm::DenseSet Leaves; // Not BinOp
+const Expr *DecisionExpr;// Root
+bool Split;
+
+DecisionState() = delete;
+DecisionState(const Expr *E, bool Split = false)
+: DecisionExpr(E), Split(Split) {}
+ };
+
+ SmallVector DecisionStack;
// Hook: dataTraverseStmtPre() is invoked prior to visiting an AST Stmt node.
bool dataTraverseStmtPre(Stmt *S) {
@@ -239,34 +246,28 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
if (MCDCMaxCond == 0)
return true;
-/// At the top of the logical operator nest, reset the number of
conditions,
-/// also forget previously seen split nesting cases.
-if (LogOpStack.empty()) {
- NumCond = 0;
- SplitNestedLogicalOp = false;
-}
-
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-/// Check for "split-nested" logical operators. This happens when a new
-/// boolean expression logical-op nest is encountered within an
existing
-/// boolean expression, separated by a non-logical operator. For
-/// example, in "x = (a && b && c && foo(d && f))", the "d && f" case
-/// starts a new boolean expression that is separated from the other
-/// conditions by the operator foo(). Split-nested cases are not
-/// supported by MC/DC.
-SplitNestedLogicalOp = SplitNestedLogicalOp || !NonLogOpStack.empty();
-
-LogOpStack.push_back(BinOp);
+/// Mark "in splitting" when a leaf is met.
+if (!DecisionStack.empty()) {
+ auto &StackTop = DecisionStack.back();
+ if (!StackTop.Split) {
+if (StackTop.Leaves.contains(S)) {
+ assert(!StackTop.Split);
+ StackTop.Split = true;
+}
return true;
}
+
+ // Split
+ assert(StackTop.Split);
+ assert(!StackTop.Leaves.contains(S));
}
-/// Keep track of non-logical operators. These are OK as long as we don't
-/// encounter a new logical operator after seeing one.
-if (!LogOpStack.empty())
- NonLogOpStack.push_back(S);
+if (const auto *E = dyn_cast(S)) {
+ if (const auto *BinOp =
+ dyn_cast(CodeGenFunction::stripCond(E));
+ BinOp && BinOp->isLogicalOp())
+DecisionStack.emplace_back(E);
+}
return true;
}
@@ -275,49 +276,57 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
// an AST Stmt node. MC/DC will use it to to signal when the top of a
// logical operation (boolean expression) nest is encountered.
bool dataTraverseStmtPost(Stmt *S) {
-/// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
-if (MCDCMaxCond == 0)
+if (DecisionStack.empty())
return true;
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-assert(LogOpStack.back() == BinOp);
-LogOpStack.pop_back();
-
-/// At the top of logical operator nest:
-if (LogOpStack.empty()) {
- /// Was the "split-nested" logical operator case encountered?
- if (SplitNestedLogicalOp) {
-unsigned DiagID = Diag.getCustomDiagID(
-DiagnosticsEngine::Warning,
-"unsupported MC/DC boolean expression; "
-"contains an operation with a nested boolean expression. "
-"Expression will not be covered");
-Diag.Report(S->getBeginLoc(), DiagID);
-return true;
- }
-
- /// Was the maximum number of conditions encoun
[llvm-branch-commits] [clang] [MC/DC] Enable nested expressions (PR #125413)
@@ -275,49 +276,57 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
// an AST Stmt node. MC/DC will use it to to signal when the top of a
// logical operation (boolean expression) nest is encountered.
bool dataTraverseStmtPost(Stmt *S) {
-/// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
-if (MCDCMaxCond == 0)
+if (DecisionStack.empty())
return true;
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-assert(LogOpStack.back() == BinOp);
-LogOpStack.pop_back();
-
-/// At the top of logical operator nest:
-if (LogOpStack.empty()) {
- /// Was the "split-nested" logical operator case encountered?
- if (SplitNestedLogicalOp) {
-unsigned DiagID = Diag.getCustomDiagID(
-DiagnosticsEngine::Warning,
-"unsupported MC/DC boolean expression; "
-"contains an operation with a nested boolean expression. "
-"Expression will not be covered");
-Diag.Report(S->getBeginLoc(), DiagID);
-return true;
- }
-
- /// Was the maximum number of conditions encountered?
- if (NumCond > MCDCMaxCond) {
-unsigned DiagID = Diag.getCustomDiagID(
-DiagnosticsEngine::Warning,
-"unsupported MC/DC boolean expression; "
-"number of conditions (%0) exceeds max (%1). "
-"Expression will not be covered");
-Diag.Report(S->getBeginLoc(), DiagID) << NumCond << MCDCMaxCond;
-return true;
- }
-
- // Otherwise, allocate the Decision.
- MCDCState.DecisionByStmt[BinOp].ID = MCDCState.DecisionByStmt.size();
-}
-return true;
+/// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
+assert(MCDCMaxCond > 0);
+
+auto &StackTop = DecisionStack.back();
+
+if (StackTop.DecisionExpr != S) {
+ if (StackTop.Leaves.contains(S)) {
+assert(StackTop.Split);
+StackTop.Split = false;
}
+
+ return true;
+}
+
+/// Allocate the entry (with Valid=false)
+auto &DecisionEntry =
+MCDCState
+.DecisionByStmt[CodeGenFunction::stripCond(StackTop.DecisionExpr)];
+
+/// Was the "split-nested" logical operator case encountered?
+if (false && DecisionStack.size() > 1) {
chapuni wrote:
Removed.
https://github.com/llvm/llvm-project/pull/125413
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [MC/DC] Enable nested expressions (PR #125413)
https://github.com/chapuni updated
https://github.com/llvm/llvm-project/pull/125413
>From c56ecc30e9fd1a674073e362fbfcc6b43f2f52e2 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi
Date: Sun, 2 Feb 2025 22:06:32 +0900
Subject: [PATCH 1/4] [MC/DC] Enable nested expressions
A warning "contains an operation with a nested boolean expression." is
no longer emitter. At the moment, split expressions are treated as
individual Decisions.
---
clang/lib/CodeGen/CodeGenPGO.cpp | 150 ++
.../test/CoverageMapping/mcdc-nested-expr.cpp | 30 +++-
.../Frontend/custom-diag-werror-interaction.c | 4 +-
3 files changed, 109 insertions(+), 75 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 7d16f673ada41..0c3973aa4dccf 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -228,10 +228,17 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
/// The stacks are also used to find error cases and notify the user. A
/// standard logical operator nest for a boolean expression could be in a
form
/// similar to this: "x = a && b && c && (d || f)"
- unsigned NumCond = 0;
- bool SplitNestedLogicalOp = false;
- SmallVector NonLogOpStack;
- SmallVector LogOpStack;
+ struct DecisionState {
+llvm::DenseSet Leaves; // Not BinOp
+const Expr *DecisionExpr;// Root
+bool Split;
+
+DecisionState() = delete;
+DecisionState(const Expr *E, bool Split = false)
+: DecisionExpr(E), Split(Split) {}
+ };
+
+ SmallVector DecisionStack;
// Hook: dataTraverseStmtPre() is invoked prior to visiting an AST Stmt node.
bool dataTraverseStmtPre(Stmt *S) {
@@ -239,34 +246,28 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
if (MCDCMaxCond == 0)
return true;
-/// At the top of the logical operator nest, reset the number of
conditions,
-/// also forget previously seen split nesting cases.
-if (LogOpStack.empty()) {
- NumCond = 0;
- SplitNestedLogicalOp = false;
-}
-
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-/// Check for "split-nested" logical operators. This happens when a new
-/// boolean expression logical-op nest is encountered within an
existing
-/// boolean expression, separated by a non-logical operator. For
-/// example, in "x = (a && b && c && foo(d && f))", the "d && f" case
-/// starts a new boolean expression that is separated from the other
-/// conditions by the operator foo(). Split-nested cases are not
-/// supported by MC/DC.
-SplitNestedLogicalOp = SplitNestedLogicalOp || !NonLogOpStack.empty();
-
-LogOpStack.push_back(BinOp);
+/// Mark "in splitting" when a leaf is met.
+if (!DecisionStack.empty()) {
+ auto &StackTop = DecisionStack.back();
+ if (!StackTop.Split) {
+if (StackTop.Leaves.contains(S)) {
+ assert(!StackTop.Split);
+ StackTop.Split = true;
+}
return true;
}
+
+ // Split
+ assert(StackTop.Split);
+ assert(!StackTop.Leaves.contains(S));
}
-/// Keep track of non-logical operators. These are OK as long as we don't
-/// encounter a new logical operator after seeing one.
-if (!LogOpStack.empty())
- NonLogOpStack.push_back(S);
+if (const auto *E = dyn_cast(S)) {
+ if (const auto *BinOp =
+ dyn_cast(CodeGenFunction::stripCond(E));
+ BinOp && BinOp->isLogicalOp())
+DecisionStack.emplace_back(E);
+}
return true;
}
@@ -275,49 +276,57 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
// an AST Stmt node. MC/DC will use it to to signal when the top of a
// logical operation (boolean expression) nest is encountered.
bool dataTraverseStmtPost(Stmt *S) {
-/// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
-if (MCDCMaxCond == 0)
+if (DecisionStack.empty())
return true;
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-assert(LogOpStack.back() == BinOp);
-LogOpStack.pop_back();
-
-/// At the top of logical operator nest:
-if (LogOpStack.empty()) {
- /// Was the "split-nested" logical operator case encountered?
- if (SplitNestedLogicalOp) {
-unsigned DiagID = Diag.getCustomDiagID(
-DiagnosticsEngine::Warning,
-"unsupported MC/DC boolean expression; "
-"contains an operation with a nested boolean expression. "
-"Expression will not be covered");
-Diag.Report(S->getBeginLoc(), DiagID);
-return true;
- }
-
- /// Was the maximum number of conditions encoun
[llvm-branch-commits] [clang] [MC/DC] Enable nested expressions (PR #125413)
@@ -228,45 +228,46 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
/// The stacks are also used to find error cases and notify the user. A
/// standard logical operator nest for a boolean expression could be in a
form
/// similar to this: "x = a && b && c && (d || f)"
- unsigned NumCond = 0;
- bool SplitNestedLogicalOp = false;
- SmallVector NonLogOpStack;
- SmallVector LogOpStack;
+ struct DecisionState {
+llvm::DenseSet Leaves; // Not BinOp
+const Expr *DecisionExpr;// Root
+bool Split;
chapuni wrote:
Done.
https://github.com/llvm/llvm-project/pull/125413
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [MC/DC] Enable nested expressions (PR #125413)
@@ -275,49 +276,57 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
// an AST Stmt node. MC/DC will use it to to signal when the top of a
// logical operation (boolean expression) nest is encountered.
bool dataTraverseStmtPost(Stmt *S) {
-/// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
-if (MCDCMaxCond == 0)
+if (DecisionStack.empty())
return true;
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-assert(LogOpStack.back() == BinOp);
-LogOpStack.pop_back();
-
-/// At the top of logical operator nest:
-if (LogOpStack.empty()) {
- /// Was the "split-nested" logical operator case encountered?
- if (SplitNestedLogicalOp) {
-unsigned DiagID = Diag.getCustomDiagID(
-DiagnosticsEngine::Warning,
-"unsupported MC/DC boolean expression; "
-"contains an operation with a nested boolean expression. "
-"Expression will not be covered");
-Diag.Report(S->getBeginLoc(), DiagID);
-return true;
- }
-
- /// Was the maximum number of conditions encountered?
- if (NumCond > MCDCMaxCond) {
-unsigned DiagID = Diag.getCustomDiagID(
-DiagnosticsEngine::Warning,
-"unsupported MC/DC boolean expression; "
-"number of conditions (%0) exceeds max (%1). "
-"Expression will not be covered");
-Diag.Report(S->getBeginLoc(), DiagID) << NumCond << MCDCMaxCond;
-return true;
- }
-
- // Otherwise, allocate the Decision.
- MCDCState.DecisionByStmt[BinOp].ID = MCDCState.DecisionByStmt.size();
-}
-return true;
+/// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
+assert(MCDCMaxCond > 0);
+
+auto &StackTop = DecisionStack.back();
+
+if (StackTop.DecisionExpr != S) {
+ if (StackTop.Leaves.contains(S)) {
+assert(StackTop.Split);
+StackTop.Split = false;
}
+
+ return true;
+}
+
+/// Allocate the entry (with Valid=false)
+auto &DecisionEntry =
+MCDCState
+.DecisionByStmt[CodeGenFunction::stripCond(StackTop.DecisionExpr)];
+
+/// Was the "split-nested" logical operator case encountered?
+if (false && DecisionStack.size() > 1) {
evodius96 wrote:
If I understand correctly --- I don't think I see a purpose in warning about
these now that the code effectively handles them. I would deprecate or just
remove it.
https://github.com/llvm/llvm-project/pull/125413
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [MC/DC] Enable nested expressions (PR #125413)
@@ -275,49 +276,57 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
// an AST Stmt node. MC/DC will use it to to signal when the top of a
// logical operation (boolean expression) nest is encountered.
bool dataTraverseStmtPost(Stmt *S) {
-/// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
-if (MCDCMaxCond == 0)
+if (DecisionStack.empty())
return true;
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-assert(LogOpStack.back() == BinOp);
-LogOpStack.pop_back();
-
-/// At the top of logical operator nest:
-if (LogOpStack.empty()) {
- /// Was the "split-nested" logical operator case encountered?
- if (SplitNestedLogicalOp) {
-unsigned DiagID = Diag.getCustomDiagID(
-DiagnosticsEngine::Warning,
-"unsupported MC/DC boolean expression; "
-"contains an operation with a nested boolean expression. "
-"Expression will not be covered");
-Diag.Report(S->getBeginLoc(), DiagID);
-return true;
- }
-
- /// Was the maximum number of conditions encountered?
- if (NumCond > MCDCMaxCond) {
-unsigned DiagID = Diag.getCustomDiagID(
-DiagnosticsEngine::Warning,
-"unsupported MC/DC boolean expression; "
-"number of conditions (%0) exceeds max (%1). "
-"Expression will not be covered");
-Diag.Report(S->getBeginLoc(), DiagID) << NumCond << MCDCMaxCond;
-return true;
- }
-
- // Otherwise, allocate the Decision.
- MCDCState.DecisionByStmt[BinOp].ID = MCDCState.DecisionByStmt.size();
-}
-return true;
+/// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
+assert(MCDCMaxCond > 0);
+
+auto &StackTop = DecisionStack.back();
+
+if (StackTop.DecisionExpr != S) {
+ if (StackTop.Leaves.contains(S)) {
+assert(StackTop.Split);
+StackTop.Split = false;
}
+
+ return true;
+}
+
+/// Allocate the entry (with Valid=false)
+auto &DecisionEntry =
+MCDCState
+.DecisionByStmt[CodeGenFunction::stripCond(StackTop.DecisionExpr)];
+
+/// Was the "split-nested" logical operator case encountered?
+if (false && DecisionStack.size() > 1) {
chapuni wrote:
This is the option for deprecating this warning, "warn if nested level >= n" or
deprecate. @evodius96 Opinions?
https://github.com/llvm/llvm-project/pull/125413
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [MC/DC] Enable nested expressions (PR #125413)
@@ -275,49 +276,57 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
// an AST Stmt node. MC/DC will use it to to signal when the top of a
// logical operation (boolean expression) nest is encountered.
bool dataTraverseStmtPost(Stmt *S) {
-/// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
-if (MCDCMaxCond == 0)
+if (DecisionStack.empty())
return true;
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-assert(LogOpStack.back() == BinOp);
-LogOpStack.pop_back();
-
-/// At the top of logical operator nest:
-if (LogOpStack.empty()) {
- /// Was the "split-nested" logical operator case encountered?
- if (SplitNestedLogicalOp) {
-unsigned DiagID = Diag.getCustomDiagID(
-DiagnosticsEngine::Warning,
-"unsupported MC/DC boolean expression; "
-"contains an operation with a nested boolean expression. "
-"Expression will not be covered");
-Diag.Report(S->getBeginLoc(), DiagID);
-return true;
- }
-
- /// Was the maximum number of conditions encountered?
- if (NumCond > MCDCMaxCond) {
-unsigned DiagID = Diag.getCustomDiagID(
-DiagnosticsEngine::Warning,
-"unsupported MC/DC boolean expression; "
-"number of conditions (%0) exceeds max (%1). "
-"Expression will not be covered");
-Diag.Report(S->getBeginLoc(), DiagID) << NumCond << MCDCMaxCond;
-return true;
- }
-
- // Otherwise, allocate the Decision.
- MCDCState.DecisionByStmt[BinOp].ID = MCDCState.DecisionByStmt.size();
-}
-return true;
+/// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
+assert(MCDCMaxCond > 0);
+
+auto &StackTop = DecisionStack.back();
+
+if (StackTop.DecisionExpr != S) {
+ if (StackTop.Leaves.contains(S)) {
+assert(StackTop.Split);
+StackTop.Split = false;
}
+
+ return true;
+}
+
+/// Allocate the entry (with Valid=false)
+auto &DecisionEntry =
+MCDCState
+.DecisionByStmt[CodeGenFunction::stripCond(StackTop.DecisionExpr)];
+
+/// Was the "split-nested" logical operator case encountered?
+if (false && DecisionStack.size() > 1) {
ornata wrote:
if (false)?
https://github.com/llvm/llvm-project/pull/125413
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [MC/DC] Enable nested expressions (PR #125413)
@@ -228,45 +228,46 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
/// The stacks are also used to find error cases and notify the user. A
/// standard logical operator nest for a boolean expression could be in a
form
/// similar to this: "x = a && b && c && (d || f)"
- unsigned NumCond = 0;
- bool SplitNestedLogicalOp = false;
- SmallVector NonLogOpStack;
- SmallVector LogOpStack;
+ struct DecisionState {
+llvm::DenseSet Leaves; // Not BinOp
+const Expr *DecisionExpr;// Root
+bool Split;
ornata wrote:
Can you document what splitting means in the code?
https://github.com/llvm/llvm-project/pull/125413
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [MC/DC] Enable nested expressions (PR #125413)
https://github.com/chapuni updated
https://github.com/llvm/llvm-project/pull/125413
>From c56ecc30e9fd1a674073e362fbfcc6b43f2f52e2 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi
Date: Sun, 2 Feb 2025 22:06:32 +0900
Subject: [PATCH 1/2] [MC/DC] Enable nested expressions
A warning "contains an operation with a nested boolean expression." is
no longer emitter. At the moment, split expressions are treated as
individual Decisions.
---
clang/lib/CodeGen/CodeGenPGO.cpp | 150 ++
.../test/CoverageMapping/mcdc-nested-expr.cpp | 30 +++-
.../Frontend/custom-diag-werror-interaction.c | 4 +-
3 files changed, 109 insertions(+), 75 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 7d16f673ada419e..0c3973aa4dccfdc 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -228,10 +228,17 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
/// The stacks are also used to find error cases and notify the user. A
/// standard logical operator nest for a boolean expression could be in a
form
/// similar to this: "x = a && b && c && (d || f)"
- unsigned NumCond = 0;
- bool SplitNestedLogicalOp = false;
- SmallVector NonLogOpStack;
- SmallVector LogOpStack;
+ struct DecisionState {
+llvm::DenseSet Leaves; // Not BinOp
+const Expr *DecisionExpr;// Root
+bool Split;
+
+DecisionState() = delete;
+DecisionState(const Expr *E, bool Split = false)
+: DecisionExpr(E), Split(Split) {}
+ };
+
+ SmallVector DecisionStack;
// Hook: dataTraverseStmtPre() is invoked prior to visiting an AST Stmt node.
bool dataTraverseStmtPre(Stmt *S) {
@@ -239,34 +246,28 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
if (MCDCMaxCond == 0)
return true;
-/// At the top of the logical operator nest, reset the number of
conditions,
-/// also forget previously seen split nesting cases.
-if (LogOpStack.empty()) {
- NumCond = 0;
- SplitNestedLogicalOp = false;
-}
-
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-/// Check for "split-nested" logical operators. This happens when a new
-/// boolean expression logical-op nest is encountered within an
existing
-/// boolean expression, separated by a non-logical operator. For
-/// example, in "x = (a && b && c && foo(d && f))", the "d && f" case
-/// starts a new boolean expression that is separated from the other
-/// conditions by the operator foo(). Split-nested cases are not
-/// supported by MC/DC.
-SplitNestedLogicalOp = SplitNestedLogicalOp || !NonLogOpStack.empty();
-
-LogOpStack.push_back(BinOp);
+/// Mark "in splitting" when a leaf is met.
+if (!DecisionStack.empty()) {
+ auto &StackTop = DecisionStack.back();
+ if (!StackTop.Split) {
+if (StackTop.Leaves.contains(S)) {
+ assert(!StackTop.Split);
+ StackTop.Split = true;
+}
return true;
}
+
+ // Split
+ assert(StackTop.Split);
+ assert(!StackTop.Leaves.contains(S));
}
-/// Keep track of non-logical operators. These are OK as long as we don't
-/// encounter a new logical operator after seeing one.
-if (!LogOpStack.empty())
- NonLogOpStack.push_back(S);
+if (const auto *E = dyn_cast(S)) {
+ if (const auto *BinOp =
+ dyn_cast(CodeGenFunction::stripCond(E));
+ BinOp && BinOp->isLogicalOp())
+DecisionStack.emplace_back(E);
+}
return true;
}
@@ -275,49 +276,57 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
// an AST Stmt node. MC/DC will use it to to signal when the top of a
// logical operation (boolean expression) nest is encountered.
bool dataTraverseStmtPost(Stmt *S) {
-/// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
-if (MCDCMaxCond == 0)
+if (DecisionStack.empty())
return true;
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-assert(LogOpStack.back() == BinOp);
-LogOpStack.pop_back();
-
-/// At the top of logical operator nest:
-if (LogOpStack.empty()) {
- /// Was the "split-nested" logical operator case encountered?
- if (SplitNestedLogicalOp) {
-unsigned DiagID = Diag.getCustomDiagID(
-DiagnosticsEngine::Warning,
-"unsupported MC/DC boolean expression; "
-"contains an operation with a nested boolean expression. "
-"Expression will not be covered");
-Diag.Report(S->getBeginLoc(), DiagID);
-return true;
- }
-
- /// Was the maximum number of conditions en
[llvm-branch-commits] [clang] [MC/DC] Enable nested expressions (PR #125413)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: NAKAMURA Takumi (chapuni)
Changes
A warning "contains an operation with a nested boolean expression." is no
longer emitted. At the moment, split expressions are treated as individual
Decisions.
---
Full diff: https://github.com/llvm/llvm-project/pull/125413.diff
3 Files Affected:
- (modified) clang/lib/CodeGen/CodeGenPGO.cpp (+81-69)
- (modified) clang/test/CoverageMapping/mcdc-nested-expr.cpp (+26-4)
- (modified) clang/test/Frontend/custom-diag-werror-interaction.c (+2-2)
``diff
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 7d16f673ada419..0c3973aa4dccfd 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -228,10 +228,17 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
/// The stacks are also used to find error cases and notify the user. A
/// standard logical operator nest for a boolean expression could be in a
form
/// similar to this: "x = a && b && c && (d || f)"
- unsigned NumCond = 0;
- bool SplitNestedLogicalOp = false;
- SmallVector NonLogOpStack;
- SmallVector LogOpStack;
+ struct DecisionState {
+llvm::DenseSet Leaves; // Not BinOp
+const Expr *DecisionExpr;// Root
+bool Split;
+
+DecisionState() = delete;
+DecisionState(const Expr *E, bool Split = false)
+: DecisionExpr(E), Split(Split) {}
+ };
+
+ SmallVector DecisionStack;
// Hook: dataTraverseStmtPre() is invoked prior to visiting an AST Stmt node.
bool dataTraverseStmtPre(Stmt *S) {
@@ -239,34 +246,28 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
if (MCDCMaxCond == 0)
return true;
-/// At the top of the logical operator nest, reset the number of
conditions,
-/// also forget previously seen split nesting cases.
-if (LogOpStack.empty()) {
- NumCond = 0;
- SplitNestedLogicalOp = false;
-}
-
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-/// Check for "split-nested" logical operators. This happens when a new
-/// boolean expression logical-op nest is encountered within an
existing
-/// boolean expression, separated by a non-logical operator. For
-/// example, in "x = (a && b && c && foo(d && f))", the "d && f" case
-/// starts a new boolean expression that is separated from the other
-/// conditions by the operator foo(). Split-nested cases are not
-/// supported by MC/DC.
-SplitNestedLogicalOp = SplitNestedLogicalOp || !NonLogOpStack.empty();
-
-LogOpStack.push_back(BinOp);
+/// Mark "in splitting" when a leaf is met.
+if (!DecisionStack.empty()) {
+ auto &StackTop = DecisionStack.back();
+ if (!StackTop.Split) {
+if (StackTop.Leaves.contains(S)) {
+ assert(!StackTop.Split);
+ StackTop.Split = true;
+}
return true;
}
+
+ // Split
+ assert(StackTop.Split);
+ assert(!StackTop.Leaves.contains(S));
}
-/// Keep track of non-logical operators. These are OK as long as we don't
-/// encounter a new logical operator after seeing one.
-if (!LogOpStack.empty())
- NonLogOpStack.push_back(S);
+if (const auto *E = dyn_cast(S)) {
+ if (const auto *BinOp =
+ dyn_cast(CodeGenFunction::stripCond(E));
+ BinOp && BinOp->isLogicalOp())
+DecisionStack.emplace_back(E);
+}
return true;
}
@@ -275,49 +276,57 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
// an AST Stmt node. MC/DC will use it to to signal when the top of a
// logical operation (boolean expression) nest is encountered.
bool dataTraverseStmtPost(Stmt *S) {
-/// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
-if (MCDCMaxCond == 0)
+if (DecisionStack.empty())
return true;
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-assert(LogOpStack.back() == BinOp);
-LogOpStack.pop_back();
-
-/// At the top of logical operator nest:
-if (LogOpStack.empty()) {
- /// Was the "split-nested" logical operator case encountered?
- if (SplitNestedLogicalOp) {
-unsigned DiagID = Diag.getCustomDiagID(
-DiagnosticsEngine::Warning,
-"unsupported MC/DC boolean expression; "
-"contains an operation with a nested boolean expression. "
-"Expression will not be covered");
-Diag.Report(S->getBeginLoc(), DiagID);
-return true;
- }
-
- /// Was the maximum number of conditions encountered?
- if (NumCond > MCDCMaxCond) {
-unsigned DiagID = Diag.getCustomDiagID(
-
[llvm-branch-commits] [clang] [MC/DC] Enable nested expressions (PR #125413)
https://github.com/chapuni created
https://github.com/llvm/llvm-project/pull/125413
A warning "contains an operation with a nested boolean expression." is no
longer emitted. At the moment, split expressions are treated as individual
Decisions.
>From c56ecc30e9fd1a674073e362fbfcc6b43f2f52e2 Mon Sep 17 00:00:00 2001
From: NAKAMURA Takumi
Date: Sun, 2 Feb 2025 22:06:32 +0900
Subject: [PATCH] [MC/DC] Enable nested expressions
A warning "contains an operation with a nested boolean expression." is
no longer emitter. At the moment, split expressions are treated as
individual Decisions.
---
clang/lib/CodeGen/CodeGenPGO.cpp | 150 ++
.../test/CoverageMapping/mcdc-nested-expr.cpp | 30 +++-
.../Frontend/custom-diag-werror-interaction.c | 4 +-
3 files changed, 109 insertions(+), 75 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp
index 7d16f673ada419e..0c3973aa4dccfdc 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -228,10 +228,17 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
/// The stacks are also used to find error cases and notify the user. A
/// standard logical operator nest for a boolean expression could be in a
form
/// similar to this: "x = a && b && c && (d || f)"
- unsigned NumCond = 0;
- bool SplitNestedLogicalOp = false;
- SmallVector NonLogOpStack;
- SmallVector LogOpStack;
+ struct DecisionState {
+llvm::DenseSet Leaves; // Not BinOp
+const Expr *DecisionExpr;// Root
+bool Split;
+
+DecisionState() = delete;
+DecisionState(const Expr *E, bool Split = false)
+: DecisionExpr(E), Split(Split) {}
+ };
+
+ SmallVector DecisionStack;
// Hook: dataTraverseStmtPre() is invoked prior to visiting an AST Stmt node.
bool dataTraverseStmtPre(Stmt *S) {
@@ -239,34 +246,28 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
if (MCDCMaxCond == 0)
return true;
-/// At the top of the logical operator nest, reset the number of
conditions,
-/// also forget previously seen split nesting cases.
-if (LogOpStack.empty()) {
- NumCond = 0;
- SplitNestedLogicalOp = false;
-}
-
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-/// Check for "split-nested" logical operators. This happens when a new
-/// boolean expression logical-op nest is encountered within an
existing
-/// boolean expression, separated by a non-logical operator. For
-/// example, in "x = (a && b && c && foo(d && f))", the "d && f" case
-/// starts a new boolean expression that is separated from the other
-/// conditions by the operator foo(). Split-nested cases are not
-/// supported by MC/DC.
-SplitNestedLogicalOp = SplitNestedLogicalOp || !NonLogOpStack.empty();
-
-LogOpStack.push_back(BinOp);
+/// Mark "in splitting" when a leaf is met.
+if (!DecisionStack.empty()) {
+ auto &StackTop = DecisionStack.back();
+ if (!StackTop.Split) {
+if (StackTop.Leaves.contains(S)) {
+ assert(!StackTop.Split);
+ StackTop.Split = true;
+}
return true;
}
+
+ // Split
+ assert(StackTop.Split);
+ assert(!StackTop.Leaves.contains(S));
}
-/// Keep track of non-logical operators. These are OK as long as we don't
-/// encounter a new logical operator after seeing one.
-if (!LogOpStack.empty())
- NonLogOpStack.push_back(S);
+if (const auto *E = dyn_cast(S)) {
+ if (const auto *BinOp =
+ dyn_cast(CodeGenFunction::stripCond(E));
+ BinOp && BinOp->isLogicalOp())
+DecisionStack.emplace_back(E);
+}
return true;
}
@@ -275,49 +276,57 @@ struct MapRegionCounters : public
RecursiveASTVisitor {
// an AST Stmt node. MC/DC will use it to to signal when the top of a
// logical operation (boolean expression) nest is encountered.
bool dataTraverseStmtPost(Stmt *S) {
-/// If MC/DC is not enabled, MCDCMaxCond will be set to 0. Do nothing.
-if (MCDCMaxCond == 0)
+if (DecisionStack.empty())
return true;
-if (const Expr *E = dyn_cast(S)) {
- const BinaryOperator *BinOp =
dyn_cast(E->IgnoreParens());
- if (BinOp && BinOp->isLogicalOp()) {
-assert(LogOpStack.back() == BinOp);
-LogOpStack.pop_back();
-
-/// At the top of logical operator nest:
-if (LogOpStack.empty()) {
- /// Was the "split-nested" logical operator case encountered?
- if (SplitNestedLogicalOp) {
-unsigned DiagID = Diag.getCustomDiagID(
-DiagnosticsEngine::Warning,
-"unsupported MC/DC boolean expression; "
-"contains an operation with a nested boolean expression. "
-"Expression will not be
