[clang] [clang][dataflow] Refactor processing of terminator element (PR #84499)
https://github.com/ymand closed https://github.com/llvm/llvm-project/pull/84499 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Refactor processing of terminator element (PR #84499)
https://github.com/ymand updated https://github.com/llvm/llvm-project/pull/84499
>From 02381f2dbdcc569889ae55a2ca5d8698f74626d8 Mon Sep 17 00:00:00 2001
From: Yitzhak Mandelbaum
Date: Fri, 8 Mar 2024 15:19:14 +
Subject: [PATCH 1/2] [clang][dataflow] Refactor processing of terminator
element
This patch vastly simplifies the code handling terminators, without changing any
behavior. Additionally, the simplification unblocks our ability to address a
(simple) FIXME in the code to invoke `transferBranch`, even when builtin options
are disabled.
---
.../TypeErasedDataflowAnalysis.cpp| 126 +-
1 file changed, 35 insertions(+), 91 deletions(-)
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 721a11c2098530..be05ab5435e842 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -11,7 +11,6 @@
//
//===--===//
-#include
#include
#include
#include
@@ -33,7 +32,6 @@
#include "clang/Analysis/FlowSensitive/Value.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallBitVector.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/Error.h"
@@ -64,88 +62,27 @@ static bool isBackedgeNode(const CFGBlock ) {
namespace {
-// The return type of the visit functions in TerminatorVisitor. The first
-// element represents the terminator expression (that is the conditional
-// expression in case of a path split in the CFG). The second element
-// represents whether the condition was true or false.
-using TerminatorVisitorRetTy = std::pair;
-
-/// Extends the flow condition of an environment based on a terminator
-/// statement.
+/// Extracts the condition expression.
class TerminatorVisitor
-: public ConstStmtVisitor {
+: public ConstStmtVisitor {
public:
- TerminatorVisitor(Environment , int BlockSuccIdx)
- : Env(Env), BlockSuccIdx(BlockSuccIdx) {}
-
- TerminatorVisitorRetTy VisitIfStmt(const IfStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitWhileStmt(const WhileStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitDoStmt(const DoStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitForStmt(const ForStmt *S) {
-auto *Cond = S->getCond();
-if (Cond != nullptr)
- return extendFlowCondition(*Cond);
-return {nullptr, false};
- }
-
- TerminatorVisitorRetTy VisitCXXForRangeStmt(const CXXForRangeStmt *) {
+ TerminatorVisitor() = default;
+ const Expr *VisitIfStmt(const IfStmt *S) { return S->getCond(); }
+ const Expr *VisitWhileStmt(const WhileStmt *S) { return S->getCond(); }
+ const Expr *VisitDoStmt(const DoStmt *S) { return S->getCond(); }
+ const Expr *VisitForStmt(const ForStmt *S) { return S->getCond(); }
+ const Expr *VisitCXXForRangeStmt(const CXXForRangeStmt *) {
// Don't do anything special for CXXForRangeStmt, because the condition
// (being implicitly generated) isn't visible from the loop body.
-return {nullptr, false};
+return nullptr;
}
-
- TerminatorVisitorRetTy VisitBinaryOperator(const BinaryOperator *S) {
+ const Expr *VisitBinaryOperator(const BinaryOperator *S) {
assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
-auto *LHS = S->getLHS();
-assert(LHS != nullptr);
-return extendFlowCondition(*LHS);
+return S->getLHS();
}
-
- TerminatorVisitorRetTy
- VisitConditionalOperator(const ConditionalOperator *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
-private:
- TerminatorVisitorRetTy extendFlowCondition(const Expr ) {
-auto *Val = Env.get(Cond);
-// In transferCFGBlock(), we ensure that we always have a `Value` for the
-// terminator condition, so assert this.
-// We consciously assert ourselves instead of asserting via `cast()` so
-// that we get a more meaningful line number if the assertion fails.
-assert(Val != nullptr);
-
-bool ConditionValue = true;
-// The condition must be inverted for the successor that encompasses the
-// "else" branch, if such exists.
-if (BlockSuccIdx == 1) {
- Val = (*Val);
- ConditionValue = false;
-}
-
-Env.assume(Val->formula());
-return {, ConditionValue};
+ const Expr *VisitConditionalOperator(const ConditionalOperator *S) {
+return S->getCond();
}
-
- Environment
- int BlockSuccIdx;
};
/// Holds data structures required for running dataflow analysis.
@@ -336,26 +273,33 @@
[clang] [clang][dataflow] Refactor processing of terminator element (PR #84499)
https://github.com/martinboehme approved this pull request. Thanks -- _so_ much better! https://github.com/llvm/llvm-project/pull/84499 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Refactor processing of terminator element (PR #84499)
@@ -337,26 +274,33 @@ computeBlockInputState(const CFGBlock ,
AnalysisContext ) {
AC.BlockStates[Pred->getBlockID()];
if (!MaybePredState)
continue;
-
-if (AC.Analysis.builtinOptions()) {
- if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
-// We have a terminator: we need to mutate an environment to describe
-// when the terminator is taken. Copy now.
+const TypeErasedDataflowAnalysisState = *MaybePredState;
+
+if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
ymand wrote:
Done
https://github.com/llvm/llvm-project/pull/84499
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Refactor processing of terminator element (PR #84499)
https://github.com/ymand updated https://github.com/llvm/llvm-project/pull/84499
>From 3b20e1823753ab46e3e259d3d8c727dea91ce1d4 Mon Sep 17 00:00:00 2001
From: Yitzhak Mandelbaum
Date: Fri, 8 Mar 2024 15:19:14 +
Subject: [PATCH 1/2] [clang][dataflow] Refactor processing of terminator
element
This patch vastly simplifies the code handling terminators, without changing any
behavior. Additionally, the simplification unblocks our ability to address a
(simple) FIXME in the code to invoke `transferBranch`, even when builtin options
are disabled.
---
.../TypeErasedDataflowAnalysis.cpp| 126 +-
1 file changed, 35 insertions(+), 91 deletions(-)
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 939247c047c66e..2d745231fd0758 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -11,7 +11,6 @@
//
//===--===//
-#include
#include
#include
#include
@@ -33,7 +32,6 @@
#include "clang/Analysis/FlowSensitive/Value.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallBitVector.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/Error.h"
@@ -64,88 +62,27 @@ static bool isBackedgeNode(const CFGBlock ) {
namespace {
-// The return type of the visit functions in TerminatorVisitor. The first
-// element represents the terminator expression (that is the conditional
-// expression in case of a path split in the CFG). The second element
-// represents whether the condition was true or false.
-using TerminatorVisitorRetTy = std::pair;
-
-/// Extends the flow condition of an environment based on a terminator
-/// statement.
+/// Extracts the condition expression.
class TerminatorVisitor
-: public ConstStmtVisitor {
+: public ConstStmtVisitor {
public:
- TerminatorVisitor(Environment , int BlockSuccIdx)
- : Env(Env), BlockSuccIdx(BlockSuccIdx) {}
-
- TerminatorVisitorRetTy VisitIfStmt(const IfStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitWhileStmt(const WhileStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitDoStmt(const DoStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitForStmt(const ForStmt *S) {
-auto *Cond = S->getCond();
-if (Cond != nullptr)
- return extendFlowCondition(*Cond);
-return {nullptr, false};
- }
-
- TerminatorVisitorRetTy VisitCXXForRangeStmt(const CXXForRangeStmt *) {
+ TerminatorVisitor() = default;
+ const Expr *VisitIfStmt(const IfStmt *S) { return S->getCond(); }
+ const Expr *VisitWhileStmt(const WhileStmt *S) { return S->getCond(); }
+ const Expr *VisitDoStmt(const DoStmt *S) { return S->getCond(); }
+ const Expr *VisitForStmt(const ForStmt *S) { return S->getCond(); }
+ const Expr *VisitCXXForRangeStmt(const CXXForRangeStmt *) {
// Don't do anything special for CXXForRangeStmt, because the condition
// (being implicitly generated) isn't visible from the loop body.
-return {nullptr, false};
+return nullptr;
}
-
- TerminatorVisitorRetTy VisitBinaryOperator(const BinaryOperator *S) {
+ const Expr *VisitBinaryOperator(const BinaryOperator *S) {
assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
-auto *LHS = S->getLHS();
-assert(LHS != nullptr);
-return extendFlowCondition(*LHS);
+return S->getLHS();
}
-
- TerminatorVisitorRetTy
- VisitConditionalOperator(const ConditionalOperator *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
-private:
- TerminatorVisitorRetTy extendFlowCondition(const Expr ) {
-auto *Val = Env.get(Cond);
-// In transferCFGBlock(), we ensure that we always have a `Value` for the
-// terminator condition, so assert this.
-// We consciously assert ourselves instead of asserting via `cast()` so
-// that we get a more meaningful line number if the assertion fails.
-assert(Val != nullptr);
-
-bool ConditionValue = true;
-// The condition must be inverted for the successor that encompasses the
-// "else" branch, if such exists.
-if (BlockSuccIdx == 1) {
- Val = (*Val);
- ConditionValue = false;
-}
-
-Env.assume(Val->formula());
-return {, ConditionValue};
+ const Expr *VisitConditionalOperator(const ConditionalOperator *S) {
+return S->getCond();
}
-
- Environment
- int BlockSuccIdx;
};
/// Holds data structures required for running dataflow analysis.
@@ -337,26 +274,33 @@
[clang] [clang][dataflow] Refactor processing of terminator element (PR #84499)
@@ -337,26 +274,33 @@ computeBlockInputState(const CFGBlock ,
AnalysisContext ) {
AC.BlockStates[Pred->getBlockID()];
if (!MaybePredState)
continue;
-
-if (AC.Analysis.builtinOptions()) {
- if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
-// We have a terminator: we need to mutate an environment to describe
-// when the terminator is taken. Copy now.
+const TypeErasedDataflowAnalysisState = *MaybePredState;
+
+if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
martinboehme wrote:
To make this clearer, here's how the whole code block could be changed to make
it (IMO) simpler (I can't do this as a suggested edit because I would have to
include deleted lines in the block of code, and github doesn't allow me to do
this):
```cxx
const Expr *TerminatorCond =
getTerminatorCondition(Pred->getTerminatorStmt());
if (TerminatorCond == nullptr) {
Builder.addUnowned(PredState);
continue;
}
bool BranchVal = blockIndexInPredecessor(*Pred, Block) == 0;
// `transferBranch` may need to mutate the environment to describe the
// dynamic effect of the terminator for a given branch. Copy now.
TypeErasedDataflowAnalysisState Copy = PredState.fork();
if (AC.Analysis.builtinOptions()) {
auto *CondVal = Copy.Env.get(*TerminatorCond);
// In transferCFGBlock(), we ensure that we always have a `Value`
// for the terminator condition, so assert this. We consciously
// assert ourselves instead of asserting via `cast()` so that we get
// a more meaningful line number if the assertion fails.
assert(CondVal != nullptr);
BoolValue *AssertedVal =
BranchVal ? CondVal : (*CondVal);
Copy.Env.assume(AssertedVal->formula());
}
AC.Analysis.transferBranchTypeErased(BranchVal, TerminatorCond,
Copy.Lattice,
Copy.Env);
Builder.addOwned(std::move(Copy));
```
https://github.com/llvm/llvm-project/pull/84499
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Refactor processing of terminator element (PR #84499)
@@ -337,26 +274,33 @@ computeBlockInputState(const CFGBlock ,
AnalysisContext ) {
AC.BlockStates[Pred->getBlockID()];
if (!MaybePredState)
continue;
-
-if (AC.Analysis.builtinOptions()) {
- if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
-// We have a terminator: we need to mutate an environment to describe
-// when the terminator is taken. Copy now.
+const TypeErasedDataflowAnalysisState = *MaybePredState;
+
+if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
martinboehme wrote:
I'd suggest doing the shorter case first, so this longer case can be indented
less:
```cxx
const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt();
if (PredTerminatorStmt == nullptr) {
Builder.addUnowned(PredState);
continue;
}
```
Even better, introduce a short function `getTerminatorCondition()` that can
handle a null argument:
```cxx
const Expr *getTerminatorCondition(const Stmt *TerminatorStmt) {
if (TerminatorStmt == nullptr) return nullptr;
return TerminatorVisitor().Visit(TerminatorStmt);
}
```
Then you can do this:
```cxx
const Expr *TerminatorCond =
getTerminatorCondition(Pred->getTerminatorStmt());
if (TerminatorCond == nullptr) {
Builder.addUnowned(PredState);
continue;
}
```
https://github.com/llvm/llvm-project/pull/84499
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Refactor processing of terminator element (PR #84499)
@@ -64,88 +62,27 @@ static bool isBackedgeNode(const CFGBlock ) {
namespace {
-// The return type of the visit functions in TerminatorVisitor. The first
-// element represents the terminator expression (that is the conditional
-// expression in case of a path split in the CFG). The second element
-// represents whether the condition was true or false.
-using TerminatorVisitorRetTy = std::pair;
-
-/// Extends the flow condition of an environment based on a terminator
-/// statement.
+/// Extracts the condition expression.
martinboehme wrote:
```suggestion
/// Extracts the terminator condition expression.
```
https://github.com/llvm/llvm-project/pull/84499
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Refactor processing of terminator element (PR #84499)
@@ -337,26 +274,33 @@ computeBlockInputState(const CFGBlock ,
AnalysisContext ) {
AC.BlockStates[Pred->getBlockID()];
if (!MaybePredState)
continue;
-
-if (AC.Analysis.builtinOptions()) {
- if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
-// We have a terminator: we need to mutate an environment to describe
-// when the terminator is taken. Copy now.
+const TypeErasedDataflowAnalysisState = *MaybePredState;
+
+if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
+ bool BranchVal = blockIndexInPredecessor(*Pred, Block) == 0;
martinboehme wrote:
Move this down to the point where it's actually needed (this is included in the
suggestion above).
https://github.com/llvm/llvm-project/pull/84499
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Refactor processing of terminator element (PR #84499)
https://github.com/martinboehme edited https://github.com/llvm/llvm-project/pull/84499 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Refactor processing of terminator element (PR #84499)
https://github.com/martinboehme requested changes to this pull request. https://github.com/llvm/llvm-project/pull/84499 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Refactor processing of terminator element (PR #84499)
https://github.com/Xazax-hun approved this pull request. https://github.com/llvm/llvm-project/pull/84499 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Refactor processing of terminator element (PR #84499)
https://github.com/ymand updated https://github.com/llvm/llvm-project/pull/84499
>From 3b20e1823753ab46e3e259d3d8c727dea91ce1d4 Mon Sep 17 00:00:00 2001
From: Yitzhak Mandelbaum
Date: Fri, 8 Mar 2024 15:19:14 +
Subject: [PATCH] [clang][dataflow] Refactor processing of terminator element
This patch vastly simplifies the code handling terminators, without changing any
behavior. Additionally, the simplification unblocks our ability to address a
(simple) FIXME in the code to invoke `transferBranch`, even when builtin options
are disabled.
---
.../TypeErasedDataflowAnalysis.cpp| 126 +-
1 file changed, 35 insertions(+), 91 deletions(-)
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 939247c047c66e..2d745231fd0758 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -11,7 +11,6 @@
//
//===--===//
-#include
#include
#include
#include
@@ -33,7 +32,6 @@
#include "clang/Analysis/FlowSensitive/Value.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallBitVector.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/Error.h"
@@ -64,88 +62,27 @@ static bool isBackedgeNode(const CFGBlock ) {
namespace {
-// The return type of the visit functions in TerminatorVisitor. The first
-// element represents the terminator expression (that is the conditional
-// expression in case of a path split in the CFG). The second element
-// represents whether the condition was true or false.
-using TerminatorVisitorRetTy = std::pair;
-
-/// Extends the flow condition of an environment based on a terminator
-/// statement.
+/// Extracts the condition expression.
class TerminatorVisitor
-: public ConstStmtVisitor {
+: public ConstStmtVisitor {
public:
- TerminatorVisitor(Environment , int BlockSuccIdx)
- : Env(Env), BlockSuccIdx(BlockSuccIdx) {}
-
- TerminatorVisitorRetTy VisitIfStmt(const IfStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitWhileStmt(const WhileStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitDoStmt(const DoStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitForStmt(const ForStmt *S) {
-auto *Cond = S->getCond();
-if (Cond != nullptr)
- return extendFlowCondition(*Cond);
-return {nullptr, false};
- }
-
- TerminatorVisitorRetTy VisitCXXForRangeStmt(const CXXForRangeStmt *) {
+ TerminatorVisitor() = default;
+ const Expr *VisitIfStmt(const IfStmt *S) { return S->getCond(); }
+ const Expr *VisitWhileStmt(const WhileStmt *S) { return S->getCond(); }
+ const Expr *VisitDoStmt(const DoStmt *S) { return S->getCond(); }
+ const Expr *VisitForStmt(const ForStmt *S) { return S->getCond(); }
+ const Expr *VisitCXXForRangeStmt(const CXXForRangeStmt *) {
// Don't do anything special for CXXForRangeStmt, because the condition
// (being implicitly generated) isn't visible from the loop body.
-return {nullptr, false};
+return nullptr;
}
-
- TerminatorVisitorRetTy VisitBinaryOperator(const BinaryOperator *S) {
+ const Expr *VisitBinaryOperator(const BinaryOperator *S) {
assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
-auto *LHS = S->getLHS();
-assert(LHS != nullptr);
-return extendFlowCondition(*LHS);
+return S->getLHS();
}
-
- TerminatorVisitorRetTy
- VisitConditionalOperator(const ConditionalOperator *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
-private:
- TerminatorVisitorRetTy extendFlowCondition(const Expr ) {
-auto *Val = Env.get(Cond);
-// In transferCFGBlock(), we ensure that we always have a `Value` for the
-// terminator condition, so assert this.
-// We consciously assert ourselves instead of asserting via `cast()` so
-// that we get a more meaningful line number if the assertion fails.
-assert(Val != nullptr);
-
-bool ConditionValue = true;
-// The condition must be inverted for the successor that encompasses the
-// "else" branch, if such exists.
-if (BlockSuccIdx == 1) {
- Val = (*Val);
- ConditionValue = false;
-}
-
-Env.assume(Val->formula());
-return {, ConditionValue};
+ const Expr *VisitConditionalOperator(const ConditionalOperator *S) {
+return S->getCond();
}
-
- Environment
- int BlockSuccIdx;
};
/// Holds data structures required for running dataflow analysis.
@@ -337,26 +274,33 @@
[clang] [clang][dataflow] Refactor processing of terminator element (PR #84499)
https://github.com/ymand updated https://github.com/llvm/llvm-project/pull/84499
>From d1bedf3a9b1d0d4c2fbfc10509797b39eef3d592 Mon Sep 17 00:00:00 2001
From: Yitzhak Mandelbaum
Date: Fri, 8 Mar 2024 15:19:14 +
Subject: [PATCH] [clang][dataflow] Refactor processing of terminator element
This patch vastly simplifies the code handling terminators, without changing any
behavior. Additionally, the simplification unblocks our ability to address a
(simple) FIXME in the code to invoke `transferBranch`, even when builtin options
are disabled.
---
.../TypeErasedDataflowAnalysis.cpp| 128 +-
1 file changed, 36 insertions(+), 92 deletions(-)
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 939247c047c66e..31682f93803003 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -11,7 +11,6 @@
//
//===--===//
-#include
#include
#include
#include
@@ -33,7 +32,6 @@
#include "clang/Analysis/FlowSensitive/Value.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallBitVector.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/Error.h"
@@ -64,88 +62,27 @@ static bool isBackedgeNode(const CFGBlock ) {
namespace {
-// The return type of the visit functions in TerminatorVisitor. The first
-// element represents the terminator expression (that is the conditional
-// expression in case of a path split in the CFG). The second element
-// represents whether the condition was true or false.
-using TerminatorVisitorRetTy = std::pair;
-
-/// Extends the flow condition of an environment based on a terminator
-/// statement.
+/// Extracts the condition expression.
class TerminatorVisitor
-: public ConstStmtVisitor {
+: public ConstStmtVisitor {
public:
- TerminatorVisitor(Environment , int BlockSuccIdx)
- : Env(Env), BlockSuccIdx(BlockSuccIdx) {}
-
- TerminatorVisitorRetTy VisitIfStmt(const IfStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitWhileStmt(const WhileStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitDoStmt(const DoStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitForStmt(const ForStmt *S) {
-auto *Cond = S->getCond();
-if (Cond != nullptr)
- return extendFlowCondition(*Cond);
-return {nullptr, false};
- }
-
- TerminatorVisitorRetTy VisitCXXForRangeStmt(const CXXForRangeStmt *) {
+ TerminatorVisitor() = default;
+ const Expr *VisitIfStmt(const IfStmt *S) { return S->getCond(); }
+ const Expr *VisitWhileStmt(const WhileStmt *S) { return S->getCond(); }
+ const Expr *VisitDoStmt(const DoStmt *S) { return S->getCond(); }
+ const Expr *VisitForStmt(const ForStmt *S) { return S->getCond(); }
+ const Expr *VisitCXXForRangeStmt(const CXXForRangeStmt *) {
// Don't do anything special for CXXForRangeStmt, because the condition
// (being implicitly generated) isn't visible from the loop body.
-return {nullptr, false};
+return nullptr;
}
-
- TerminatorVisitorRetTy VisitBinaryOperator(const BinaryOperator *S) {
+ const Expr *VisitBinaryOperator(const BinaryOperator *S) {
assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
-auto *LHS = S->getLHS();
-assert(LHS != nullptr);
-return extendFlowCondition(*LHS);
- }
-
- TerminatorVisitorRetTy
- VisitConditionalOperator(const ConditionalOperator *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
+return S->getLHS();
}
-
-private:
- TerminatorVisitorRetTy extendFlowCondition(const Expr ) {
-auto *Val = Env.get(Cond);
-// In transferCFGBlock(), we ensure that we always have a `Value` for the
-// terminator condition, so assert this.
-// We consciously assert ourselves instead of asserting via `cast()` so
-// that we get a more meaningful line number if the assertion fails.
-assert(Val != nullptr);
-
-bool ConditionValue = true;
-// The condition must be inverted for the successor that encompasses the
-// "else" branch, if such exists.
-if (BlockSuccIdx == 1) {
- Val = (*Val);
- ConditionValue = false;
-}
-
-Env.assume(Val->formula());
-return {, ConditionValue};
+ const Expr *VisitConditionalOperator(const ConditionalOperator *S) {
+return S->getCond();
}
-
- Environment
- int BlockSuccIdx;
};
/// Holds data structures required for running dataflow analysis.
@@ -337,26 +274,33 @@
[clang] [clang][dataflow] Refactor processing of terminator element (PR #84499)
https://github.com/ymand updated https://github.com/llvm/llvm-project/pull/84499
>From b7887543320f8727545a7ce8e6d9e02b77604472 Mon Sep 17 00:00:00 2001
From: Yitzhak Mandelbaum
Date: Fri, 8 Mar 2024 15:19:14 +
Subject: [PATCH] [clang][dataflow] Refactor processing of terminator element
This patch vastly simplifies the code handling terminators, without changing any
behavior. Additionally, the simplification unblocks our ability to address a
(simple) FIXME in the code to invoke `transferBranch`, even when builtin options
are disabled.
---
.../TypeErasedDataflowAnalysis.cpp| 128 +-
1 file changed, 36 insertions(+), 92 deletions(-)
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 939247c047c66e..e953271d54d30a 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -11,7 +11,6 @@
//
//===--===//
-#include
#include
#include
#include
@@ -33,7 +32,6 @@
#include "clang/Analysis/FlowSensitive/Value.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallBitVector.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/Error.h"
@@ -64,88 +62,27 @@ static bool isBackedgeNode(const CFGBlock ) {
namespace {
-// The return type of the visit functions in TerminatorVisitor. The first
-// element represents the terminator expression (that is the conditional
-// expression in case of a path split in the CFG). The second element
-// represents whether the condition was true or false.
-using TerminatorVisitorRetTy = std::pair;
-
-/// Extends the flow condition of an environment based on a terminator
-/// statement.
+/// Extracts the condition expression.
class TerminatorVisitor
-: public ConstStmtVisitor {
+: public ConstStmtVisitor {
public:
- TerminatorVisitor(Environment , int BlockSuccIdx)
- : Env(Env), BlockSuccIdx(BlockSuccIdx) {}
-
- TerminatorVisitorRetTy VisitIfStmt(const IfStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitWhileStmt(const WhileStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitDoStmt(const DoStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitForStmt(const ForStmt *S) {
-auto *Cond = S->getCond();
-if (Cond != nullptr)
- return extendFlowCondition(*Cond);
-return {nullptr, false};
- }
-
- TerminatorVisitorRetTy VisitCXXForRangeStmt(const CXXForRangeStmt *) {
+ TerminatorVisitor() = default;
+ const Expr *VisitIfStmt(const IfStmt *S) { return S->getCond(); }
+ const Expr *VisitWhileStmt(const WhileStmt *S) { return S->getCond(); }
+ const Expr *VisitDoStmt(const DoStmt *S) { return S->getCond(); }
+ const Expr *VisitForStmt(const ForStmt *S) { return S->getCond(); }
+ const Expr *VisitCXXForRangeStmt(const CXXForRangeStmt *) {
// Don't do anything special for CXXForRangeStmt, because the condition
// (being implicitly generated) isn't visible from the loop body.
-return {nullptr, false};
+return nullptr;
}
-
- TerminatorVisitorRetTy VisitBinaryOperator(const BinaryOperator *S) {
+ const Expr *VisitBinaryOperator(const BinaryOperator *S) {
assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
-auto *LHS = S->getLHS();
-assert(LHS != nullptr);
-return extendFlowCondition(*LHS);
+return S->getLHS();
}
-
- TerminatorVisitorRetTy
- VisitConditionalOperator(const ConditionalOperator *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
+ const Expr *VisitConditionalOperator(const ConditionalOperator *S) {
+return S->getCond();
}
-
-private:
- TerminatorVisitorRetTy extendFlowCondition(const Expr ) {
-auto *Val = Env.get(Cond);
-// In transferCFGBlock(), we ensure that we always have a `Value` for the
-// terminator condition, so assert this.
-// We consciously assert ourselves instead of asserting via `cast()` so
-// that we get a more meaningful line number if the assertion fails.
-assert(Val != nullptr);
-
-bool ConditionValue = true;
-// The condition must be inverted for the successor that encompasses the
-// "else" branch, if such exists.
-if (BlockSuccIdx == 1) {
- Val = (*Val);
- ConditionValue = false;
-}
-
-Env.assume(Val->formula());
-return {, ConditionValue};
- }
-
- Environment
- int BlockSuccIdx;
};
/// Holds data structures required for running dataflow analysis.
@@ -337,26 +274,33 @@
[clang] [clang][dataflow] Refactor processing of terminator element (PR #84499)
https://github.com/ymand updated https://github.com/llvm/llvm-project/pull/84499
>From 81945389b60e95019ff916d356127119183e8198 Mon Sep 17 00:00:00 2001
From: Yitzhak Mandelbaum
Date: Fri, 8 Mar 2024 15:19:14 +
Subject: [PATCH] [clang][dataflow] Refactor processing of terminator element
This patch vastly simplifies the code handling terminators, without changing any
behavior. Additionally, the simplification unblocks our ability to address a
(simple) FIXME in the code to invoke `transferBranch`, even when builtin options
are disabled.
---
.../TypeErasedDataflowAnalysis.cpp| 133 +-
1 file changed, 39 insertions(+), 94 deletions(-)
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 939247c047c66e..a1b73365b186b0 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -11,7 +11,6 @@
//
//===--===//
-#include
#include
#include
#include
@@ -33,7 +32,6 @@
#include "clang/Analysis/FlowSensitive/Value.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallBitVector.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/Error.h"
@@ -64,88 +62,27 @@ static bool isBackedgeNode(const CFGBlock ) {
namespace {
-// The return type of the visit functions in TerminatorVisitor. The first
-// element represents the terminator expression (that is the conditional
-// expression in case of a path split in the CFG). The second element
-// represents whether the condition was true or false.
-using TerminatorVisitorRetTy = std::pair;
-
-/// Extends the flow condition of an environment based on a terminator
-/// statement.
+/// Extracts the condition expression.
class TerminatorVisitor
-: public ConstStmtVisitor {
+: public ConstStmtVisitor {
public:
- TerminatorVisitor(Environment , int BlockSuccIdx)
- : Env(Env), BlockSuccIdx(BlockSuccIdx) {}
-
- TerminatorVisitorRetTy VisitIfStmt(const IfStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitWhileStmt(const WhileStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitDoStmt(const DoStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitForStmt(const ForStmt *S) {
-auto *Cond = S->getCond();
-if (Cond != nullptr)
- return extendFlowCondition(*Cond);
-return {nullptr, false};
- }
-
- TerminatorVisitorRetTy VisitCXXForRangeStmt(const CXXForRangeStmt *) {
+ TerminatorVisitor() = default;
+ const Expr *VisitIfStmt(const IfStmt *S) { return S->getCond(); }
+ const Expr *VisitWhileStmt(const WhileStmt *S) { return S->getCond(); }
+ const Expr *VisitDoStmt(const DoStmt *S) { return S->getCond(); }
+ const Expr *VisitForStmt(const ForStmt *S) { return S->getCond(); }
+ const Expr *VisitCXXForRangeStmt(const CXXForRangeStmt *) {
// Don't do anything special for CXXForRangeStmt, because the condition
// (being implicitly generated) isn't visible from the loop body.
-return {nullptr, false};
+return nullptr;
}
-
- TerminatorVisitorRetTy VisitBinaryOperator(const BinaryOperator *S) {
+ const Expr *VisitBinaryOperator(const BinaryOperator *S) {
assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
-auto *LHS = S->getLHS();
-assert(LHS != nullptr);
-return extendFlowCondition(*LHS);
+return S->getLHS();
}
-
- TerminatorVisitorRetTy
- VisitConditionalOperator(const ConditionalOperator *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
+ const Expr *VisitConditionalOperator(const ConditionalOperator *S) {
+return S->getCond();
}
-
-private:
- TerminatorVisitorRetTy extendFlowCondition(const Expr ) {
-auto *Val = Env.get(Cond);
-// In transferCFGBlock(), we ensure that we always have a `Value` for the
-// terminator condition, so assert this.
-// We consciously assert ourselves instead of asserting via `cast()` so
-// that we get a more meaningful line number if the assertion fails.
-assert(Val != nullptr);
-
-bool ConditionValue = true;
-// The condition must be inverted for the successor that encompasses the
-// "else" branch, if such exists.
-if (BlockSuccIdx == 1) {
- Val = (*Val);
- ConditionValue = false;
-}
-
-Env.assume(Val->formula());
-return {, ConditionValue};
- }
-
- Environment
- int BlockSuccIdx;
};
/// Holds data structures required for running dataflow analysis.
@@ -337,26 +274,33 @@
[clang] [clang][dataflow] Refactor processing of terminator element (PR #84499)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff cb6f657a70f7a8d6ecd4fcc2101550a7400f94a7
210a2b83d3647c97ad4fc5eab9e0d9c68a5711a4 --
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
``
View the diff from clang-format here.
``diff
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 3acd6be874..c6522f1569 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -258,29 +258,29 @@ computeBlockInputState(const CFGBlock ,
AnalysisContext ) {
continue;
const TypeErasedDataflowAnalysisState = *MaybePredState;
- if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
-bool BranchVal = blockIndexInPredecessor(*Pred, Block) == 0;
-const Expr *Cond = TerminatorVisitor().Visit(PredTerminatorStmt);
-if (Cond != nullptr) {
- // `transferBranch` may need to mutate the environment to describe
the
- // dynamic effect of the terminator for a given branch. Copy now.
- TypeErasedDataflowAnalysisState Copy = MaybePredState->fork();
- if (AC.Analysis.builtinOptions()) {
-auto *CondVal = Copy.Env.get(*Cond);
-// In transferCFGBlock(), we ensure that we always have a `Value`
-// for the terminator condition, so assert this. We consciously
-// assert ourselves instead of asserting via `cast()` so that we
get
-// a more meaningful line number if the assertion fails.
-assert(CondVal != nullptr);
-BoolValue *AssertedVal =
-BranchVal ? CondVal : (*CondVal);
-Copy.Env.assume(AssertedVal->formula());
- }
- AC.Analysis.transferBranchTypeErased(BranchVal, Cond, Copy.Lattice,
- Copy.Env);
- Builder.addOwned(std::move(Copy));
- continue;
+if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
+ bool BranchVal = blockIndexInPredecessor(*Pred, Block) == 0;
+ const Expr *Cond = TerminatorVisitor().Visit(PredTerminatorStmt);
+ if (Cond != nullptr) {
+// `transferBranch` may need to mutate the environment to describe the
+// dynamic effect of the terminator for a given branch. Copy now.
+TypeErasedDataflowAnalysisState Copy = MaybePredState->fork();
+if (AC.Analysis.builtinOptions()) {
+ auto *CondVal = Copy.Env.get(*Cond);
+ // In transferCFGBlock(), we ensure that we always have a `Value`
+ // for the terminator condition, so assert this. We consciously
+ // assert ourselves instead of asserting via `cast()` so that we get
+ // a more meaningful line number if the assertion fails.
+ assert(CondVal != nullptr);
+ BoolValue *AssertedVal =
+ BranchVal ? CondVal : (*CondVal);
+ Copy.Env.assume(AssertedVal->formula());
}
+AC.Analysis.transferBranchTypeErased(BranchVal, Cond, Copy.Lattice,
+ Copy.Env);
+Builder.addOwned(std::move(Copy));
+continue;
+ }
}
Builder.addUnowned(PredState);
}
``
https://github.com/llvm/llvm-project/pull/84499
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Refactor processing of terminator element (PR #84499)
llvmbot wrote:
@llvm/pr-subscribers-clang-analysis
Author: Yitzhak Mandelbaum (ymand)
Changes
This patch vastly simplifies the code handling terminators, without changing any
behavior. Additionally, the simplification unblocks our ability to address a
(simple) FIXME in the code to invoke `transferBranch`, even when builtin options
are disabled.
---
Full diff: https://github.com/llvm/llvm-project/pull/84499.diff
1 Files Affected:
- (modified) clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
(+46-119)
``diff
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 939247c047c66e..3acd6be874a6b1 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -11,7 +11,6 @@
//
//===--===//
-#include
#include
#include
#include
@@ -33,7 +32,6 @@
#include "clang/Analysis/FlowSensitive/Value.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallBitVector.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/Error.h"
@@ -64,88 +62,27 @@ static bool isBackedgeNode(const CFGBlock ) {
namespace {
-// The return type of the visit functions in TerminatorVisitor. The first
-// element represents the terminator expression (that is the conditional
-// expression in case of a path split in the CFG). The second element
-// represents whether the condition was true or false.
-using TerminatorVisitorRetTy = std::pair;
-
-/// Extends the flow condition of an environment based on a terminator
-/// statement.
+/// Extracts the condition expression.
class TerminatorVisitor
-: public ConstStmtVisitor {
+: public ConstStmtVisitor {
public:
- TerminatorVisitor(Environment , int BlockSuccIdx)
- : Env(Env), BlockSuccIdx(BlockSuccIdx) {}
-
- TerminatorVisitorRetTy VisitIfStmt(const IfStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitWhileStmt(const WhileStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitDoStmt(const DoStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitForStmt(const ForStmt *S) {
-auto *Cond = S->getCond();
-if (Cond != nullptr)
- return extendFlowCondition(*Cond);
-return {nullptr, false};
- }
-
- TerminatorVisitorRetTy VisitCXXForRangeStmt(const CXXForRangeStmt *) {
+ TerminatorVisitor() = default;
+ const Expr *VisitIfStmt(const IfStmt *S) { return S->getCond(); }
+ const Expr *VisitWhileStmt(const WhileStmt *S) { return S->getCond(); }
+ const Expr *VisitDoStmt(const DoStmt *S) { return S->getCond(); }
+ const Expr *VisitForStmt(const ForStmt *S) { return S->getCond(); }
+ const Expr *VisitCXXForRangeStmt(const CXXForRangeStmt *) {
// Don't do anything special for CXXForRangeStmt, because the condition
// (being implicitly generated) isn't visible from the loop body.
-return {nullptr, false};
+return nullptr;
}
-
- TerminatorVisitorRetTy VisitBinaryOperator(const BinaryOperator *S) {
+ const Expr *VisitBinaryOperator(const BinaryOperator *S) {
assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
-auto *LHS = S->getLHS();
-assert(LHS != nullptr);
-return extendFlowCondition(*LHS);
+return S->getLHS();
}
-
- TerminatorVisitorRetTy
- VisitConditionalOperator(const ConditionalOperator *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
+ const Expr *VisitConditionalOperator(const ConditionalOperator *S) {
+return S->getCond();
}
-
-private:
- TerminatorVisitorRetTy extendFlowCondition(const Expr ) {
-auto *Val = Env.get(Cond);
-// In transferCFGBlock(), we ensure that we always have a `Value` for the
-// terminator condition, so assert this.
-// We consciously assert ourselves instead of asserting via `cast()` so
-// that we get a more meaningful line number if the assertion fails.
-assert(Val != nullptr);
-
-bool ConditionValue = true;
-// The condition must be inverted for the successor that encompasses the
-// "else" branch, if such exists.
-if (BlockSuccIdx == 1) {
- Val = (*Val);
- ConditionValue = false;
-}
-
-Env.assume(Val->formula());
-return {, ConditionValue};
- }
-
- Environment
- int BlockSuccIdx;
};
/// Holds data structures required for running dataflow analysis.
@@ -221,7 +158,6 @@ class PrettyStackTraceCFGElement : public
llvm::PrettyStackTraceEntry {
// Avoids unneccesary copies of the environment.
class JoinedStateBuilder {
[clang] [clang][dataflow] Refactor processing of terminator element (PR #84499)
https://github.com/ymand created https://github.com/llvm/llvm-project/pull/84499
This patch vastly simplifies the code handling terminators, without changing any
behavior. Additionally, the simplification unblocks our ability to address a
(simple) FIXME in the code to invoke `transferBranch`, even when builtin options
are disabled.
>From 210a2b83d3647c97ad4fc5eab9e0d9c68a5711a4 Mon Sep 17 00:00:00 2001
From: Yitzhak Mandelbaum
Date: Fri, 8 Mar 2024 15:19:14 +
Subject: [PATCH] [clang][dataflow] Refactor processing of terminator element
This patch vastly simplifies the code handling terminators, without changing any
behavior. Additionally, the simplification unblocks our ability to address a
(simple) FIXME in the code to invoke `transferBranch`, even when builtin options
are disabled.
---
.../TypeErasedDataflowAnalysis.cpp| 165 +-
1 file changed, 46 insertions(+), 119 deletions(-)
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index 939247c047c66e..3acd6be874a6b1 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -11,7 +11,6 @@
//
//===--===//
-#include
#include
#include
#include
@@ -33,7 +32,6 @@
#include "clang/Analysis/FlowSensitive/Value.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallBitVector.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/Error.h"
@@ -64,88 +62,27 @@ static bool isBackedgeNode(const CFGBlock ) {
namespace {
-// The return type of the visit functions in TerminatorVisitor. The first
-// element represents the terminator expression (that is the conditional
-// expression in case of a path split in the CFG). The second element
-// represents whether the condition was true or false.
-using TerminatorVisitorRetTy = std::pair;
-
-/// Extends the flow condition of an environment based on a terminator
-/// statement.
+/// Extracts the condition expression.
class TerminatorVisitor
-: public ConstStmtVisitor {
+: public ConstStmtVisitor {
public:
- TerminatorVisitor(Environment , int BlockSuccIdx)
- : Env(Env), BlockSuccIdx(BlockSuccIdx) {}
-
- TerminatorVisitorRetTy VisitIfStmt(const IfStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitWhileStmt(const WhileStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitDoStmt(const DoStmt *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
- }
-
- TerminatorVisitorRetTy VisitForStmt(const ForStmt *S) {
-auto *Cond = S->getCond();
-if (Cond != nullptr)
- return extendFlowCondition(*Cond);
-return {nullptr, false};
- }
-
- TerminatorVisitorRetTy VisitCXXForRangeStmt(const CXXForRangeStmt *) {
+ TerminatorVisitor() = default;
+ const Expr *VisitIfStmt(const IfStmt *S) { return S->getCond(); }
+ const Expr *VisitWhileStmt(const WhileStmt *S) { return S->getCond(); }
+ const Expr *VisitDoStmt(const DoStmt *S) { return S->getCond(); }
+ const Expr *VisitForStmt(const ForStmt *S) { return S->getCond(); }
+ const Expr *VisitCXXForRangeStmt(const CXXForRangeStmt *) {
// Don't do anything special for CXXForRangeStmt, because the condition
// (being implicitly generated) isn't visible from the loop body.
-return {nullptr, false};
+return nullptr;
}
-
- TerminatorVisitorRetTy VisitBinaryOperator(const BinaryOperator *S) {
+ const Expr *VisitBinaryOperator(const BinaryOperator *S) {
assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
-auto *LHS = S->getLHS();
-assert(LHS != nullptr);
-return extendFlowCondition(*LHS);
+return S->getLHS();
}
-
- TerminatorVisitorRetTy
- VisitConditionalOperator(const ConditionalOperator *S) {
-auto *Cond = S->getCond();
-assert(Cond != nullptr);
-return extendFlowCondition(*Cond);
+ const Expr *VisitConditionalOperator(const ConditionalOperator *S) {
+return S->getCond();
}
-
-private:
- TerminatorVisitorRetTy extendFlowCondition(const Expr ) {
-auto *Val = Env.get(Cond);
-// In transferCFGBlock(), we ensure that we always have a `Value` for the
-// terminator condition, so assert this.
-// We consciously assert ourselves instead of asserting via `cast()` so
-// that we get a more meaningful line number if the assertion fails.
-assert(Val != nullptr);
-
-bool ConditionValue = true;
-// The condition must be inverted for the successor that encompasses the
-// "else" branch, if such exists.
-if (BlockSuccIdx == 1) {
- Val = (*Val);
-
