[clang-tools-extra] 42179bb - [clang-tidy] Add check for possibly incomplete switch statements

Shivam Gupta via cfe-commits cfe-commits at lists.llvm.org
Sun Jul 16 22:11:26 PDT 2023


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);
+}


        


More information about the cfe-commits mailing list