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

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL requested changes to this revision.
PiotrZSL added a comment.
This revision now requires changes to proceed.

Missing documentation for check.
Once you ready with update, please switch review into ready for review.



================
Comment at: clang-tools-extra/clang-tidy/misc/IncompleteSwitchCheck.cpp:19
+  Finder->addMatcher(
+      switchStmt(has(implicitCastExpr().bind("cast")),
+                 unless(hasAncestor(switchStmt(has(defaultStmt())))))
----------------
no need to check casts, just check if its not enum... or check if its integer (validate type of expression).
and use IgnoreUnlessSpelledInSource to exclude implicit code (like other checks)


================
Comment at: clang-tools-extra/clang-tidy/misc/IncompleteSwitchCheck.cpp:20
+      switchStmt(has(implicitCastExpr().bind("cast")),
+                 unless(hasAncestor(switchStmt(has(defaultStmt())))))
+          .bind("switch"),
----------------
so switch in switch is allowed ? why 


================
Comment at: clang-tools-extra/clang-tidy/misc/IncompleteSwitchCheck.cpp:31
+
+  const auto *s = Result.Nodes.getNodeAs<SwitchStmt>("switch");
+  diag(s->getSwitchLoc(), "switching on non-enum value without "
----------------
put some meaningful names of variables according to LLVM standard


================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:222
 
+- New :doc:`misc-incomplete-switch
+  <clang-tidy/checks/misc/incomplete-switch>` check.
----------------
name is to generic. maybe misc-switch-missing-default-case or something similar, maybe it should be readability module or bugprone ? Still misc could be not bad trade-off.


================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:225
+
+  Detects incomplete switch statements without a default case.
+
----------------
put info about non enum, for enums we got compiler warnings already


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/misc/incomplete-switch.cpp:1
+// RUN: clang-tidy --checks='-*,misc-incomplete-switch' %s -- | FileCheck -implicit-check-not="{{warning|error}}:" %s
+
----------------
Check currently use different run command. Base on new one.


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

https://reviews.llvm.org/D4784