[clang] [CFG] Add a BuildOption to consider default branch of switch on covered enumerations (PR #161345)
Fred Tingaud via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 2 07:41:01 PDT 2025
https://github.com/frederic-tingaud-sonarsource updated https://github.com/llvm/llvm-project/pull/161345
>From 4af057dd6b5709d7de819108a14949ad5a0445fd Mon Sep 17 00:00:00 2001
From: Fred Tingaud <frederic.tingaud at sonarsource.com>
Date: Mon, 29 Sep 2025 18:37:48 +0200
Subject: [PATCH 1/2] CFG: Add a BuildOption to consider default branch of
switch on covered enumerations.
By default, the `default:` branch of a switch on fully covered enumarations is considered as "Unreachable".
It is a sane assumption in most cases, but not always. That commit allows to change such behavior when needed.
---
clang/include/clang/Analysis/CFG.h | 1 +
clang/lib/Analysis/CFG.cpp | 5 +-
clang/unittests/Analysis/CFGTest.cpp | 152 +++++++++++++++++++++++++++
3 files changed, 157 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/Analysis/CFG.h b/clang/include/clang/Analysis/CFG.h
index 1b1ff5e558ec5..edfd655b579b9 100644
--- a/clang/include/clang/Analysis/CFG.h
+++ b/clang/include/clang/Analysis/CFG.h
@@ -1251,6 +1251,7 @@ class CFG {
bool MarkElidedCXXConstructors = false;
bool AddVirtualBaseBranches = false;
bool OmitImplicitValueInitializers = false;
+ bool SwitchKeepDefaultCoveredEnum = false;
BuildOptions() = default;
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 60a2d113c08e2..e5843f3815c5f 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -4516,9 +4516,12 @@ CFGBlock *CFGBuilder::VisitSwitchStmt(SwitchStmt *Terminator) {
//
// Note: We add a successor to a switch that is considered covered yet has no
// case statements if the enumeration has no enumerators.
+ // We also consider this successor reachable if
+ // BuildOpts.SwitchReqDefaultCoveredEnum is true.
bool SwitchAlwaysHasSuccessor = false;
SwitchAlwaysHasSuccessor |= switchExclusivelyCovered;
- SwitchAlwaysHasSuccessor |= Terminator->isAllEnumCasesCovered() &&
+ SwitchAlwaysHasSuccessor |= !BuildOpts.SwitchKeepDefaultCoveredEnum &&
+ Terminator->isAllEnumCasesCovered() &&
Terminator->getSwitchCaseList();
addSuccessor(SwitchTerminatedBlock, DefaultCaseBlock,
!SwitchAlwaysHasSuccessor);
diff --git a/clang/unittests/Analysis/CFGTest.cpp b/clang/unittests/Analysis/CFGTest.cpp
index 46a6751391cf5..cce0f6bbcfa74 100644
--- a/clang/unittests/Analysis/CFGTest.cpp
+++ b/clang/unittests/Analysis/CFGTest.cpp
@@ -93,6 +93,158 @@ TEST(CFG, DependantBaseAddImplicitDtors) {
.getStatus());
}
+TEST(CFG, SwitchCoveredEnumNoDefault) {
+ const char *Code = R"(
+ enum class E {E1, E2};
+ int f(E e) {
+ switch(e) {
+ case E::E1:
+ return 1;
+ case E::E2:
+ return 2;
+ }
+ return 0;
+ }
+ )";
+ CFG::BuildOptions Options;
+ Options.SwitchKeepDefaultCoveredEnum = true;
+ BuildResult B = BuildCFG(Code, Options);
+ EXPECT_EQ(BuildResult::BuiltCFG, B.getStatus());
+
+ // [B5 (ENTRY)]
+ // Succs (1): B2
+ //
+ // [B1]
+ // 1: 0
+ // 2: return [B1.1];
+ // Preds (1): B2
+ // Succs (1): B0
+ //
+ // [B2]
+ // 1: e (ImplicitCastExpr, LValueToRValue, E)
+ // T: switch [B2.1]
+ // Preds (1): B5
+ // Succs (3): B3 B4 B1
+ //
+ // [B3]
+ // case E::E2:
+ // 1: 2
+ // 2: return [B3.1];
+ // Preds (1): B2
+ // Succs (1): B0
+ //
+ // [B4]
+ // case E::E1:
+ // 1: 1
+ // 2: return [B4.1];
+ // Preds (1): B2
+ // Succs (1): B0
+ //
+ // [B0 (EXIT)]
+ // Preds (3): B1 B3 B4
+
+ const auto &Entry = B.getCFG()->getEntry();
+ EXPECT_EQ(1u, Entry.succ_size());
+ // First successor of Entry is the switch
+ CFGBlock *SwitchBlock = *Entry.succ_begin();
+ EXPECT_EQ(3u, SwitchBlock->succ_size());
+ // Last successor of the switch is after the switch
+ auto NoCaseSucc = SwitchBlock->succ_rbegin();
+ EXPECT_TRUE(NoCaseSucc->isReachable());
+
+ // Checking that the same node is Unreachable without this setting
+ Options.SwitchKeepDefaultCoveredEnum = false;
+ B = BuildCFG(Code, Options);
+ EXPECT_EQ(BuildResult::BuiltCFG, B.getStatus());
+
+ const auto &Entry2 = B.getCFG()->getEntry();
+ EXPECT_EQ(1u, Entry2.succ_size());
+ CFGBlock *SwitchBlock2 = *Entry2.succ_begin();
+ EXPECT_EQ(3u, SwitchBlock2->succ_size());
+ auto NoCaseSucc2 = SwitchBlock2->succ_rbegin();
+ EXPECT_FALSE(NoCaseSucc2->isReachable());
+}
+
+TEST(CFG, SwitchCoveredEnumWithDefault) {
+ const char *Code = R"(
+ enum class E {E1, E2};
+ int f(E e) {
+ switch(e) {
+ case E::E1:
+ return 1;
+ case E::E2:
+ return 2;
+ default:
+ return 0;
+ }
+ return -1;
+ }
+ )";
+ CFG::BuildOptions Options;
+ Options.SwitchKeepDefaultCoveredEnum = true;
+ BuildResult B = BuildCFG(Code, Options);
+ EXPECT_EQ(BuildResult::BuiltCFG, B.getStatus());
+
+ // [B6 (ENTRY)]
+ // Succs (1): B2
+ //
+ // [B1]
+ // 1: -1
+ // 2: return [B1.1];
+ // Succs (1): B0
+ //
+ // [B2]
+ // 1: e (ImplicitCastExpr, LValueToRValue, E)
+ // T: switch [B2.1]
+ // Preds (1): B6
+ // Succs (3): B4 B5 B3
+ //
+ // [B3]
+ // default:
+ // 1: 0
+ // 2: return [B3.1];
+ // Preds (1): B2
+ // Succs (1): B0
+ //
+ // [B4]
+ // case E::E2:
+ // 1: 2
+ // 2: return [B4.1];
+ // Preds (1): B2
+ // Succs (1): B0
+ //
+ // [B5]
+ // case E::E1:
+ // 1: 1
+ // 2: return [B5.1];
+ // Preds (1): B2
+ // Succs (1): B0
+ //
+ // [B0 (EXIT)]
+ // Preds (4): B1 B3 B4 B5
+
+ const auto &Entry = B.getCFG()->getEntry();
+ EXPECT_EQ(1u, Entry.succ_size());
+ // First successor of Entry is the switch
+ CFGBlock *SwitchBlock = *Entry.succ_begin();
+ EXPECT_EQ(3u, SwitchBlock->succ_size());
+ // Last successor of the switch is the default branch
+ auto defaultBlock = SwitchBlock->succ_rbegin();
+ EXPECT_TRUE(defaultBlock->isReachable());
+
+ // Checking that the same node is Unreachable without this setting
+ Options.SwitchKeepDefaultCoveredEnum = false;
+ B = BuildCFG(Code, Options);
+ EXPECT_EQ(BuildResult::BuiltCFG, B.getStatus());
+
+ const auto &Entry2 = B.getCFG()->getEntry();
+ EXPECT_EQ(1u, Entry2.succ_size());
+ CFGBlock *SwitchBlock2 = *Entry2.succ_begin();
+ EXPECT_EQ(3u, SwitchBlock2->succ_size());
+ auto defaultBlock2 = SwitchBlock2->succ_rbegin();
+ EXPECT_FALSE(defaultBlock2->isReachable());
+}
+
TEST(CFG, IsLinear) {
auto expectLinear = [](bool IsLinear, const char *Code) {
BuildResult B = BuildCFG(Code);
>From dd2c98b92abfde18da44c39eab5a16bff1977eb3 Mon Sep 17 00:00:00 2001
From: Fred Tingaud <frederic.tingaud at sonarsource.com>
Date: Thu, 2 Oct 2025 11:54:01 +0200
Subject: [PATCH 2/2] Apply suggestions
---
clang/include/clang/Analysis/CFG.h | 2 +-
clang/lib/Analysis/CFG.cpp | 6 +++---
clang/unittests/Analysis/CFGTest.cpp | 27 ++++++++++++++-------------
3 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/clang/include/clang/Analysis/CFG.h b/clang/include/clang/Analysis/CFG.h
index edfd655b579b9..6dd7d138e4357 100644
--- a/clang/include/clang/Analysis/CFG.h
+++ b/clang/include/clang/Analysis/CFG.h
@@ -1251,7 +1251,7 @@ class CFG {
bool MarkElidedCXXConstructors = false;
bool AddVirtualBaseBranches = false;
bool OmitImplicitValueInitializers = false;
- bool SwitchKeepDefaultCoveredEnum = false;
+ bool AssumeReachableDefaultInSwitchStatements = false;
BuildOptions() = default;
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index e5843f3815c5f..cdde849b0e026 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -4520,9 +4520,9 @@ CFGBlock *CFGBuilder::VisitSwitchStmt(SwitchStmt *Terminator) {
// BuildOpts.SwitchReqDefaultCoveredEnum is true.
bool SwitchAlwaysHasSuccessor = false;
SwitchAlwaysHasSuccessor |= switchExclusivelyCovered;
- SwitchAlwaysHasSuccessor |= !BuildOpts.SwitchKeepDefaultCoveredEnum &&
- Terminator->isAllEnumCasesCovered() &&
- Terminator->getSwitchCaseList();
+ SwitchAlwaysHasSuccessor |=
+ !BuildOpts.AssumeReachableDefaultInSwitchStatements &&
+ Terminator->isAllEnumCasesCovered() && Terminator->getSwitchCaseList();
addSuccessor(SwitchTerminatedBlock, DefaultCaseBlock,
!SwitchAlwaysHasSuccessor);
diff --git a/clang/unittests/Analysis/CFGTest.cpp b/clang/unittests/Analysis/CFGTest.cpp
index cce0f6bbcfa74..aeeff6b723532 100644
--- a/clang/unittests/Analysis/CFGTest.cpp
+++ b/clang/unittests/Analysis/CFGTest.cpp
@@ -107,7 +107,7 @@ TEST(CFG, SwitchCoveredEnumNoDefault) {
}
)";
CFG::BuildOptions Options;
- Options.SwitchKeepDefaultCoveredEnum = true;
+ Options.AssumeReachableDefaultInSwitchStatements = true;
BuildResult B = BuildCFG(Code, Options);
EXPECT_EQ(BuildResult::BuiltCFG, B.getStatus());
@@ -143,24 +143,25 @@ TEST(CFG, SwitchCoveredEnumNoDefault) {
// [B0 (EXIT)]
// Preds (3): B1 B3 B4
- const auto &Entry = B.getCFG()->getEntry();
- EXPECT_EQ(1u, Entry.succ_size());
+ auto *CFG = B.getCFG();
+ const auto &Entry = CFG->getEntry();
+ ASSERT_EQ(1u, Entry.succ_size());
// First successor of Entry is the switch
CFGBlock *SwitchBlock = *Entry.succ_begin();
- EXPECT_EQ(3u, SwitchBlock->succ_size());
+ ASSERT_EQ(3u, SwitchBlock->succ_size());
// Last successor of the switch is after the switch
auto NoCaseSucc = SwitchBlock->succ_rbegin();
EXPECT_TRUE(NoCaseSucc->isReachable());
// Checking that the same node is Unreachable without this setting
- Options.SwitchKeepDefaultCoveredEnum = false;
+ Options.AssumeReachableDefaultInSwitchStatements = false;
B = BuildCFG(Code, Options);
EXPECT_EQ(BuildResult::BuiltCFG, B.getStatus());
const auto &Entry2 = B.getCFG()->getEntry();
- EXPECT_EQ(1u, Entry2.succ_size());
+ ASSERT_EQ(1u, Entry2.succ_size());
CFGBlock *SwitchBlock2 = *Entry2.succ_begin();
- EXPECT_EQ(3u, SwitchBlock2->succ_size());
+ ASSERT_EQ(3u, SwitchBlock2->succ_size());
auto NoCaseSucc2 = SwitchBlock2->succ_rbegin();
EXPECT_FALSE(NoCaseSucc2->isReachable());
}
@@ -181,7 +182,7 @@ TEST(CFG, SwitchCoveredEnumWithDefault) {
}
)";
CFG::BuildOptions Options;
- Options.SwitchKeepDefaultCoveredEnum = true;
+ Options.AssumeReachableDefaultInSwitchStatements = true;
BuildResult B = BuildCFG(Code, Options);
EXPECT_EQ(BuildResult::BuiltCFG, B.getStatus());
@@ -224,23 +225,23 @@ TEST(CFG, SwitchCoveredEnumWithDefault) {
// Preds (4): B1 B3 B4 B5
const auto &Entry = B.getCFG()->getEntry();
- EXPECT_EQ(1u, Entry.succ_size());
+ ASSERT_EQ(1u, Entry.succ_size());
// First successor of Entry is the switch
CFGBlock *SwitchBlock = *Entry.succ_begin();
- EXPECT_EQ(3u, SwitchBlock->succ_size());
+ ASSERT_EQ(3u, SwitchBlock->succ_size());
// Last successor of the switch is the default branch
auto defaultBlock = SwitchBlock->succ_rbegin();
EXPECT_TRUE(defaultBlock->isReachable());
// Checking that the same node is Unreachable without this setting
- Options.SwitchKeepDefaultCoveredEnum = false;
+ Options.AssumeReachableDefaultInSwitchStatements = false;
B = BuildCFG(Code, Options);
EXPECT_EQ(BuildResult::BuiltCFG, B.getStatus());
const auto &Entry2 = B.getCFG()->getEntry();
- EXPECT_EQ(1u, Entry2.succ_size());
+ ASSERT_EQ(1u, Entry2.succ_size());
CFGBlock *SwitchBlock2 = *Entry2.succ_begin();
- EXPECT_EQ(3u, SwitchBlock2->succ_size());
+ ASSERT_EQ(3u, SwitchBlock2->succ_size());
auto defaultBlock2 = SwitchBlock2->succ_rbegin();
EXPECT_FALSE(defaultBlock2->isReachable());
}
More information about the cfe-commits
mailing list