From 42179bbf6bcc9f90256b443c30f5e99f862bc2f6 Mon Sep 17 00:00:00 2001 From: Shivam Gupta Date: Sun, 16 Jul 2023 22:13:55 +0530 Subject: [PATCH] [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 --- .../clang-tidy/bugprone/BugproneTidyModule.cpp | 3 + .../clang-tidy/bugprone/CMakeLists.txt | 1 + .../bugprone/SwitchMissingDefaultCaseCheck.cpp | 47 +++++++++++++ .../bugprone/SwitchMissingDefaultCaseCheck.h | 38 ++++++++++ clang-tools-extra/docs/ReleaseNotes.rst | 6 ++ .../bugprone/switch-missing-default-case.rst | 56 +++++++++++++++ clang-tools-extra/docs/clang-tidy/checks/list.rst | 1 + .../bugprone/switch-missing-default-case.cpp | 80 ++++++++++++++++++++++ 8 files changed, 232 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/bugprone/SwitchMissingDefaultCaseCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/SwitchMissingDefaultCaseCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/switch-missing-default-case.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/switch-missing-default-case.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index e62e536..7509e94 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 @@ public: "bugprone-implicit-widening-of-multiplication-result"); CheckFactories.registerCheck( "bugprone-inaccurate-erase"); + CheckFactories.registerCheck( + "bugprone-switch-missing-default-case"); CheckFactories.registerCheck( "bugprone-incorrect-roundings"); CheckFactories.registerCheck("bugprone-infinite-loop"); diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index 363d1a8..8bd892e 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 0000000..d1d50fe --- /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("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 0000000..b0d6e20 --- /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 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 e2b2ea3..3b3bf0c 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 + ` 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 ` 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 0000000..648c2c2 --- /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 `_ + 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 966e986..d7284a3 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-implicit-widening-of-multiplication-result `_, "Yes" `bugprone-inaccurate-erase `_, "Yes" + `bugprone-switch-missing-default-case `_, `bugprone-incorrect-roundings `_, `bugprone-infinite-loop `_, `bugprone-integer-division `_, 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 0000000..14e83c8 --- /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 +void testTemplate(T Value) { + switch (Value) { + case 0: + break; + } +} + +void exampleUsage() { + testTemplate(5); + testTemplate(EnumType::eE2); +} -- 2.7.4