[PATCH] D4784: [clang-tidy] Add check for possibly incomplete switch statements

Piotr Zegar via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Jul 16 21:15:54 PDT 2023


PiotrZSL accepted this revision.
PiotrZSL added a comment.
This revision is now accepted and ready to land.

Just few nits (column numbers in test, missing doxygen comment, ...).
Please fix those before committing.

Except that, looking good to me.



================
Comment at: clang-tools-extra/clang-tidy/bugprone/SwitchMissingDefaultCaseCheck.cpp:16-25
+AST_MATCHER(SwitchStmt, hasDefaultCase) {
+  const SwitchCase *Case = Node.getSwitchCaseList();
+  while (Case) {
+    if (DefaultStmt::classof(Case))
+      return true;
+
+    Case = Case->getNextSwitchCase();
----------------
Put this into anonymous namespace to avoid ODR issues


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SwitchMissingDefaultCaseCheck.cpp:40-41
+  const auto *Switch = Result.Nodes.getNodeAs<SwitchStmt>("switch");
+  if (!Switch)
+    return;
+
----------------
should never happend


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SwitchMissingDefaultCaseCheck.h:18
+
+class SwitchMissingDefaultCaseCheck : public ClangTidyCheck {
+public:
----------------
Missing class doxygen comment.
Check other classes, should be something like:

```
/// Ensures that switch statements without default cases are flagged, focuses only
/// on covering cases with non-enums where the compiler may not issue warnings.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/switch-missing-default-case.html

```


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/switch-missing-default-case.cpp:9
+  int I1 = 0;
+  // CHECK-MESSAGES: [[@LINE+1]]:11: warning: switching on non-enum value without default case may not cover all cases [bugprone-switch-missing-default-case]
+  switch (I1) {
----------------



================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/switch-missing-default-case.cpp:16
+  MyInt I2 = 0;
+  // CHECK-MESSAGES: [[@LINE+1]]:11: warning: switching on non-enum value without default case may not cover all cases [bugprone-switch-missing-default-case]
+  switch (I2) {
----------------



================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/switch-missing-default-case.cpp:23
+  int getValue(void);
+  // CHECK-MESSAGES: [[@LINE+1]]:11: warning: switching on non-enum value without default case may not cover all cases [bugprone-switch-missing-default-case]
+  switch (getValue()) {
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D4784



More information about the cfe-commits mailing list