https://github.com/frederic-tingaud-sonarsource created https://github.com/llvm/llvm-project/pull/161345
By default, the `default:` branch (or the successor if there is no `default` and cases return) of a switch on fully covered enumerations is considered as "Unreachable". It is a sane assumption in most cases, but not always. That commit allows to change such behavior when needed. >From 4af057dd6b5709d7de819108a14949ad5a0445fd Mon Sep 17 00:00:00 2001 From: Fred Tingaud <[email protected]> Date: Mon, 29 Sep 2025 18:37:48 +0200 Subject: [PATCH] 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); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
