[clang] [Sema] Warn about omitting deprecated enumerator in switch (PR #138562)
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Tue May 20 09:51:33 PDT 2025
https://github.com/zmodem updated https://github.com/llvm/llvm-project/pull/138562
>From e221ba3b0f7b08bcfc56bf75f7505265c332637d Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Mon, 5 May 2025 20:24:15 +0200
Subject: [PATCH 1/6] [Sema] Warn about omitting deprecated enumerator in
switch
This undoes part of 3e4e3b17c14c15c23c0ed18ca9165b42b1b13ae3 which
added the "Omitting a deprecated constant is ok; it should never
materialize." logic.
That seems wrong: deprecated means the enumerator is likely to be
removed in future versions, not that it canot materialize.
---
clang/lib/Sema/SemaStmt.cpp | 6 +++++-
clang/test/Sema/switch-availability.c | 2 +-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index e8c1f8490342a..990d2fadaf5aa 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -1667,8 +1667,12 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch,
// Don't warn about omitted unavailable EnumConstantDecls.
switch (EI->second->getAvailability()) {
case AR_Deprecated:
- // Omitting a deprecated constant is ok; it should never materialize.
+ // Deprecated enumerators still need to be handled: they may be
+ // deprecated, but can still occur.
+ break;
+
case AR_Unavailable:
+ // Omitting an unavailable enumerator is ok; it should never occur.
continue;
case AR_NotYetIntroduced:
diff --git a/clang/test/Sema/switch-availability.c b/clang/test/Sema/switch-availability.c
index 888edddac463d..b4f8726addc0b 100644
--- a/clang/test/Sema/switch-availability.c
+++ b/clang/test/Sema/switch-availability.c
@@ -15,7 +15,7 @@ enum SwitchTwo {
};
void testSwitchTwo(enum SwitchTwo st) {
- switch (st) {} // expected-warning{{enumeration values 'Vim' and 'Emacs' not handled in switch}}
+ switch (st) {} // expected-warning{{enumeration values 'Ed', 'Vim' and 'Emacs' not handled in switch}}
}
enum SwitchThree {
>From 6bc923d27d77009b37de12c9e33d2e83835d6a4d Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Tue, 6 May 2025 09:31:50 +0200
Subject: [PATCH 2/6] check for -Wreturn-type
---
clang/test/Sema/switch-availability.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/clang/test/Sema/switch-availability.c b/clang/test/Sema/switch-availability.c
index b4f8726addc0b..137cdc976ec2d 100644
--- a/clang/test/Sema/switch-availability.c
+++ b/clang/test/Sema/switch-availability.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify -Wswitch -triple x86_64-apple-macosx10.12 %s
+// RUN: %clang_cc1 -verify -Wswitch -Wreturn-type -triple x86_64-apple-macosx10.12 %s
enum SwitchOne {
Unavail __attribute__((availability(macos, unavailable))),
@@ -25,3 +25,16 @@ enum SwitchThree {
void testSwitchThree(enum SwitchThree st) {
switch (st) {} // expected-warning{{enumeration value 'New' not handled in switch}}
}
+
+enum SwitchFour {
+ Red,
+ Green,
+ Blue [[deprecated]]
+};
+
+int testSwitchFour(enum SwitchFour e) {
+ switch (e) { // expected-warning{{enumeration value 'Blue' not handled in switch}}
+ case Red: return 1;
+ case Green: return 2;
+ }
+} // expected-warning{{non-void function does not return a value in all control paths}}
>From 432ab6cb5bc2fe11a575146e53af1083a96c8405 Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Tue, 6 May 2025 16:30:25 +0200
Subject: [PATCH 3/6] don't forget the oxford comma
---
clang/test/Sema/switch-availability.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/test/Sema/switch-availability.c b/clang/test/Sema/switch-availability.c
index 137cdc976ec2d..fc44375fc83a0 100644
--- a/clang/test/Sema/switch-availability.c
+++ b/clang/test/Sema/switch-availability.c
@@ -15,7 +15,7 @@ enum SwitchTwo {
};
void testSwitchTwo(enum SwitchTwo st) {
- switch (st) {} // expected-warning{{enumeration values 'Ed', 'Vim' and 'Emacs' not handled in switch}}
+ switch (st) {} // expected-warning{{enumeration values 'Ed', 'Vim', and 'Emacs' not handled in switch}}
}
enum SwitchThree {
>From 6530e7faae5be297bdfcaae7baee44b7321eb3c4 Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Thu, 15 May 2025 12:56:05 +0200
Subject: [PATCH 4/6] suppress 'deprecated' warning in case expressions
---
clang/include/clang/Sema/Sema.h | 3 +++
clang/lib/Parse/ParseExpr.cpp | 3 +++
clang/lib/Sema/SemaAvailability.cpp | 5 +++++
clang/lib/Sema/SemaStmt.cpp | 4 ++--
clang/test/Sema/switch-availability.c | 8 ++++++++
clang/test/SemaObjC/unguarded-availability.m | 6 +++---
6 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 19343eb0af092..bb69930446177 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6767,6 +6767,9 @@ class Sema final : public SemaBase {
};
std::optional<InitializationContext> DelayedDefaultInitializationContext;
+ /// Whether evaluating an expression for a switch case label.
+ bool IsCaseExpr = false;
+
ExpressionEvaluationContextRecord(ExpressionEvaluationContext Context,
unsigned NumCleanupObjects,
CleanupInfo ParentCleanup,
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 4b5d677f4ba87..094b677a50e80 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -273,8 +273,11 @@ ExprResult Parser::ParseArrayBoundExpression() {
}
ExprResult Parser::ParseCaseExpression(SourceLocation CaseLoc) {
+
EnterExpressionEvaluationContext ConstantEvaluated(
Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
+ Actions.currentEvaluationContext().IsCaseExpr = true;
+
ExprResult LHS(ParseCastExpression(CastParseKind::AnyCastExpr, false,
TypeCastState::NotTypeCast));
ExprResult Res(ParseRHSOfBinaryExpression(LHS, prec::Conditional));
diff --git a/clang/lib/Sema/SemaAvailability.cpp b/clang/lib/Sema/SemaAvailability.cpp
index 96aa65412906c..3a3b7b365d8e7 100644
--- a/clang/lib/Sema/SemaAvailability.cpp
+++ b/clang/lib/Sema/SemaAvailability.cpp
@@ -549,6 +549,11 @@ static void DoEmitAvailabilityWarning(Sema &S, AvailabilityResult K,
return;
}
case AR_Deprecated:
+ // Don't diagnose deprecated values in case expressions: they still need to
+ // be handled for the switch to be considered covered.
+ if (S.currentEvaluationContext().IsCaseExpr)
+ return;
+
diag = !ObjCPropertyAccess ? diag::warn_deprecated
: diag::warn_property_method_deprecated;
diag_message = diag::warn_deprecated_message;
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 990d2fadaf5aa..b841f994601e3 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -1667,8 +1667,8 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch,
// Don't warn about omitted unavailable EnumConstantDecls.
switch (EI->second->getAvailability()) {
case AR_Deprecated:
- // Deprecated enumerators still need to be handled: they may be
- // deprecated, but can still occur.
+ // Deprecated enumerators need to be handled: they may be deprecated,
+ // but can still occur.
break;
case AR_Unavailable:
diff --git a/clang/test/Sema/switch-availability.c b/clang/test/Sema/switch-availability.c
index fc44375fc83a0..df6cb03f9d5f6 100644
--- a/clang/test/Sema/switch-availability.c
+++ b/clang/test/Sema/switch-availability.c
@@ -38,3 +38,11 @@ int testSwitchFour(enum SwitchFour e) {
case Green: return 2;
}
} // expected-warning{{non-void function does not return a value in all control paths}}
+
+int testSwitchFourCovered(enum SwitchFour e) {
+ switch (e) {
+ case Red: return 1;
+ case Green: return 2;
+ case Blue: return 3; // no warning
+ }
+}
diff --git a/clang/test/SemaObjC/unguarded-availability.m b/clang/test/SemaObjC/unguarded-availability.m
index ecd91990174ae..4fd891ad099da 100644
--- a/clang/test/SemaObjC/unguarded-availability.m
+++ b/clang/test/SemaObjC/unguarded-availability.m
@@ -343,21 +343,21 @@ @interface BaseClass (CategoryWithNewProtocolRequirement) <NewProtocol>
@end
typedef enum {
- AK_Dodo __attribute__((availability(macos, deprecated=10.3))), // expected-note 3 {{marked deprecated here}}
+ AK_Dodo __attribute__((availability(macos, deprecated=10.3))), // expected-note 1 {{marked deprecated here}}
AK_Cat __attribute__((availability(macos, introduced=10.4))),
AK_CyborgCat __attribute__((availability(macos, introduced=10.12))), // expected-note {{'AK_CyborgCat' has been marked as being introduced in macOS 10.12 here, but the deployment target is macOS 10.9}}
} Animals;
void switchAnimals(Animals a) {
switch (a) {
- case AK_Dodo: break; // expected-warning{{'AK_Dodo' is deprecated}}
+ case AK_Dodo: break; // no warn
case AK_Cat: break;
case AK_Cat|AK_CyborgCat: break; // expected-warning{{case value not in enum}}
case AK_CyborgCat: break; // no warn
}
switch (a) {
- case AK_Dodo...AK_CyborgCat: // expected-warning {{'AK_Dodo' is depr}}
+ case AK_Dodo...AK_CyborgCat: // no warn
break;
}
>From a7a0be92ccdd62ae73d94519096c558dfccaa385 Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Thu, 15 May 2025 13:07:49 +0200
Subject: [PATCH 5/6] drop empty line
---
clang/lib/Parse/ParseExpr.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 094b677a50e80..75e9f8b6afd6d 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -273,7 +273,6 @@ ExprResult Parser::ParseArrayBoundExpression() {
}
ExprResult Parser::ParseCaseExpression(SourceLocation CaseLoc) {
-
EnterExpressionEvaluationContext ConstantEvaluated(
Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
Actions.currentEvaluationContext().IsCaseExpr = true;
>From 9523bbd125f437503f51a2ebd0fe1f25cce8f86e Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Tue, 20 May 2025 18:48:10 +0200
Subject: [PATCH 6/6] pack IsCaseExpr with the other contextual bools
---
clang/include/clang/Sema/Sema.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index bb69930446177..d6113e33e564c 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6747,6 +6747,9 @@ class Sema final : public SemaBase {
/// example, in a for-range initializer).
bool InLifetimeExtendingContext = false;
+ /// Whether evaluating an expression for a switch case label.
+ bool IsCaseExpr = false;
+
/// Whether we should rebuild CXXDefaultArgExpr and CXXDefaultInitExpr.
bool RebuildDefaultArgOrDefaultInit = false;
@@ -6767,9 +6770,6 @@ class Sema final : public SemaBase {
};
std::optional<InitializationContext> DelayedDefaultInitializationContext;
- /// Whether evaluating an expression for a switch case label.
- bool IsCaseExpr = false;
-
ExpressionEvaluationContextRecord(ExpressionEvaluationContext Context,
unsigned NumCleanupObjects,
CleanupInfo ParentCleanup,
More information about the cfe-commits
mailing list