Hi, We are seeing a bot failure with this commit. Please see the bot page here: http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/5470/consoleFull#17462642768254eaf0-7326-4999-85b0-388101f2d404
Please let me know if you will be able to provide a patch for this in the next couple of hours, otherwise I will need to revert your commit. Thank you for your assistance in keeping our bots green. Respectfully, Mike Edwards On Sat, Nov 18, 2017 at 11:48 AM, Jonas Toth via cfe-commits < cfe-commits@lists.llvm.org> wrote: > 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 */ > + } > +} > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
