[clang] c80443c - [NFC][analyzer] Eliminate SwitchNodeBuilder (#188096)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 3 00:46:11 PDT 2026
Author: Donát Nagy
Date: 2026-04-03T09:46:06+02:00
New Revision: c80443cd37b2e2788cba67ffa180a6331e5f0791
URL: https://github.com/llvm/llvm-project/commit/c80443cd37b2e2788cba67ffa180a6331e5f0791
DIFF: https://github.com/llvm/llvm-project/commit/c80443cd37b2e2788cba67ffa180a6331e5f0791.diff
LOG: [NFC][analyzer] Eliminate SwitchNodeBuilder (#188096)
This commit removes the class `SwitchNodeBuilder` because it just
obscured the logic of switch handling by hiding some parts of it in
another source file.
Added:
Modified:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/test/Analysis/switch-basics.c
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
index 76bd8346f269e..7dfdca0d54a67 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
@@ -51,7 +51,6 @@ class CoreEngine {
friend class ExprEngine;
friend class NodeBuilder;
friend class NodeBuilderContext;
- friend class SwitchNodeBuilder;
public:
using BlocksExhausted =
@@ -332,23 +331,6 @@ class BranchNodeBuilder : public NodeBuilder {
ExplodedNode *Pred);
};
-class SwitchNodeBuilder : public NodeBuilder {
-public:
- SwitchNodeBuilder(ExplodedNodeSet &DstSet, const NodeBuilderContext &Ctx)
- : NodeBuilder(DstSet, Ctx) {}
-
- using iterator = CFGBlock::const_succ_reverse_iterator;
-
- iterator begin() { return C.getBlock()->succ_rbegin() + 1; }
- iterator end() { return C.getBlock()->succ_rend(); }
-
- ExplodedNode *generateCaseStmtNode(const CFGBlock *Block,
- ProgramStateRef State, ExplodedNode *Pred);
-
- ExplodedNode *generateDefaultCaseNode(ProgramStateRef State,
- ExplodedNode *Pred);
-};
-
} // namespace ento
} // namespace clang
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index c346c20a7af87..746354417be66 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -89,7 +89,6 @@ class ProgramState;
class ProgramStateManager;
class RegionAndSymbolInvalidationTraits;
class SymbolManager;
-class SwitchNodeBuilder;
/// Hints for figuring out of a call should be inlined during evalCall().
struct EvalCallOptions {
diff --git a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
index a348f7947ad98..17bee557fe029 100644
--- a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -707,26 +707,3 @@ ExplodedNode *BranchNodeBuilder::generateNode(ProgramStateRef State,
ExplodedNode *Succ = NodeBuilder::generateNode(Loc, State, NodePred);
return Succ;
}
-
-ExplodedNode *SwitchNodeBuilder::generateCaseStmtNode(const CFGBlock *Block,
- ProgramStateRef St,
- ExplodedNode *Pred) {
- BlockEdge BE(C.getBlock(), Block, Pred->getLocationContext());
- return generateNode(BE, St, Pred);
-}
-
-ExplodedNode *SwitchNodeBuilder::generateDefaultCaseNode(ProgramStateRef St,
- ExplodedNode *Pred) {
- // Get the block for the default case.
- const CFGBlock *Src = C.getBlock();
- assert(Src->succ_rbegin() != Src->succ_rend());
- CFGBlock *DefaultBlock = *Src->succ_rbegin();
-
- // Basic correctness check for default blocks that are unreachable and not
- // caught by earlier stages.
- if (!DefaultBlock)
- return nullptr;
-
- BlockEdge BE(Src, DefaultBlock, Pred->getLocationContext());
- return generateNode(BE, St, Pred);
-}
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index b6d2c96627520..ca544aad46e0a 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3083,9 +3083,23 @@ void ExprEngine::processEndOfFunction(ExplodedNode *Pred,
/// nodes by processing the 'effects' of a switch statement.
void ExprEngine::processSwitch(const SwitchStmt *Switch, ExplodedNode *Pred,
ExplodedNodeSet &Dst) {
+ const ASTContext &ACtx = getContext();
+ const LocationContext *LCtx = Pred->getLocationContext();
const Expr *Condition = Switch->getCond();
- SwitchNodeBuilder Builder(Dst, *currBldrCtx);
+ // The block that is terminated by the switch statement.
+ const CFGBlock *SwitchBlock = getCurrBlock();
+ // Note that successors may be null if they are pruned as unreachable.
+ assert(SwitchBlock->succ_size() && "Switch must have at least one successor");
+ // The reversed iteration order is present since the beginning, when in 2008
+ // commit 80ebc1d1c95704b0ff0386b3a3cbc8b3ff960654 added support for handling
+ // switch statements. I don't see any advantage over regular forward
+ // iteration -- but switching the order would perturb the insertion order of
+ // the work list and therefore the analysis results.
+ llvm::iterator_range<CFGBlock::const_succ_reverse_iterator> CaseBlocks(
+ SwitchBlock->succ_rbegin() + 1, SwitchBlock->succ_rend());
+ const CFGBlock *DefaultBlock = *SwitchBlock->succ_rbegin();
+
ExplodedNodeSet CheckersOutSet;
getCheckerManager().runCheckersForBranchCondition(
@@ -3094,30 +3108,29 @@ void ExprEngine::processSwitch(const SwitchStmt *Switch, ExplodedNode *Pred,
for (ExplodedNode *Node : CheckersOutSet) {
ProgramStateRef State = Node->getState();
- SVal CondV = State->getSVal(Condition, Node->getLocationContext());
+ SVal CondV = State->getSVal(Condition, LCtx);
if (CondV.isUndef()) {
// This can only happen if core.uninitialized.Branch is disabled.
continue;
}
-
std::optional<NonLoc> CondNL = CondV.getAs<NonLoc>();
- for (const CFGBlock *Block : Builder) {
+ for (const CFGBlock *CaseBlock : CaseBlocks) {
// Successor may be pruned out during CFG construction.
- if (!Block)
+ if (!CaseBlock)
continue;
- const CaseStmt *Case = cast<CaseStmt>(Block->getLabel());
+ const CaseStmt *Case = cast<CaseStmt>(CaseBlock->getLabel());
// Evaluate the LHS of the case value.
- llvm::APSInt V1 = Case->getLHS()->EvaluateKnownConstInt(getContext());
+ llvm::APSInt V1 = Case->getLHS()->EvaluateKnownConstInt(ACtx);
assert(V1.getBitWidth() ==
getContext().getIntWidth(Condition->getType()));
// Get the RHS of the case, if it exists.
llvm::APSInt V2;
if (const Expr *E = Case->getRHS())
- V2 = E->EvaluateKnownConstInt(getContext());
+ V2 = E->EvaluateKnownConstInt(ACtx);
else
V2 = V1;
@@ -3132,8 +3145,10 @@ void ExprEngine::processSwitch(const SwitchStmt *Switch, ExplodedNode *Pred,
StateMatching = State;
}
- if (StateMatching)
- Builder.generateCaseStmtNode(Block, StateMatching, Node);
+ if (StateMatching) {
+ BlockEdge BE(SwitchBlock, CaseBlock, LCtx);
+ Dst.insert(Engine.makeNode(BE, StateMatching, Node));
+ }
// If _not_ entering the current case is infeasible, then we are done
// with processing the paths through the current Node.
@@ -3143,6 +3158,10 @@ void ExprEngine::processSwitch(const SwitchStmt *Switch, ExplodedNode *Pred,
if (!State)
continue;
+ // The default block may be null if it is "optimized out" by CFG creation.
+ if (!DefaultBlock)
+ continue;
+
// If we have switch(enum value), the default branch is not
// feasible if all of the enum constants not covered by 'case:' statements
// are not feasible values for the switch condition.
@@ -3155,7 +3174,8 @@ void ExprEngine::processSwitch(const SwitchStmt *Switch, ExplodedNode *Pred,
continue;
}
- Builder.generateDefaultCaseNode(State, Node);
+ BlockEdge BE(SwitchBlock, DefaultBlock, LCtx);
+ Dst.insert(Engine.makeNode(BE, State, Node));
}
}
diff --git a/clang/test/Analysis/switch-basics.c b/clang/test/Analysis/switch-basics.c
index 2a0f9c2a3cf1b..72c728eecacc1 100644
--- a/clang/test/Analysis/switch-basics.c
+++ b/clang/test/Analysis/switch-basics.c
@@ -1,6 +1,6 @@
// RUN: %clang_analyze_cc1 -verify %s -analyzer-checker=core
-// This file tests ExprEngine::processSwitch and the class SwitchNodeBuilder.
+// This file tests ExprEngine::processSwitch.
int switch_simple(int x) {
// Validate that switch behaves as expected in a very simple situation.
@@ -93,6 +93,16 @@ int switch_no_compound_stmt(int x) {
return 0;
}
+void switch_empty(int x) {
+ // Validate that the engine does not crash on these empty switches.
+ // (These are pretty useless, but the analyzer should still handle them.)
+
+ switch (x) {}
+
+ switch (x)
+ ;
+}
+
int switch_with_case_range(int x) {
// Validate that the GNU case range extension is properly handled.
switch (x) {
More information about the cfe-commits
mailing list