JonasToth updated this revision to Diff 138850.
JonasToth added a comment.
- reorder check in release notes to get it in alphabetical order
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D40737
Files:
clang-tidy/hicpp/CMakeLists.txt
clang-tidy/hicpp/HICPPTidyModule.cpp
clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
clang-tidy/hicpp/MultiwayPathsCoveredCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/hicpp-multiway-paths-covered-else.cpp
test/clang-tidy/hicpp-multiway-paths-covered.cpp
Index: test/clang-tidy/hicpp-multiway-paths-covered.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/hicpp-multiway-paths-covered.cpp
@@ -0,0 +1,468 @@
+// 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 */
+ }
+}
Index: test/clang-tidy/hicpp-multiway-paths-covered-else.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/hicpp-multiway-paths-covered-else.cpp
@@ -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 */
+ }
+}
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -121,6 +121,7 @@
hicpp-invalid-access-moved (redirects to bugprone-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 performance-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>
Index: docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst
@@ -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`.
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -102,6 +102,11 @@
using ``decltype`` specifiers and lambda with otherwise unutterable
return types.
+- 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 `modernize-use-uncaught-exceptions
<http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-uncaught-exceptions.html>`_ check
Index: clang-tidy/hicpp/MultiwayPathsCoveredCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/hicpp/MultiwayPathsCoveredCheck.h
@@ -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, std::size_t CaseCount);
+ void handleSwitchWithoutDefault(
+ const SwitchStmt *Switch, std::size_t CaseCount,
+ 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
Index: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
@@ -0,0 +1,177 @@
+//===--- 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 <iostream>
+#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(
+ 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")))),
+ ignoringImpCasts(declRefExpr().bind("non-enum-condition"))),
+ // 'unless()' must be the last match here and must be bound,
+ // otherwise the matcher does not work correctly, because it
+ // will not explicitly ignore enum conditions.
+ unless(ignoringImpCasts(
+ declRefExpr(hasType(enumType())).bind("enum-condition"))))))
+ .bind("switch"),
+ 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 std::pair<std::size_t, bool> countCaseLabels(const SwitchStmt *Switch) {
+ std::size_t CaseCount = 0;
+ bool HasDefault = false;
+
+ const SwitchCase *CurrentCase = Switch->getSwitchCaseList();
+ while (CurrentCase) {
+ ++CaseCount;
+ if (isa<DefaultStmt>(CurrentCase))
+ HasDefault = true;
+
+ CurrentCase = CurrentCase->getNextSwitchCase();
+ }
+
+ return std::make_pair(CaseCount, HasDefault);
+}
+/// 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;
+ }
+ const auto *Switch = Result.Nodes.getNodeAs<SwitchStmt>("switch");
+ std::size_t SwitchCaseCount;
+ bool SwitchHasDefault;
+ std::tie(SwitchCaseCount, SwitchHasDefault) = countCaseLabels(Switch);
+
+ // 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 (SwitchHasDefault) {
+ handleSwitchWithDefault(Switch, SwitchCaseCount);
+ return;
+ }
+ // Checks all 'switch' statements that do not define a default label.
+ // Here the heavy lifting happens.
+ if (!SwitchHasDefault && SwitchCaseCount > 0) {
+ handleSwitchWithoutDefault(Switch, SwitchCaseCount, Result);
+ return;
+ }
+ // Warns for degenerated 'switch' statements that neither define a case nor
+ // a default label.
+ if (!SwitchHasDefault && SwitchCaseCount == 0) {
+ diag(Switch->getLocStart(), "degenerated switch without labels");
+ return;
+ }
+ llvm_unreachable("matched a case, that was not explicitly handled");
+}
+
+void MultiwayPathsCoveredCheck::handleSwitchWithDefault(
+ const SwitchStmt *Switch, std::size_t CaseCount) {
+ 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, std::size_t CaseCount,
+ 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");
+ // Determine the number of case labels. Because '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.
+ // 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));
+ }
+
+ return 0ul;
+ }();
+
+ // 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
Index: clang-tidy/hicpp/HICPPTidyModule.cpp
===================================================================
--- clang-tidy/hicpp/HICPPTidyModule.cpp
+++ clang-tidy/hicpp/HICPPTidyModule.cpp
@@ -36,6 +36,7 @@
#include "../readability/FunctionSizeCheck.h"
#include "../readability/IdentifierNamingCheck.h"
#include "ExceptionBaseclassCheck.h"
+#include "MultiwayPathsCoveredCheck.h"
#include "NoAssemblerCheck.h"
#include "SignedBitwiseCheck.h"
@@ -54,6 +55,8 @@
"hicpp-deprecated-headers");
CheckFactories.registerCheck<ExceptionBaseclassCheck>(
"hicpp-exception-baseclass");
+ CheckFactories.registerCheck<MultiwayPathsCoveredCheck>(
+ "hicpp-multiway-paths-covered");
CheckFactories.registerCheck<SignedBitwiseCheck>("hicpp-signed-bitwise");
CheckFactories.registerCheck<google::ExplicitConstructorCheck>(
"hicpp-explicit-conversions");
Index: clang-tidy/hicpp/CMakeLists.txt
===================================================================
--- clang-tidy/hicpp/CMakeLists.txt
+++ clang-tidy/hicpp/CMakeLists.txt
@@ -2,6 +2,7 @@
add_clang_library(clangTidyHICPPModule
ExceptionBaseclassCheck.cpp
+ MultiwayPathsCoveredCheck.cpp
NoAssemblerCheck.cpp
HICPPTidyModule.cpp
SignedBitwiseCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
