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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to