Author: Shivam Gupta Date: 2023-07-17T10:40:11+05:30 New Revision: 42179bbf6bcc9f90256b443c30f5e99f862bc2f6
URL: https://github.com/llvm/llvm-project/commit/42179bbf6bcc9f90256b443c30f5e99f862bc2f6 DIFF: https://github.com/llvm/llvm-project/commit/42179bbf6bcc9f90256b443c30f5e99f862bc2f6.diff LOG: [clang-tidy] Add check for possibly incomplete switch statements While clang warns about a possibly incomplete switch statement when switching over an enum variable and failing to cover all enum values (either explicitly or with a default case), no such warning is emitted if a plain integer variable is used as switch variable. Add a clang-tidy check to diagnose these scenarios. No fixit hint is provided since there are multiple possible solutions. Differential Revision: https://reviews.llvm.org/D4784 Added: clang-tools-extra/clang-tidy/bugprone/SwitchMissingDefaultCaseCheck.cpp clang-tools-extra/clang-tidy/bugprone/SwitchMissingDefaultCaseCheck.h clang-tools-extra/docs/clang-tidy/checks/bugprone/switch-missing-default-case.rst clang-tools-extra/test/clang-tidy/checkers/bugprone/switch-missing-default-case.cpp Modified: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index e62e536555c29c..7509e94950e10e 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -65,6 +65,7 @@ #include "SuspiciousSemicolonCheck.h" #include "SuspiciousStringCompareCheck.h" #include "SwappedArgumentsCheck.h" +#include "SwitchMissingDefaultCaseCheck.h" #include "TerminatingContinueCheck.h" #include "ThrowKeywordMissingCheck.h" #include "TooSmallLoopVariableCheck.h" @@ -116,6 +117,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-implicit-widening-of-multiplication-result"); CheckFactories.registerCheck<InaccurateEraseCheck>( "bugprone-inaccurate-erase"); + CheckFactories.registerCheck<SwitchMissingDefaultCaseCheck>( + "bugprone-switch-missing-default-case"); CheckFactories.registerCheck<IncorrectRoundingsCheck>( "bugprone-incorrect-roundings"); CheckFactories.registerCheck<InfiniteLoopCheck>("bugprone-infinite-loop"); diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index 363d1a85b0ae51..8bd892eeb41ecd 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -21,6 +21,7 @@ add_clang_library(clangTidyBugproneModule ForwardingReferenceOverloadCheck.cpp ImplicitWideningOfMultiplicationResultCheck.cpp InaccurateEraseCheck.cpp + SwitchMissingDefaultCaseCheck.cpp IncorrectRoundingsCheck.cpp InfiniteLoopCheck.cpp IntegerDivisionCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/SwitchMissingDefaultCaseCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SwitchMissingDefaultCaseCheck.cpp new file mode 100644 index 00000000000000..d1d50fe26b29e1 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/SwitchMissingDefaultCaseCheck.cpp @@ -0,0 +1,47 @@ +//===--- SwitchMissingDefaultCaseCheck.cpp - clang-tidy -------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "SwitchMissingDefaultCaseCheck.h" +#include "clang/AST/ASTContext.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +namespace { + +AST_MATCHER(SwitchStmt, hasDefaultCase) { + const SwitchCase *Case = Node.getSwitchCaseList(); + while (Case) { + if (DefaultStmt::classof(Case)) + return true; + + Case = Case->getNextSwitchCase(); + } + return false; +} +} // namespace + +void SwitchMissingDefaultCaseCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + switchStmt(hasCondition(expr(unless(isInstantiationDependent()), + hasType(qualType(hasCanonicalType( + unless(hasDeclaration(enumDecl()))))))), + unless(hasDefaultCase())) + .bind("switch"), + this); +} + +void SwitchMissingDefaultCaseCheck::check( + const ast_matchers::MatchFinder::MatchResult &Result) { + const auto *Switch = Result.Nodes.getNodeAs<SwitchStmt>("switch"); + + diag(Switch->getSwitchLoc(), "switching on non-enum value without " + "default case may not cover all cases"); +} +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/SwitchMissingDefaultCaseCheck.h b/clang-tools-extra/clang-tidy/bugprone/SwitchMissingDefaultCaseCheck.h new file mode 100644 index 00000000000000..b0d6e2062b997d --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/SwitchMissingDefaultCaseCheck.h @@ -0,0 +1,38 @@ +//===--- SwitchMissingDefaultCaseCheck.h - clang-tidy -----------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SWITCHMISSINGDEFAULTCASECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SWITCHMISSINGDEFAULTCASECHECK_H + +#include "../ClangTidyCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" + +namespace clang::tidy::bugprone { + +/// 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 +class SwitchMissingDefaultCaseCheck : public ClangTidyCheck { +public: + SwitchMissingDefaultCaseCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + std::optional<TraversalKind> getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SWITCHMISSINGDEFAULTCASECHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index e2b2ea3491aa8f..3b3bf0ca55508e 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -121,6 +121,12 @@ New checks Detect implicit and explicit casts of ``enum`` type into ``bool`` where ``enum`` type doesn't have a zero-value enumerator. +- New :doc:`bugprone-switch-missing-default-case + <clang-tidy/checks/bugprone/switch-missing-default-case>` check. + + Ensures that switch statements without default cases are flagged, focuses only + on covering cases with non-enums where the compiler may not issue warnings. + - New :doc:`bugprone-unique-ptr-array-mismatch <clang-tidy/checks/bugprone/unique-ptr-array-mismatch>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/switch-missing-default-case.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/switch-missing-default-case.rst new file mode 100644 index 00000000000000..648c2c208a4ecf --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/switch-missing-default-case.rst @@ -0,0 +1,56 @@ +.. title:: clang-tidy - bugprone-switch-missing-default-case + +bugprone-switch-missing-default-case +==================================== + +Ensures that switch statements without default cases are flagged, focuses only +on covering cases with non-enums where the compiler may not issue warnings. + +Switch statements without a default case can lead to unexpected +behavior and incomplete handling of all possible cases. When a switch statement +lacks a default case, if a value is encountered that does not match any of the +specified cases, the program will continue execution without any defined +behavior or handling. + +This check helps identify switch statements that are missing a default case, +allowing developers to ensure that all possible cases are handled properly. +Adding a default case allows for graceful handling of unexpected or unmatched +values, reducing the risk of program errors and unexpected behavior. + +Example: + +.. code-block:: c++ + + // Example 1: + // warning: switching on non-enum value without default case may not cover all cases + switch (i) { + case 0: + break; + } + + // Example 2: + enum E { eE1 }; + E e = eE1; + switch (e) { // no-warning + case eE1: + break; + } + + // Example 3: + int i = 0; + switch (i) { // no-warning + case 0: + break; + default: + break; + } + +.. note:: + Enum types are already covered by compiler warnings (comes under -Wswitch) + when a switch statement does not handle all enum values. This check focuses + on non-enum types where the compiler warnings may not be present. + +.. seealso:: + The `CppCoreGuideline ES.79 <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-default>`_ + provide guidelines on switch statements, including the recommendation to + always provide a default case. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 966e98672e0fb5..d7284a3c4a145b 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -92,6 +92,7 @@ Clang-Tidy Checks `bugprone-forwarding-reference-overload <bugprone/forwarding-reference-overload.html>`_, `bugprone-implicit-widening-of-multiplication-result <bugprone/implicit-widening-of-multiplication-result.html>`_, "Yes" `bugprone-inaccurate-erase <bugprone/inaccurate-erase.html>`_, "Yes" + `bugprone-switch-missing-default-case <bugprone/switch-missing-default-case.html>`_, `bugprone-incorrect-roundings <bugprone/incorrect-roundings.html>`_, `bugprone-infinite-loop <bugprone/infinite-loop.html>`_, `bugprone-integer-division <bugprone/integer-division.html>`_, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/switch-missing-default-case.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/switch-missing-default-case.cpp new file mode 100644 index 00000000000000..14e83c8207fdd7 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/switch-missing-default-case.cpp @@ -0,0 +1,80 @@ +// RUN: %check_clang_tidy %s bugprone-switch-missing-default-case %t -- -- -fno-delayed-template-parsing + +typedef int MyInt; +enum EnumType { eE2 }; +typedef EnumType MyEnum; + +void positive() { + int I1 = 0; + // CHECK-MESSAGES: [[@LINE+1]]:3: warning: switching on non-enum value without default case may not cover all cases [bugprone-switch-missing-default-case] + switch (I1) { + case 0: + break; + } + + MyInt I2 = 0; + // CHECK-MESSAGES: [[@LINE+1]]:3: warning: switching on non-enum value without default case may not cover all cases [bugprone-switch-missing-default-case] + switch (I2) { + case 0: + break; + } + + int getValue(void); + // CHECK-MESSAGES: [[@LINE+1]]:3: warning: switching on non-enum value without default case may not cover all cases [bugprone-switch-missing-default-case] + switch (getValue()) { + case 0: + break; + } +} + +void negative() { + enum E { eE1 }; + E E1 = eE1; + switch (E1) { // no-warning + case eE1: + break; + } + + MyEnum E2 = eE2; + switch (E2) { // no-warning + case eE2: + break; + } + + int I1 = 0; + switch (I1) { // no-warning + case 0: + break; + default: + break; + } + + MyInt I2 = 0; + switch (I2) { // no-warning + case 0: + break; + default: + break; + } + + int getValue(void); + switch (getValue()) { // no-warning + case 0: + break; + default: + break; + } +} + +template<typename T> +void testTemplate(T Value) { + switch (Value) { + case 0: + break; + } +} + +void exampleUsage() { + testTemplate(5); + testTemplate(EnumType::eE2); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
