[clang] [Sema] Warn about omitting deprecated enumerator in switch (PR #138562)
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Thu May 22 05:49: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/9] [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/9] 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/9] 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/9] 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/9] 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/9] 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,
>From 77739b69369a562e79a71eb68fe8372c1706b29f Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Wed, 21 May 2025 20:44:57 +0200
Subject: [PATCH 7/9] add a -Wdeprecated-switch-case flag
---
clang/include/clang/Basic/DiagnosticGroups.td | 3 ++-
clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 ++
clang/lib/Sema/SemaAvailability.cpp | 12 ++++++------
clang/test/Sema/switch-availability.c | 11 +++++++++--
clang/test/SemaObjC/unguarded-availability.m | 6 +++---
5 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 3c24387630d0c..6f15151e7e07e 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -233,7 +233,8 @@ def DeprecatedCopyWithDtor : DiagGroup<"deprecated-copy-with-dtor", [DeprecatedC
def DeprecatedLiteralOperator : DiagGroup<"deprecated-literal-operator">;
// For compatibility with GCC.
def : DiagGroup<"deprecated-copy-dtor", [DeprecatedCopyWithDtor]>;
-def DeprecatedDeclarations : DiagGroup<"deprecated-declarations">;
+def DeprecatedSwitchCase : DiagGroup<"deprecated-switch-case">;
+def DeprecatedDeclarations : DiagGroup<"deprecated-declarations", [DeprecatedSwitchCase]>;
def DeprecatedRedundantConstexprStaticDef : DiagGroup<"deprecated-redundant-constexpr-static-def">;
def UnavailableDeclarations : DiagGroup<"unavailable-declarations">;
def UnguardedAvailabilityNew : DiagGroup<"unguarded-availability-new">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index e5a7cdc14a737..92eb554f0750c 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6009,6 +6009,8 @@ def note_not_found_by_two_phase_lookup : Note<"%0 should be declared prior to th
def err_undeclared_use : Error<"use of undeclared %0">;
def warn_deprecated : Warning<"%0 is deprecated">,
InGroup<DeprecatedDeclarations>;
+def warn_deprecated_switch_case : Warning<"%0 is deprecated">,
+ InGroup<DeprecatedSwitchCase>;
def note_from_diagnose_if : Note<"from 'diagnose_if' attribute on %0:">;
def warn_property_method_deprecated :
Warning<"property access is using %0 method which is deprecated">,
diff --git a/clang/lib/Sema/SemaAvailability.cpp b/clang/lib/Sema/SemaAvailability.cpp
index 3a3b7b365d8e7..04dddaf92e67c 100644
--- a/clang/lib/Sema/SemaAvailability.cpp
+++ b/clang/lib/Sema/SemaAvailability.cpp
@@ -549,13 +549,13 @@ 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;
+ if (ObjCPropertyAccess)
+ diag = diag::warn_property_method_deprecated;
+ else if (S.currentEvaluationContext().IsCaseExpr)
+ diag = diag::warn_deprecated_switch_case;
+ else
+ diag = diag::warn_deprecated;
- diag = !ObjCPropertyAccess ? diag::warn_deprecated
- : diag::warn_property_method_deprecated;
diag_message = diag::warn_deprecated_message;
diag_fwdclass_message = diag::warn_deprecated_fwdclass_message;
property_note_select = /* deprecated */ 0;
diff --git a/clang/test/Sema/switch-availability.c b/clang/test/Sema/switch-availability.c
index df6cb03f9d5f6..517b3f100b24c 100644
--- a/clang/test/Sema/switch-availability.c
+++ b/clang/test/Sema/switch-availability.c
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -verify -Wswitch -Wreturn-type -triple x86_64-apple-macosx10.12 %s
+// RUN: %clang_cc1 -verify -Wswitch -Wreturn-type -Wno-deprecated-switch-case -DNO_DEPRECATED_CASE -triple x86_64-apple-macosx10.12 %s
enum SwitchOne {
Unavail __attribute__((availability(macos, unavailable))),
@@ -29,6 +30,9 @@ void testSwitchThree(enum SwitchThree st) {
enum SwitchFour {
Red,
Green,
+#ifndef NO_DEPRECATED_CASE
+// expected-note at +2{{'Blue' has been explicitly marked deprecated here}}
+#endif
Blue [[deprecated]]
};
@@ -43,6 +47,9 @@ int testSwitchFourCovered(enum SwitchFour e) {
switch (e) {
case Red: return 1;
case Green: return 2;
- case Blue: return 3; // no warning
- }
+#ifndef NO_DEPRECATED_CASE
+// expected-warning at +2{{'Blue' is deprecated}}
+#endif
+ case Blue: return 3;
+ } // no warning
}
diff --git a/clang/test/SemaObjC/unguarded-availability.m b/clang/test/SemaObjC/unguarded-availability.m
index 4fd891ad099da..ecd91990174ae 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 1 {{marked deprecated here}}
+ AK_Dodo __attribute__((availability(macos, deprecated=10.3))), // expected-note 3 {{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; // no warn
+ case AK_Dodo: break; // expected-warning{{'AK_Dodo' is deprecated}}
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: // no warn
+ case AK_Dodo...AK_CyborgCat: // expected-warning {{'AK_Dodo' is depr}}
break;
}
>From 37da445486819b248ed4d70677720a1bbe44f7f0 Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Thu, 22 May 2025 09:54:43 +0200
Subject: [PATCH 8/9] reuse the warn_deprecated text
---
clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 92eb554f0750c..2654348188f24 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6009,7 +6009,7 @@ def note_not_found_by_two_phase_lookup : Note<"%0 should be declared prior to th
def err_undeclared_use : Error<"use of undeclared %0">;
def warn_deprecated : Warning<"%0 is deprecated">,
InGroup<DeprecatedDeclarations>;
-def warn_deprecated_switch_case : Warning<"%0 is deprecated">,
+def warn_deprecated_switch_case : Warning<warn_deprecated.Summary>,
InGroup<DeprecatedSwitchCase>;
def note_from_diagnose_if : Note<"from 'diagnose_if' attribute on %0:">;
def warn_property_method_deprecated :
>From df403dadff3ead77f288bd3dd93666fb9357969c Mon Sep 17 00:00:00 2001
From: Hans Wennborg <hans at chromium.org>
Date: Thu, 22 May 2025 14:48:35 +0200
Subject: [PATCH 9/9] release note
---
clang/docs/ReleaseNotes.rst | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 918ff952bb2c3..95cff4c138503 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -497,6 +497,33 @@ Improvements to Clang's diagnostics
- ``-Wreserved-identifier`` now fires on reserved parameter names in a function
declaration which is not a definition.
+- ``-Wswitch`` will now diagnose unhandled enumerators in switches also when
+ the enumerator is deprecated. Warnings about using deprecated enumerators in
+ switch cases have moved behind a new ``-Wdeprecated-switch-case`` flag.
+
+ For example:
+
+ .. code-block:: c
+
+ enum E {
+ Red,
+ Green,
+ Blue [[deprecated]]
+ };
+ void example(enum E e) {
+ switch (e) {
+ case Red: // stuff...
+ case Green: // stuff...
+ }
+ }
+
+ will result in a warning about ``Blue`` not being handled in the switch.
+
+ The warning can be fixed either by adding a ``default:``, or by adding
+ ``case Blue:``. Since the enumerator is deprecated, the latter approach will
+ trigger a ``'Blue' is deprecated`` warning, which can be turned off with
+ ``-Wno-deprecated-switch-case``.
+
Improvements to Clang's time-trace
----------------------------------
More information about the cfe-commits
mailing list