Skip to content

Commit 53f7c0e

Browse files
committedApr 7, 2016
[clang-tidy] Fix infinite loop in MisplacedWideningCastCheck.
Summary: In Release mode, the check was infinite looping over chromium code base. It seems there is something strange with the creation of the Maps. I believe the compiler is making some assumption with the implicit conversion from enum <-> int. By moving the map to a standard switch/cases, we no longer allocate memory and we can keep the same behavior. For a small amount of elements, this is fine. Reviewers: alexfh Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D18833 llvm-svn: 265679
1 parent d37630e commit 53f7c0e

File tree

1 file changed

+87
-61
lines changed

1 file changed

+87
-61
lines changed
 

‎clang-tools-extra/clang-tidy/misc/MisplacedWideningCastCheck.cpp

+87-61
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
#include "MisplacedWideningCastCheck.h"
1111
#include "clang/AST/ASTContext.h"
1212
#include "clang/ASTMatchers/ASTMatchFinder.h"
13-
#include "llvm/ADT/DenseMap.h"
1413

1514
using namespace clang::ast_matchers;
1615

@@ -29,18 +28,19 @@ void MisplacedWideningCastCheck::storeOptions(
2928
}
3029

3130
void MisplacedWideningCastCheck::registerMatchers(MatchFinder *Finder) {
32-
auto Calc = expr(anyOf(binaryOperator(anyOf(
33-
hasOperatorName("+"), hasOperatorName("-"),
34-
hasOperatorName("*"), hasOperatorName("<<"))),
35-
unaryOperator(hasOperatorName("~"))),
36-
hasType(isInteger()))
37-
.bind("Calc");
38-
39-
auto ExplicitCast =
31+
const auto Calc =
32+
expr(anyOf(binaryOperator(
33+
anyOf(hasOperatorName("+"), hasOperatorName("-"),
34+
hasOperatorName("*"), hasOperatorName("<<"))),
35+
unaryOperator(hasOperatorName("~"))),
36+
hasType(isInteger()))
37+
.bind("Calc");
38+
39+
const auto ExplicitCast =
4040
explicitCastExpr(hasDestinationType(isInteger()), has(Calc));
41-
auto ImplicitCast =
41+
const auto ImplicitCast =
4242
implicitCastExpr(hasImplicitDestinationType(isInteger()), has(Calc));
43-
auto Cast = expr(anyOf(ExplicitCast, ImplicitCast)).bind("Cast");
43+
const auto Cast = expr(anyOf(ExplicitCast, ImplicitCast)).bind("Cast");
4444

4545
Finder->addMatcher(varDecl(hasInitializer(Cast)), this);
4646
Finder->addMatcher(returnStmt(hasReturnValue(Cast)), this);
@@ -50,11 +50,12 @@ void MisplacedWideningCastCheck::registerMatchers(MatchFinder *Finder) {
5050
binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!="),
5151
hasOperatorName("<"), hasOperatorName("<="),
5252
hasOperatorName(">"), hasOperatorName(">=")),
53-
anyOf(hasLHS(Cast), hasRHS(Cast))),
53+
hasEitherOperand(Cast)),
5454
this);
5555
}
5656

57-
static unsigned getMaxCalculationWidth(ASTContext &Context, const Expr *E) {
57+
static unsigned getMaxCalculationWidth(const ASTContext &Context,
58+
const Expr *E) {
5859
E = E->IgnoreParenImpCasts();
5960

6061
if (const auto *Bop = dyn_cast<BinaryOperator>(E)) {
@@ -94,64 +95,89 @@ static unsigned getMaxCalculationWidth(ASTContext &Context, const Expr *E) {
9495
return Context.getIntWidth(E->getType());
9596
}
9697

97-
static llvm::SmallDenseMap<int, int> createRelativeIntSizesMap() {
98-
llvm::SmallDenseMap<int, int> Result;
99-
Result[BuiltinType::UChar] = 1;
100-
Result[BuiltinType::SChar] = 1;
101-
Result[BuiltinType::Char_U] = 1;
102-
Result[BuiltinType::Char_S] = 1;
103-
Result[BuiltinType::UShort] = 2;
104-
Result[BuiltinType::Short] = 2;
105-
Result[BuiltinType::UInt] = 3;
106-
Result[BuiltinType::Int] = 3;
107-
Result[BuiltinType::ULong] = 4;
108-
Result[BuiltinType::Long] = 4;
109-
Result[BuiltinType::ULongLong] = 5;
110-
Result[BuiltinType::LongLong] = 5;
111-
Result[BuiltinType::UInt128] = 6;
112-
Result[BuiltinType::Int128] = 6;
113-
return Result;
98+
static int relativeIntSizes(BuiltinType::Kind Kind) {
99+
switch (Kind) {
100+
case BuiltinType::UChar:
101+
return 1;
102+
case BuiltinType::SChar:
103+
return 1;
104+
case BuiltinType::Char_U:
105+
return 1;
106+
case BuiltinType::Char_S:
107+
return 1;
108+
case BuiltinType::UShort:
109+
return 2;
110+
case BuiltinType::Short:
111+
return 2;
112+
case BuiltinType::UInt:
113+
return 3;
114+
case BuiltinType::Int:
115+
return 3;
116+
case BuiltinType::ULong:
117+
return 4;
118+
case BuiltinType::Long:
119+
return 4;
120+
case BuiltinType::ULongLong:
121+
return 5;
122+
case BuiltinType::LongLong:
123+
return 5;
124+
case BuiltinType::UInt128:
125+
return 6;
126+
case BuiltinType::Int128:
127+
return 6;
128+
default:
129+
return 0;
130+
}
114131
}
115132

116-
static llvm::SmallDenseMap<int, int> createRelativeCharSizesMap() {
117-
llvm::SmallDenseMap<int, int> Result;
118-
Result[BuiltinType::UChar] = 1;
119-
Result[BuiltinType::SChar] = 1;
120-
Result[BuiltinType::Char_U] = 1;
121-
Result[BuiltinType::Char_S] = 1;
122-
Result[BuiltinType::Char16] = 2;
123-
Result[BuiltinType::Char32] = 3;
124-
return Result;
133+
static int relativeCharSizes(BuiltinType::Kind Kind) {
134+
switch (Kind) {
135+
case BuiltinType::UChar:
136+
return 1;
137+
case BuiltinType::SChar:
138+
return 1;
139+
case BuiltinType::Char_U:
140+
return 1;
141+
case BuiltinType::Char_S:
142+
return 1;
143+
case BuiltinType::Char16:
144+
return 2;
145+
case BuiltinType::Char32:
146+
return 3;
147+
default:
148+
return 0;
149+
}
125150
}
126151

127-
static llvm::SmallDenseMap<int, int> createRelativeCharSizesWMap() {
128-
llvm::SmallDenseMap<int, int> Result;
129-
Result[BuiltinType::UChar] = 1;
130-
Result[BuiltinType::SChar] = 1;
131-
Result[BuiltinType::Char_U] = 1;
132-
Result[BuiltinType::Char_S] = 1;
133-
Result[BuiltinType::WChar_U] = 2;
134-
Result[BuiltinType::WChar_S] = 2;
135-
return Result;
152+
static int relativeCharSizesW(BuiltinType::Kind Kind) {
153+
switch (Kind) {
154+
case BuiltinType::UChar:
155+
return 1;
156+
case BuiltinType::SChar:
157+
return 1;
158+
case BuiltinType::Char_U:
159+
return 1;
160+
case BuiltinType::Char_S:
161+
return 1;
162+
case BuiltinType::WChar_U:
163+
return 2;
164+
case BuiltinType::WChar_S:
165+
return 2;
166+
default:
167+
return 0;
168+
}
136169
}
137170

138171
static bool isFirstWider(BuiltinType::Kind First, BuiltinType::Kind Second) {
139-
static const llvm::SmallDenseMap<int, int> RelativeIntSizes(
140-
createRelativeIntSizesMap());
141-
static const llvm::SmallDenseMap<int, int> RelativeCharSizes(
142-
createRelativeCharSizesMap());
143-
static const llvm::SmallDenseMap<int, int> RelativeCharSizesW(
144-
createRelativeCharSizesWMap());
145-
146172
int FirstSize, SecondSize;
147-
if ((FirstSize = RelativeIntSizes.lookup(First)) &&
148-
(SecondSize = RelativeIntSizes.lookup(Second)))
173+
if ((FirstSize = relativeIntSizes(First)) != 0 &&
174+
(SecondSize = relativeIntSizes(Second)) != 0)
149175
return FirstSize > SecondSize;
150-
if ((FirstSize = RelativeCharSizes.lookup(First)) &&
151-
(SecondSize = RelativeCharSizes.lookup(Second)))
176+
if ((FirstSize = relativeCharSizes(First)) != 0 &&
177+
(SecondSize = relativeCharSizes(Second)) != 0)
152178
return FirstSize > SecondSize;
153-
if ((FirstSize = RelativeCharSizesW.lookup(First)) &&
154-
(SecondSize = RelativeCharSizesW.lookup(Second)))
179+
if ((FirstSize = relativeCharSizesW(First)) != 0 &&
180+
(SecondSize = relativeCharSizesW(Second)) != 0)
155181
return FirstSize > SecondSize;
156182
return false;
157183
}

0 commit comments

Comments
 (0)
Please sign in to comment.