[Sema] Warn about omitting deprecated enumerator in switch (#138562)

This undoes part of 3e4e3b1 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 cannot materialize.

Also move warnings about the use of deprecated enumerators in switch cases
behind a separate flag, -Wdeprecated-switch-case, for users who wish to
handle such enums explicitly and suppress the warning.

Author: zmodem

clang/docs/ReleaseNotes.rst

@@ -565,6 +565,33 @@ Improvements to Clang's diagnostics
 
 - Clang now suggests corrections for unknown attribute names.
 
+- ``-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
 ----------------------------------
 

clang/include/clang/Basic/DiagnosticGroups.td

@@ -234,7 +234,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">;

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -6038,6 +6038,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<warn_deprecated.Summary>,
+    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">,

clang/include/clang/Sema/Sema.h

@@ -6761,6 +6761,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;
 

clang/lib/Parse/ParseExpr.cpp

@@ -169,6 +169,8 @@ 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));

clang/lib/Sema/SemaAvailability.cpp

@@ -547,8 +547,13 @@ static void DoEmitAvailabilityWarning(Sema &S, AvailabilityResult K,
     return;
   }
   case AR_Deprecated:
-    diag = !ObjCPropertyAccess ? diag::warn_deprecated
-                               : diag::warn_property_method_deprecated;
+    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_message = diag::warn_deprecated_message;
     diag_fwdclass_message = diag::warn_deprecated_fwdclass_message;
     property_note_select = /* deprecated */ 0;

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 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:

clang/test/Sema/switch-availability.c

@@ -1,4 +1,5 @@
-// 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
+// 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))),
@@ -15,7 +16,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 {
@@ -25,3 +26,30 @@ enum SwitchThree {
 void testSwitchThree(enum SwitchThree st) {
   switch (st) {} // expected-warning{{enumeration value 'New' not handled in switch}}
 }
+
+enum SwitchFour {
+  Red,
+  Green,
+#ifndef NO_DEPRECATED_CASE
+// expected-note@+2{{'Blue' has been explicitly marked deprecated here}}
+#endif
+  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}}
+
+int testSwitchFourCovered(enum SwitchFour e) {
+  switch (e) {
+  case Red:   return 1;
+  case Green: return 2;
+#ifndef NO_DEPRECATED_CASE
+// expected-warning@+2{{'Blue' is deprecated}}
+#endif
+  case Blue:  return 3;
+  } // no warning
+}