[PATCH] D63139: [Diagnostics] Implement -Wswitch-unreachable

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D63139#1603667 <https://reviews.llvm.org/D63139#1603667>, @xbolva00 wrote:

> Ping again
>
> Is ‘CaseStmt´ AST C++ issue is a blocker for this patch or not?


I do not think it is a blocker for this patch. It's the existing behavior of the AST and while annoying, it can be improved later.



================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8192
+def warn_unreachable_stmt_in_switch : Warning<
+  "statement will be never executed">, InGroup<DiagGroup<"switch-unreachable">>;
 def warn_bool_switch_condition : Warning<
----------------
I thought we had a warning group for this already, and we do, it's `-Wunreachable-code`. I think the new diagnostic group be a child of the existing one, if we need the group at all.


================
Comment at: test/SemaCXX/warn-unreachable-stmt-switch.cpp:1-4
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wswitch-unreachable %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wswitch-unreachable %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify  %s
----------------
You should add a RUN line that also passes `-Wunreachable-code` to ensure this doesn't introduce duplicate diagnostics. It seems some number of these are already covered by that warning class: https://godbolt.org/z/f60YxB


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63139/new/

https://reviews.llvm.org/D63139