[clang] Don't treat the default switch path as unreachable when all enumerators are covered (PR #123166)
Jeremy Rifkin via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 16 17:13:17 PST 2025
https://github.com/jeremy-rifkin updated https://github.com/llvm/llvm-project/pull/123166
>From e1ce92c0f54301cacaba316d38d44d20c6d61cb8 Mon Sep 17 00:00:00 2001
From: Jeremy Rifkin <51220084+jeremy-rifkin at users.noreply.github.com>
Date: Thu, 16 Jan 2025 00:27:03 -0600
Subject: [PATCH 1/3] Don't treat the default switch case as unreachable even
when all enumerators are covered
---
clang/lib/Analysis/CFG.cpp | 12 ++----------
clang/lib/Sema/AnalysisBasedWarnings.cpp | 3 ---
clang/test/Analysis/cfg.cpp | 8 ++++----
...cxx-uninitialized-object-unguarded-access.cpp | 2 ++
clang/test/Sema/return.c | 2 +-
clang/test/SemaCXX/array-bounds.cpp | 16 +++++++---------
6 files changed, 16 insertions(+), 27 deletions(-)
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 304bbb2b422c61..b819df35189bb8 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -4408,17 +4408,9 @@ CFGBlock *CFGBuilder::VisitSwitchStmt(SwitchStmt *Terminator) {
}
// If we have no "default:" case, the default transition is to the code
- // following the switch body. Moreover, take into account if all the
- // cases of a switch are covered (e.g., switching on an enum value).
- //
- // Note: We add a successor to a switch that is considered covered yet has no
- // case statements if the enumeration has no enumerators.
- bool SwitchAlwaysHasSuccessor = false;
- SwitchAlwaysHasSuccessor |= switchExclusivelyCovered;
- SwitchAlwaysHasSuccessor |= Terminator->isAllEnumCasesCovered() &&
- Terminator->getSwitchCaseList();
+ // following the switch body.
addSuccessor(SwitchTerminatedBlock, DefaultCaseBlock,
- !SwitchAlwaysHasSuccessor);
+ !switchExclusivelyCovered);
// Add the terminator and condition in the switch block.
SwitchTerminatedBlock->setTerminator(Terminator);
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 589869d0186575..2fa6f0a881e808 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -456,10 +456,7 @@ static ControlFlowKind CheckFallThrough(AnalysisDeclContext &AC) {
bool HasPlainEdge = false;
bool HasAbnormalEdge = false;
- // Ignore default cases that aren't likely to be reachable because all
- // enums in a switch(X) have explicit case statements.
CFGBlock::FilterOptions FO;
- FO.IgnoreDefaultsWithCoveredEnums = 1;
for (CFGBlock::filtered_pred_iterator I =
cfg->getExit().filtered_pred_start_end(FO);
diff --git a/clang/test/Analysis/cfg.cpp b/clang/test/Analysis/cfg.cpp
index 44a89df28e3b29..940994a7932192 100644
--- a/clang/test/Analysis/cfg.cpp
+++ b/clang/test/Analysis/cfg.cpp
@@ -178,7 +178,7 @@ namespace NoReturnSingleSuccessor {
// CHECK-NEXT: 1: x
// CHECK-NEXT: 2: [B1.1] (ImplicitCastExpr, LValueToRValue, int)
// CHECK-NEXT: 3: return [B1.2];
-// CHECK-NEXT: Preds (5): B3 B4 B5 B6 B2(Unreachable)
+// CHECK-NEXT: Preds (5): B3 B4 B5 B6 B2
// CHECK-NEXT: Succs (1): B0
// CHECK: [B2]
// CHECK-NEXT: 1: 0
@@ -188,7 +188,7 @@ namespace NoReturnSingleSuccessor {
// CHECK-NEXT: 5: [B2.4] (ImplicitCastExpr, IntegralCast, int)
// CHECK-NEXT: T: switch [B2.5]
// CHECK-NEXT: Preds (1): B7
-// CHECK-NEXT: Succs (5): B3 B4 B5 B6 B1(Unreachable)
+// CHECK-NEXT: Succs (5): B3 B4 B5 B6 B1
// CHECK: [B3]
// CHECK-NEXT: case D:
// CHECK-NEXT: 1: 4
@@ -254,14 +254,14 @@ int test_enum_with_extension(enum MyEnum value) {
// CHECK-NEXT: 5: [B2.4] (ImplicitCastExpr, IntegralCast, int)
// CHECK-NEXT: T: switch [B2.5]
// CHECK-NEXT: Preds (1): B7
-// CHECK-NEXT: Succs (4): B4 B5 B6 B3(Unreachable)
+// CHECK-NEXT: Succs (4): B4 B5 B6 B3
// CHECK: [B3]
// CHECK-NEXT: default:
// CHECK-NEXT: 1: 4
// CHECK-NEXT: 2: x
// CHECK-NEXT: 3: [B3.2] = [B3.1]
// CHECK-NEXT: T: break;
-// CHECK-NEXT: Preds (1): B2(Unreachable)
+// CHECK-NEXT: Preds (1): B2
// CHECK-NEXT: Succs (1): B1
// CHECK: [B4]
// CHECK-NEXT: case C:
diff --git a/clang/test/Analysis/cxx-uninitialized-object-unguarded-access.cpp b/clang/test/Analysis/cxx-uninitialized-object-unguarded-access.cpp
index 611e1d8255976c..b791c527c2e33e 100644
--- a/clang/test/Analysis/cxx-uninitialized-object-unguarded-access.cpp
+++ b/clang/test/Analysis/cxx-uninitialized-object-unguarded-access.cpp
@@ -383,6 +383,7 @@ class SwitchGuardedFieldsTest {
case Kind::V:
return -1;
}
+ halt();
}
int operator+() {
@@ -392,6 +393,7 @@ class SwitchGuardedFieldsTest {
case Kind::V:
return -1;
}
+ halt();
}
};
diff --git a/clang/test/Sema/return.c b/clang/test/Sema/return.c
index 14d5300e819492..b0814f152cbe95 100644
--- a/clang/test/Sema/return.c
+++ b/clang/test/Sema/return.c
@@ -265,7 +265,7 @@ int test_enum_cases(enum Cases C) {
case C4: return 3;
case C3: return 4;
}
-} // no-warning
+} // expected-warning {{non-void function does not return a value in all control paths}}
// PR12318 - Don't give a may reach end of non-void function warning.
int test34(int x) {
diff --git a/clang/test/SemaCXX/array-bounds.cpp b/clang/test/SemaCXX/array-bounds.cpp
index b584e1e7cd4537..3889a4bd0f4687 100644
--- a/clang/test/SemaCXX/array-bounds.cpp
+++ b/clang/test/SemaCXX/array-bounds.cpp
@@ -133,7 +133,7 @@ int test_pr9296() {
int test_sizeof_as_condition(int flag) {
int arr[2] = { 0, 0 }; // expected-note {{array 'arr' declared here}}
- if (flag)
+ if (flag)
return sizeof(char) != sizeof(char) ? arr[2] : arr[1];
return sizeof(char) == sizeof(char) ? arr[2] : arr[1]; // expected-warning {{array index 2 is past the end of the array (that has type 'int[2]')}}
}
@@ -170,16 +170,14 @@ void test_nested_switch() {
}
}
-// Test that if all the values of an enum covered, that the 'default' branch
-// is unreachable.
+// Test that a warning is not emitted if the code is unreachable.
enum Values { A, B, C, D };
void test_all_enums_covered(enum Values v) {
int x[2];
- switch (v) {
- case A: return;
- case B: return;
- case C: return;
- case D: return;
+ if(v == A) {
+ return;
+ } else {
+ return;
}
x[2] = 0; // no-warning
}
@@ -244,7 +242,7 @@ void test_pr10771() {
}
int test_pr11007_aux(const char * restrict, ...);
-
+
// Test checking with varargs.
void test_pr11007() {
double a[5]; // expected-note {{array 'a' declared here}}
>From 75fb28290148215d30b8286fe95a997728eca5cd Mon Sep 17 00:00:00 2001
From: Jeremy Rifkin <51220084+jeremy-rifkin at users.noreply.github.com>
Date: Thu, 16 Jan 2025 00:42:18 -0600
Subject: [PATCH 2/3] Update objective c warn-called-once
---
clang/test/SemaObjC/warn-called-once.m | 21 ---------------------
1 file changed, 21 deletions(-)
diff --git a/clang/test/SemaObjC/warn-called-once.m b/clang/test/SemaObjC/warn-called-once.m
index f23e41e00ee298..e121b27a97345b 100644
--- a/clang/test/SemaObjC/warn-called-once.m
+++ b/clang/test/SemaObjC/warn-called-once.m
@@ -444,27 +444,6 @@ void never_called_switch_none(int cond, void (^callback)(void) CALLED_ONCE) {
}
}
-enum YesNoOrMaybe {
- YES,
- NO,
- MAYBE
-};
-
-void exhaustive_switch(enum YesNoOrMaybe cond, void (^callback)(void) CALLED_ONCE) {
- switch (cond) {
- case YES:
- callback();
- break;
- case NO:
- callback();
- break;
- case MAYBE:
- callback();
- break;
- }
- // no-warning
-}
-
void called_twice_exceptions(void (^callback)(void) CALLED_ONCE) {
// TODO: Obj-C exceptions are not supported in CFG,
// we should report warnings in these as well.
>From 05c6f753055a48a5fc5623368110f4b96d7058e7 Mon Sep 17 00:00:00 2001
From: Jeremy Rifkin <51220084+jeremy-rifkin at users.noreply.github.com>
Date: Thu, 16 Jan 2025 19:13:07 -0600
Subject: [PATCH 3/3] Update return.mm test case
---
clang/test/CodeGenObjCXX/return.mm | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/clang/test/CodeGenObjCXX/return.mm b/clang/test/CodeGenObjCXX/return.mm
index fb77f336dfc00a..16284c54eaaf56 100644
--- a/clang/test/CodeGenObjCXX/return.mm
+++ b/clang/test/CodeGenObjCXX/return.mm
@@ -12,19 +12,7 @@ - (int)method {
@end
-enum Enum {
- a
-};
-
-int (^block)(Enum) = ^int(Enum e) {
- switch (e) {
- case a:
- return 1;
- }
-};
-
-// Ensure that both methods and blocks don't use the -fstrict-return undefined
-// behaviour optimization.
+// Ensure that methods don't use the -fstrict-return undefined behaviour optimization.
// CHECK-NOT: call void @llvm.trap
// CHECK-NOT: unreachable
More information about the cfe-commits
mailing list