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