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------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
#include "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(
switchStmt(
hasCondition(expr(
// 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(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 than 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->getBeginLoc(),
"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.
// FIXME: Evaluate, if emitting a fix-it to simplify that statement is
// reasonable.
if (!SwitchHasDefault && SwitchCaseCount == 0) {
diag(Switch->getBeginLoc(),
"switch statement without labels has no effect");
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->getBeginLoc(),
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 static_cast<std::size_t>(0);
}();
// FIXME: Transform the 'switch' into an 'if' for CaseCount == 1.
if (CaseCount < MaxPathsPossible)
diag(Switch->getBeginLoc(),
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
|
