[clang-tools-extra] r318600 - [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

Jonas Toth via cfe-commits cfe-commits at lists.llvm.org
Sat Nov 18 11:48:33 PST 2017


Author: jonastoth
Date: Sat Nov 18 11:48:33 2017
New Revision: 318600

URL: http://llvm.org/viewvc/llvm-project?rev=318600&view=rev
Log:
[clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

Summary:
This check searches for missing `else` branches in `if-else if`-chains and
missing `default` labels in `switch` statements, that use integers as condition.

It is very similar to -Wswitch, but concentrates on integers only, since enums are
already covered.

The option to warn for missing `else` branches is deactivated by default, since it is
very noise on larger code bases.

Running it on LLVM:
{F5354858} for default configuration
{F5354866} just for llvm/lib/Analysis/ScalarEvolution.cpp, the else-path checker is very noisy!

Reviewers: alexfh, aaron.ballman, hokein

Reviewed By: aaron.ballman

Subscribers: lebedev.ri, Eugene.Zelenko, cfe-commits, mgorny, JDevlieghere, xazax.hun

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D37808


Added:
    clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
    clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.h
    clang-tools-extra/trunk/docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst
    clang-tools-extra/trunk/test/clang-tidy/hicpp-multiway-paths-covered-else.cpp
    clang-tools-extra/trunk/test/clang-tidy/hicpp-multiway-paths-covered.cpp
Modified:
    clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt
    clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
    clang-tools-extra/trunk/docs/ReleaseNotes.rst
    clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt?rev=318600&r1=318599&r2=318600&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/hicpp/CMakeLists.txt Sat Nov 18 11:48:33 2017
@@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyHICPPModule
   ExceptionBaseclassCheck.cpp
+  MultiwayPathsCoveredCheck.cpp
   NoAssemblerCheck.cpp
   HICPPTidyModule.cpp
   SignedBitwiseCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp?rev=318600&r1=318599&r2=318600&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp Sat Nov 18 11:48:33 2017
@@ -35,6 +35,7 @@
 #include "../readability/FunctionSizeCheck.h"
 #include "../readability/IdentifierNamingCheck.h"
 #include "ExceptionBaseclassCheck.h"
+#include "MultiwayPathsCoveredCheck.h"
 #include "NoAssemblerCheck.h"
 #include "SignedBitwiseCheck.h"
 
@@ -53,6 +54,8 @@ public:
         "hicpp-exception-baseclass");
     CheckFactories.registerCheck<SignedBitwiseCheck>(
         "hicpp-signed-bitwise");
+    CheckFactories.registerCheck<MultiwayPathsCoveredCheck>(
+        "hicpp-multiway-paths-covered");
     CheckFactories.registerCheck<google::ExplicitConstructorCheck>(
         "hicpp-explicit-conversions");
     CheckFactories.registerCheck<readability::FunctionSizeCheck>(

Added: clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp?rev=318600&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp Sat Nov 18 11:48:33 2017
@@ -0,0 +1,179 @@
+//===--- MultiwayPathsCoveredCheck.cpp - clang-tidy------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "MultiwayPathsCoveredCheck.h"
+#include "clang/AST/ASTContext.h"
+
+#include <limits>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace hicpp {
+
+void MultiwayPathsCoveredCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "WarnOnMissingElse", WarnOnMissingElse);
+}
+
+void MultiwayPathsCoveredCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      stmt(allOf(
+          anyOf(switchStmt(forEachSwitchCase(defaultStmt()))
+                    .bind("switch-default"),
+                switchStmt(unless(hasDescendant(switchCase())))
+                    .bind("degenerate-switch"),
+                // This matcher must be the last one of the three
+                // 'switchStmt' options.
+                // Otherwise the cases 'switch-default' and
+                // 'degenerate-switch' are not found correctly.
+                switchStmt(forEachSwitchCase(unless(defaultStmt())))
+                    .bind("switch-no-default")),
+          switchStmt(hasCondition(allOf(
+              // Match on switch statements that have either a bit-field or an
+              // integer condition. The ordering in 'anyOf()' is important
+              // because the last condition is the most general.
+              anyOf(ignoringImpCasts(memberExpr(hasDeclaration(
+                        fieldDecl(isBitField()).bind("bitfield")))),
+                    hasDescendant(declRefExpr().bind("non-enum-condition"))),
+              // 'unless()' must be the last match here and must be bound,
+              // otherwise the matcher does not work correctly.
+              unless(hasDescendant(declRefExpr(hasType(enumType()))
+                                       .bind("enum-condition")))))))),
+      this);
+
+  // This option is noisy, therefore matching is configurable.
+  if (WarnOnMissingElse) {
+    Finder->addMatcher(
+        ifStmt(allOf(hasParent(ifStmt()), unless(hasElse(anything()))))
+            .bind("else-if"),
+        this);
+  }
+}
+
+static unsigned countCaseLabels(const SwitchStmt *Switch) {
+  unsigned CaseCount = 0;
+
+  const SwitchCase *CurrentCase = Switch->getSwitchCaseList();
+  while (CurrentCase) {
+    ++CaseCount;
+    CurrentCase = CurrentCase->getNextSwitchCase();
+  }
+
+  return CaseCount;
+}
+/// This function calculate 2 ** Bits and returns
+/// numeric_limits<std::size_t>::max() if an overflow occured.
+static std::size_t twoPow(std::size_t Bits) {
+  return Bits >= std::numeric_limits<std::size_t>::digits
+             ? std::numeric_limits<std::size_t>::max()
+             : static_cast<size_t>(1) << Bits;
+}
+/// Get the number of possible values that can be switched on for the type T.
+///
+/// \return - 0 if bitcount could not be determined
+///         - numeric_limits<std::size_t>::max() when overflow appeared due to
+///           more then 64 bits type size.
+static std::size_t getNumberOfPossibleValues(QualType T,
+                                             const ASTContext &Context) {
+  // `isBooleanType` must come first because `bool` is an integral type as well
+  // and would not return 2 as result.
+  if (T->isBooleanType())
+    return 2;
+  else if (T->isIntegralType(Context))
+    return twoPow(Context.getTypeSize(T));
+  else
+    return 1;
+}
+
+void MultiwayPathsCoveredCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *ElseIfWithoutElse =
+          Result.Nodes.getNodeAs<IfStmt>("else-if")) {
+    diag(ElseIfWithoutElse->getLocStart(),
+         "potentially uncovered codepath; add an ending else statement");
+    return;
+  }
+  // Checks the sanity of 'switch' statements that actually do define
+  // a default branch but might be degenerated by having no or only one case.
+  if (const auto *SwitchWithDefault =
+          Result.Nodes.getNodeAs<SwitchStmt>("switch-default")) {
+    handleSwitchWithDefault(SwitchWithDefault);
+    return;
+  }
+  // Checks all 'switch' statements that do not define a default label.
+  // Here the heavy lifting happens.
+  if (const auto *SwitchWithoutDefault =
+          Result.Nodes.getNodeAs<SwitchStmt>("switch-no-default")) {
+    handleSwitchWithoutDefault(SwitchWithoutDefault, Result);
+    return;
+  }
+  // Warns for degenerated 'switch' statements that neither define a case nor
+  // a default label.
+  if (const auto *DegenerateSwitch =
+          Result.Nodes.getNodeAs<SwitchStmt>("degenerate-switch")) {
+    diag(DegenerateSwitch->getLocStart(), "degenerated switch without labels");
+    return;
+  }
+  llvm_unreachable("matched a case, that was not explicitly handled");
+}
+
+void MultiwayPathsCoveredCheck::handleSwitchWithDefault(
+    const SwitchStmt *Switch) {
+  unsigned CaseCount = countCaseLabels(Switch);
+  assert(CaseCount > 0 && "Switch statement with supposedly one default "
+                          "branch did not contain any case labels");
+  if (CaseCount == 1 || CaseCount == 2)
+    diag(Switch->getLocStart(),
+         CaseCount == 1
+             ? "degenerated switch with default label only"
+             : "switch could be better written as an if/else statement");
+}
+
+void MultiwayPathsCoveredCheck::handleSwitchWithoutDefault(
+    const SwitchStmt *Switch, const MatchFinder::MatchResult &Result) {
+  // The matcher only works because some nodes are explicitly matched and
+  // bound but ignored. This is necessary to build the excluding logic for
+  // enums and 'switch' statements without a 'default' branch.
+  assert(!Result.Nodes.getNodeAs<DeclRefExpr>("enum-condition") &&
+         "switch over enum is handled by warnings already, explicitly ignoring "
+         "them");
+  assert(!Result.Nodes.getNodeAs<SwitchStmt>("switch-default") &&
+         "switch stmts with default branch do cover all paths, explicitly "
+         "ignoring them");
+  // Determine the number of case labels. Since 'default' is not present
+  // and duplicating case labels is not allowed this number represents
+  // the number of codepaths. It can be directly compared to 'MaxPathsPossible'
+  // to see if some cases are missing.
+  unsigned CaseCount = countCaseLabels(Switch);
+  // CaseCount == 0 is caught in DegenerateSwitch. Necessary because the
+  // matcher used for here does not match on degenerate 'switch'.
+  assert(CaseCount > 0 && "Switch statement without any case found. This case "
+                          "should be excluded by the matcher and is handled "
+                          "separatly.");
+  std::size_t MaxPathsPossible = [&]() {
+    if (const auto *GeneralCondition =
+            Result.Nodes.getNodeAs<DeclRefExpr>("non-enum-condition"))
+      return getNumberOfPossibleValues(GeneralCondition->getType(),
+                                       *Result.Context);
+    if (const auto *BitfieldDecl =
+            Result.Nodes.getNodeAs<FieldDecl>("bitfield"))
+      return twoPow(BitfieldDecl->getBitWidthValue(*Result.Context));
+    llvm_unreachable("either bit-field or non-enum must be condition");
+  }();
+
+  // FIXME: Transform the 'switch' into an 'if' for CaseCount == 1.
+  if (CaseCount < MaxPathsPossible)
+    diag(Switch->getLocStart(),
+         CaseCount == 1 ? "switch with only one case; use an if statement"
+                        : "potential uncovered code path; add a default label");
+}
+} // namespace hicpp
+} // namespace tidy
+} // namespace clang

Added: clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.h?rev=318600&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/hicpp/MultiwayPathsCoveredCheck.h Sat Nov 18 11:48:33 2017
@@ -0,0 +1,51 @@
+//===--- MultiwayPathsCoveredCheck.h - clang-tidy----------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_HICPP_MULTIWAY_PATHS_COVERED_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_HICPP_MULTIWAY_PATHS_COVERED_H
+
+#include "../ClangTidy.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <iostream>
+
+namespace clang {
+namespace tidy {
+namespace hicpp {
+
+/// Find occasions where not all codepaths are explicitly covered in code.
+/// This includes 'switch' without a 'default'-branch and 'if'-'else if'-chains
+/// without a final 'else'-branch.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/hicpp-multiway-paths-covered.html
+class MultiwayPathsCoveredCheck : public ClangTidyCheck {
+public:
+  MultiwayPathsCoveredCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context),
+        WarnOnMissingElse(Options.get("WarnOnMissingElse", 0)) {}
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  void handleSwitchWithDefault(const SwitchStmt *Switch);
+  void handleSwitchWithoutDefault(
+      const SwitchStmt *Switch,
+      const ast_matchers::MatchFinder::MatchResult &Result);
+  /// This option can be configured to warn on missing 'else' branches in an
+  /// 'if-else if' chain. The default is false because this option might be
+  /// noisy on some code bases.
+  const bool WarnOnMissingElse;
+};
+
+} // namespace hicpp
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_HICPP_MULTIWAY_PATHS_COVERED_H

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=318600&r1=318599&r2=318600&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Sat Nov 18 11:48:33 2017
@@ -158,6 +158,11 @@ Improvements to clang-tidy
   Ensures that all exception will be instances of ``std::exception`` and classes 
   that are derived from it.
 
+- New `hicpp-multiway-paths-covered
+  <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-multiway-paths-covered.html>`_ check
+
+  Checks on ``switch`` and ``if`` - ``else if`` constructs that do not cover all possible code paths.
+
 - New `hicpp-signed-bitwise
   <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-signed-bitwise.html>`_ check
 

Added: clang-tools-extra/trunk/docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst?rev=318600&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst Sat Nov 18 11:48:33 2017
@@ -0,0 +1,95 @@
+.. title:: clang-tidy - hicpp-multiway-paths-covered
+
+hicpp-multiway-paths-covered
+============================
+
+This check discovers situations where code paths are not fully-covered.
+It furthermore suggests using ``if`` instead of ``switch`` if the code will be more clear.
+The `rule 6.1.2 <http://www.codingstandard.com/rule/6-1-2-explicitly-cover-all-paths-through-multi-way-selection-statements/>`_
+and `rule 6.1.4 <http://www.codingstandard.com/rule/6-1-4-ensure-that-a-switch-statement-has-at-least-two-case-labels-distinct-from-the-default-label/>`_
+of the High Integrity C++ Coding Standard are enforced.
+
+``if-else if`` chains that miss a final ``else`` branch might lead to unexpected 
+program execution and be the result of a logical error.
+If the missing ``else`` branch is intended you can leave it empty with a clarifying
+comment.
+This warning can be noisy on some code bases, so it is disabled by default.
+
+.. code-block:: c++
+
+  void f1() {
+    int i = determineTheNumber();
+
+     if(i > 0) { 
+       // Some Calculation 
+     } else if (i < 0) { 
+       // Precondition violated or something else. 
+     }
+     // ...
+  }
+
+Similar arguments hold for ``switch`` statements which do not cover all possible code paths.
+
+.. code-block:: c++
+
+  // The missing default branch might be a logical error. It can be kept empty
+  // if there is nothing to do, making it explicit.
+  void f2(int i) {
+    switch (i) {
+    case 0: // something
+      break;
+    case 1: // something else
+      break;
+    }
+    // All other numbers?
+  }
+
+  // Violates this rule as well, but already emits a compiler warning (-Wswitch).
+  enum Color { Red, Green, Blue, Yellow };
+  void f3(enum Color c) {
+    switch (c) {
+    case Red: // We can't drive for now.
+      break;
+    case Green:  // We are allowed to drive.
+      break;
+    }
+    // Other cases missing
+  }
+
+
+The `rule 6.1.4 <http://www.codingstandard.com/rule/6-1-4-ensure-that-a-switch-statement-has-at-least-two-case-labels-distinct-from-the-default-label/>`_
+requires every ``switch`` statement to have at least two ``case`` labels other than a `default` label.
+Otherwise, the ``switch`` could be better expressed with an ``if`` statement.
+Degenerated ``switch`` statements without any labels are caught as well.
+
+.. code-block:: c++
+
+  // Degenerated switch that could be better written as `if`
+  int i = 42;
+  switch(i) {
+    case 1: // do something here
+    default: // do somethe else here
+  }
+
+  // Should rather be the following:
+  if (i == 1) { 
+    // do something here 
+  }
+  else { 
+    // do something here 
+  }
+
+
+.. code-block:: c++
+  
+  // A completly degenerated switch will be diagnosed.
+  int i = 42;
+  switch(i) {}
+
+
+Options
+-------
+
+.. option:: WarnOnMissingElse
+
+  Activates warning for missing``else`` branches. Default is `0`.

Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=318600&r1=318599&r2=318600&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Sat Nov 18 11:48:33 2017
@@ -81,6 +81,7 @@ Clang-Tidy Checks
    hicpp-invalid-access-moved (redirects to misc-use-after-move) <hicpp-invalid-access-moved>
    hicpp-member-init (redirects to cppcoreguidelines-pro-type-member-init) <hicpp-member-init>
    hicpp-move-const-arg (redirects to misc-move-const-arg) <hicpp-move-const-arg>
+   hicpp-multiway-paths-covered
    hicpp-named-parameter (redirects to readability-named-parameter) <hicpp-named-parameter>
    hicpp-new-delete-operators (redirects to misc-new-delete-overloads) <hicpp-new-delete-operators>
    hicpp-no-array-decay (redirects to cppcoreguidelines-pro-bounds-array-to-pointer-decay) <hicpp-no-array-decay>

Added: clang-tools-extra/trunk/test/clang-tidy/hicpp-multiway-paths-covered-else.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/hicpp-multiway-paths-covered-else.cpp?rev=318600&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/hicpp-multiway-paths-covered-else.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/hicpp-multiway-paths-covered-else.cpp Sat Nov 18 11:48:33 2017
@@ -0,0 +1,57 @@
+// RUN: %check_clang_tidy %s hicpp-multiway-paths-covered %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: hicpp-multiway-paths-covered.WarnOnMissingElse, value: 1}]}'\
+// RUN: --
+
+enum OS { Mac,
+          Windows,
+          Linux };
+
+void problematic_if(int i, enum OS os) {
+  if (i > 0) {
+    return;
+  } else if (i < 0) {
+    // CHECK-MESSAGES: [[@LINE-1]]:10: warning: potentially uncovered codepath; add an ending else statement
+    return;
+  }
+
+  // Could be considered as false positive because all paths are covered logically.
+  // I still think this is valid since the possibility of a final 'everything else'
+  // codepath is expected from if-else if.
+  if (i > 0) {
+    return;
+  } else if (i <= 0) {
+    // CHECK-MESSAGES: [[@LINE-1]]:10: warning: potentially uncovered codepath; add an ending else statement
+    return;
+  }
+
+  // Test if nesting of if-else chains does get caught as well.
+  if (os == Mac) {
+    return;
+  } else if (os == Linux) {
+    // These checks are kind of degenerated, but the check will not try to solve
+    // if logically all paths are covered, which is more the area of the static analyzer.
+    if (true) {
+      return;
+    } else if (false) {
+      // CHECK-MESSAGES: [[@LINE-1]]:12: warning: potentially uncovered codepath; add an ending else statement
+      return;
+    }
+    return;
+  } else {
+    /* unreachable */
+    if (true) // check if the parent would match here as well
+      return;
+    // No warning for simple if statements, since it is common to just test one condition
+    // and ignore the opposite.
+  }
+
+  // Ok, because all paths are covered
+  if (i > 0) {
+    return;
+  } else if (i < 0) {
+    return;
+  } else {
+    /* error, maybe precondition failed */
+  }
+}

Added: clang-tools-extra/trunk/test/clang-tidy/hicpp-multiway-paths-covered.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/hicpp-multiway-paths-covered.cpp?rev=318600&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/hicpp-multiway-paths-covered.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/hicpp-multiway-paths-covered.cpp Sat Nov 18 11:48:33 2017
@@ -0,0 +1,467 @@
+// RUN: %check_clang_tidy %s hicpp-multiway-paths-covered %t
+
+enum OS { Mac,
+          Windows,
+          Linux };
+
+struct Bitfields {
+  unsigned UInt : 3;
+  int SInt : 1;
+};
+
+int return_integer() { return 42; }
+
+void bad_switch(int i) {
+  switch (i) {
+    // CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case 0:
+    break;
+  }
+  // No default in this switch
+  switch (i) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+    break;
+  case 1:
+    break;
+  case 2:
+    break;
+  }
+
+  // degenerate, maybe even warning
+  switch (i) {
+    // CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch without labels
+  }
+
+  switch (int j = return_integer()) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+  case 2:
+    break;
+  }
+
+  // Degenerated, only default case.
+  switch (i) {
+    // CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch with default label only
+  default:
+    break;
+  }
+
+  // Degenerated, only one case label and default case -> Better as if-stmt.
+  switch (i) {
+    // CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch could be better written as an if/else statement
+  case 0:
+    break;
+  default:
+    break;
+  }
+
+  unsigned long long BigNumber = 0;
+  switch (BigNumber) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+    break;
+  }
+
+  const int &IntRef = i;
+  switch (IntRef) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+    break;
+  }
+
+  char C = 'A';
+  switch (C) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 'A':
+    break;
+  case 'B':
+    break;
+  }
+
+  Bitfields Bf;
+  // UInt has 3 bits size.
+  switch (Bf.UInt) {
+    // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+    break;
+  }
+  // All paths explicitly covered.
+  switch (Bf.UInt) {
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+    break;
+  }
+  // SInt has 1 bit size, so this is somewhat degenerated.
+  switch (Bf.SInt) {
+    // CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case 0:
+    break;
+  }
+  // All paths explicitly covered.
+  switch (Bf.SInt) {
+  case 0:
+  case 1:
+    break;
+  }
+
+  bool Flag = false;
+  switch (Flag) {
+    // CHECK-MESSAGES:[[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case true:
+    break;
+  }
+
+  switch (Flag) {
+    // CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch with default label only
+  default:
+    break;
+  }
+
+  // This `switch` will create a frontend warning from '-Wswitch-bool' but is
+  // ok for this check.
+  switch (Flag) {
+  case true:
+    break;
+  case false:
+    break;
+  }
+}
+
+void unproblematic_switch(unsigned char c) {
+  switch (c) {
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+  case 8:
+  case 9:
+  case 10:
+  case 11:
+  case 12:
+  case 13:
+  case 14:
+  case 15:
+  case 16:
+  case 17:
+  case 18:
+  case 19:
+  case 20:
+  case 21:
+  case 22:
+  case 23:
+  case 24:
+  case 25:
+  case 26:
+  case 27:
+  case 28:
+  case 29:
+  case 30:
+  case 31:
+  case 32:
+  case 33:
+  case 34:
+  case 35:
+  case 36:
+  case 37:
+  case 38:
+  case 39:
+  case 40:
+  case 41:
+  case 42:
+  case 43:
+  case 44:
+  case 45:
+  case 46:
+  case 47:
+  case 48:
+  case 49:
+  case 50:
+  case 51:
+  case 52:
+  case 53:
+  case 54:
+  case 55:
+  case 56:
+  case 57:
+  case 58:
+  case 59:
+  case 60:
+  case 61:
+  case 62:
+  case 63:
+  case 64:
+  case 65:
+  case 66:
+  case 67:
+  case 68:
+  case 69:
+  case 70:
+  case 71:
+  case 72:
+  case 73:
+  case 74:
+  case 75:
+  case 76:
+  case 77:
+  case 78:
+  case 79:
+  case 80:
+  case 81:
+  case 82:
+  case 83:
+  case 84:
+  case 85:
+  case 86:
+  case 87:
+  case 88:
+  case 89:
+  case 90:
+  case 91:
+  case 92:
+  case 93:
+  case 94:
+  case 95:
+  case 96:
+  case 97:
+  case 98:
+  case 99:
+  case 100:
+  case 101:
+  case 102:
+  case 103:
+  case 104:
+  case 105:
+  case 106:
+  case 107:
+  case 108:
+  case 109:
+  case 110:
+  case 111:
+  case 112:
+  case 113:
+  case 114:
+  case 115:
+  case 116:
+  case 117:
+  case 118:
+  case 119:
+  case 120:
+  case 121:
+  case 122:
+  case 123:
+  case 124:
+  case 125:
+  case 126:
+  case 127:
+  case 128:
+  case 129:
+  case 130:
+  case 131:
+  case 132:
+  case 133:
+  case 134:
+  case 135:
+  case 136:
+  case 137:
+  case 138:
+  case 139:
+  case 140:
+  case 141:
+  case 142:
+  case 143:
+  case 144:
+  case 145:
+  case 146:
+  case 147:
+  case 148:
+  case 149:
+  case 150:
+  case 151:
+  case 152:
+  case 153:
+  case 154:
+  case 155:
+  case 156:
+  case 157:
+  case 158:
+  case 159:
+  case 160:
+  case 161:
+  case 162:
+  case 163:
+  case 164:
+  case 165:
+  case 166:
+  case 167:
+  case 168:
+  case 169:
+  case 170:
+  case 171:
+  case 172:
+  case 173:
+  case 174:
+  case 175:
+  case 176:
+  case 177:
+  case 178:
+  case 179:
+  case 180:
+  case 181:
+  case 182:
+  case 183:
+  case 184:
+  case 185:
+  case 186:
+  case 187:
+  case 188:
+  case 189:
+  case 190:
+  case 191:
+  case 192:
+  case 193:
+  case 194:
+  case 195:
+  case 196:
+  case 197:
+  case 198:
+  case 199:
+  case 200:
+  case 201:
+  case 202:
+  case 203:
+  case 204:
+  case 205:
+  case 206:
+  case 207:
+  case 208:
+  case 209:
+  case 210:
+  case 211:
+  case 212:
+  case 213:
+  case 214:
+  case 215:
+  case 216:
+  case 217:
+  case 218:
+  case 219:
+  case 220:
+  case 221:
+  case 222:
+  case 223:
+  case 224:
+  case 225:
+  case 226:
+  case 227:
+  case 228:
+  case 229:
+  case 230:
+  case 231:
+  case 232:
+  case 233:
+  case 234:
+  case 235:
+  case 236:
+  case 237:
+  case 238:
+  case 239:
+  case 240:
+  case 241:
+  case 242:
+  case 243:
+  case 244:
+  case 245:
+  case 246:
+  case 247:
+  case 248:
+  case 249:
+  case 250:
+  case 251:
+  case 252:
+  case 253:
+  case 254:
+  case 255:
+    break;
+  }
+
+  // Some paths are covered by the switch and a default case is present.
+  switch (c) {
+  case 1:
+  case 2:
+  case 3:
+  default:
+    break;
+  }
+}
+
+OS return_enumerator() {
+  return Linux;
+}
+
+// Enumpaths are already covered by a warning, this is just to ensure, that there is
+// no interference or false positives.
+// -Wswitch warns about uncovered enum paths and each here described case is already
+// covered.
+void switch_enums(OS os) {
+  switch (os) {
+  case Linux:
+    break;
+  }
+
+  switch (OS another_os = return_enumerator()) {
+  case Linux:
+    break;
+  }
+
+  switch (os) {
+  }
+}
+
+/// All of these cases will not emit a warning per default, but with explicit activation.
+/// Covered in extra test file.
+void problematic_if(int i, enum OS os) {
+  if (i > 0) {
+    return;
+  } else if (i < 0) {
+    return;
+  }
+
+  if (os == Mac) {
+    return;
+  } else if (os == Linux) {
+    if (true) {
+      return;
+    } else if (false) {
+      return;
+    }
+    return;
+  } else {
+    /* unreachable */
+    if (true) // check if the parent would match here as well
+      return;
+  }
+
+  // Ok, because all paths are covered
+  if (i > 0) {
+    return;
+  } else if (i < 0) {
+    return;
+  } else {
+    /* error, maybe precondition failed */
+  }
+}




More information about the cfe-commits mailing list