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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
