1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
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
|
