etienneb created this revision.
etienneb added a reviewer: alexfh.
etienneb added a subscriber: cfe-commits.
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.
http://reviews.llvm.org/D18833
Files:
clang-tidy/misc/MisplacedWideningCastCheck.cpp
Index: clang-tidy/misc/MisplacedWideningCastCheck.cpp
===================================================================
--- clang-tidy/misc/MisplacedWideningCastCheck.cpp
+++ clang-tidy/misc/MisplacedWideningCastCheck.cpp
@@ -10,7 +10,6 @@
#include "MisplacedWideningCastCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "llvm/ADT/DenseMap.h"
using namespace clang::ast_matchers;
@@ -29,18 +28,19 @@
}
void MisplacedWideningCastCheck::registerMatchers(MatchFinder *Finder) {
- auto Calc = expr(anyOf(binaryOperator(anyOf(
- hasOperatorName("+"), hasOperatorName("-"),
- hasOperatorName("*"), hasOperatorName("<<"))),
- unaryOperator(hasOperatorName("~"))),
- hasType(isInteger()))
- .bind("Calc");
+ const auto Calc =
+ expr(anyOf(binaryOperator(
+ anyOf(hasOperatorName("+"), hasOperatorName("-"),
+ hasOperatorName("*"), hasOperatorName("<<"))),
+ unaryOperator(hasOperatorName("~"))),
+ hasType(isInteger()))
+ .bind("Calc");
- auto ExplicitCast =
+ const auto ExplicitCast =
explicitCastExpr(hasDestinationType(isInteger()), has(Calc));
- auto ImplicitCast =
+ const auto ImplicitCast =
implicitCastExpr(hasImplicitDestinationType(isInteger()), has(Calc));
- auto Cast = expr(anyOf(ExplicitCast, ImplicitCast)).bind("Cast");
+ const auto Cast = expr(anyOf(ExplicitCast, ImplicitCast)).bind("Cast");
Finder->addMatcher(varDecl(hasInitializer(Cast)), this);
Finder->addMatcher(returnStmt(hasReturnValue(Cast)), this);
@@ -50,20 +50,19 @@
binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!="),
hasOperatorName("<"), hasOperatorName("<="),
hasOperatorName(">"), hasOperatorName(">=")),
- anyOf(hasLHS(Cast), hasRHS(Cast))),
+ hasEitherOperand(Cast)),
this);
}
-static unsigned getMaxCalculationWidth(ASTContext &Context, const Expr *E) {
+static unsigned getMaxCalculationWidth(const ASTContext &Context,
+ const Expr *E) {
E = E->IgnoreParenImpCasts();
if (const auto *Bop = dyn_cast<BinaryOperator>(E)) {
unsigned LHSWidth = getMaxCalculationWidth(Context, Bop->getLHS());
unsigned RHSWidth = getMaxCalculationWidth(Context, Bop->getRHS());
- if (Bop->getOpcode() == BO_Mul)
- return LHSWidth + RHSWidth;
- if (Bop->getOpcode() == BO_Add)
- return std::max(LHSWidth, RHSWidth) + 1;
+ if (Bop->getOpcode() == BO_Mul) return LHSWidth + RHSWidth;
+ if (Bop->getOpcode() == BO_Add) return std::max(LHSWidth, RHSWidth) + 1;
if (Bop->getOpcode() == BO_Rem) {
llvm::APSInt Val;
if (Bop->getRHS()->EvaluateAsInt(Val, Context))
@@ -82,8 +81,7 @@
}
} else if (const auto *Uop = dyn_cast<UnaryOperator>(E)) {
// There is truncation when ~ is used.
- if (Uop->getOpcode() == UO_Not)
- return 1024U;
+ if (Uop->getOpcode() == UO_Not) return 1024U;
QualType T = Uop->getType();
return T->isIntegerType() ? Context.getIntWidth(T) : 1024U;
@@ -94,111 +92,134 @@
return Context.getIntWidth(E->getType());
}
-static llvm::SmallDenseMap<int, int> createRelativeIntSizesMap() {
- llvm::SmallDenseMap<int, int> Result(14);
- Result[BuiltinType::UChar] = 1;
- Result[BuiltinType::SChar] = 1;
- Result[BuiltinType::Char_U] = 1;
- Result[BuiltinType::Char_S] = 1;
- Result[BuiltinType::UShort] = 2;
- Result[BuiltinType::Short] = 2;
- Result[BuiltinType::UInt] = 3;
- Result[BuiltinType::Int] = 3;
- Result[BuiltinType::ULong] = 4;
- Result[BuiltinType::Long] = 4;
- Result[BuiltinType::ULongLong] = 5;
- Result[BuiltinType::LongLong] = 5;
- Result[BuiltinType::UInt128] = 6;
- Result[BuiltinType::Int128] = 6;
- return Result;
-}
-
-static llvm::SmallDenseMap<int, int> createRelativeCharSizesMap() {
- llvm::SmallDenseMap<int, int> Result(6);
- Result[BuiltinType::UChar] = 1;
- Result[BuiltinType::SChar] = 1;
- Result[BuiltinType::Char_U] = 1;
- Result[BuiltinType::Char_S] = 1;
- Result[BuiltinType::Char16] = 2;
- Result[BuiltinType::Char32] = 3;
- return Result;
-}
-
-static llvm::SmallDenseMap<int, int> createRelativeCharSizesWMap() {
- llvm::SmallDenseMap<int, int> Result(6);
- Result[BuiltinType::UChar] = 1;
- Result[BuiltinType::SChar] = 1;
- Result[BuiltinType::Char_U] = 1;
- Result[BuiltinType::Char_S] = 1;
- Result[BuiltinType::WChar_U] = 2;
- Result[BuiltinType::WChar_S] = 2;
- return Result;
+static int RelativeIntSizes(BuiltinType::Kind kind) {
+ switch (kind) {
+ case BuiltinType::UChar:
+ return 1;
+ case BuiltinType::SChar:
+ return 1;
+ case BuiltinType::Char_U:
+ return 1;
+ case BuiltinType::Char_S:
+ return 1;
+ case BuiltinType::UShort:
+ return 2;
+ case BuiltinType::Short:
+ return 2;
+ case BuiltinType::UInt:
+ return 3;
+ case BuiltinType::Int:
+ return 3;
+ case BuiltinType::ULong:
+ return 4;
+ case BuiltinType::Long:
+ return 4;
+ case BuiltinType::ULongLong:
+ return 5;
+ case BuiltinType::LongLong:
+ return 5;
+ case BuiltinType::UInt128:
+ return 6;
+ case BuiltinType::Int128:
+ return 6;
+ default:
+ return 0;
+ }
}
-static bool isFirstWider(BuiltinType::Kind First, BuiltinType::Kind Second) {
- static const llvm::SmallDenseMap<int, int> RelativeIntSizes(
- createRelativeIntSizesMap());
- static const llvm::SmallDenseMap<int, int> RelativeCharSizes(
- createRelativeCharSizesMap());
- static const llvm::SmallDenseMap<int, int> RelativeCharSizesW(
- createRelativeCharSizesWMap());
+static int RelativeCharSizes(BuiltinType::Kind kind) {
+ switch (kind) {
+ case BuiltinType::UChar:
+ return 1;
+ case BuiltinType::SChar:
+ return 1;
+ case BuiltinType::Char_U:
+ return 1;
+ case BuiltinType::Char_S:
+ return 1;
+ case BuiltinType::Char16:
+ return 2;
+ case BuiltinType::Char32:
+ return 3;
+ default:
+ return 0;
+ }
+}
+
+static int RelativeCharSizesW(BuiltinType::Kind kind) {
+ switch (kind) {
+ case BuiltinType::UChar:
+ return 1;
+ case BuiltinType::SChar:
+ return 1;
+ case BuiltinType::Char_U:
+ return 1;
+ case BuiltinType::Char_S:
+ return 1;
+ case BuiltinType::WChar_U:
+ return 2;
+ case BuiltinType::WChar_S:
+ return 2;
+ default:
+ return 0;
+ }
+}
+static bool isFirstWider(BuiltinType::Kind First, BuiltinType::Kind Second) {
int FirstSize, SecondSize;
- if ((FirstSize = RelativeIntSizes.lookup(First)) &&
- (SecondSize = RelativeIntSizes.lookup(Second)))
+ if ((FirstSize = RelativeIntSizes(First)) != 0 &&
+ (SecondSize = RelativeIntSizes(Second)) != 0)
return FirstSize > SecondSize;
- if ((FirstSize = RelativeCharSizes.lookup(First)) &&
- (SecondSize = RelativeCharSizes.lookup(Second)))
+ if ((FirstSize = RelativeCharSizes(First)) != 0 &&
+ (SecondSize = RelativeCharSizes(Second)) != 0)
return FirstSize > SecondSize;
- if ((FirstSize = RelativeCharSizesW.lookup(First)) &&
- (SecondSize = RelativeCharSizesW.lookup(Second)))
+ if ((FirstSize = RelativeCharSizesW(First)) != 0 &&
+ (SecondSize = RelativeCharSizesW(Second)) != 0)
return FirstSize > SecondSize;
+
return false;
}
void MisplacedWideningCastCheck::check(const MatchFinder::MatchResult &Result) {
const auto *Cast = Result.Nodes.getNodeAs<CastExpr>("Cast");
- if (!CheckImplicitCasts && isa<ImplicitCastExpr>(Cast))
- return;
- if (Cast->getLocStart().isMacroID())
- return;
+ if (!CheckImplicitCasts && isa<ImplicitCastExpr>(Cast)) return;
+ if (Cast->getLocStart().isMacroID()) return;
const auto *Calc = Result.Nodes.getNodeAs<Expr>("Calc");
- if (Calc->getLocStart().isMacroID())
- return;
+ if (Calc->getLocStart().isMacroID()) return;
- ASTContext &Context = *Result.Context;
+ const ASTContext &Context = *Result.Context;
QualType CastType = Cast->getType();
QualType CalcType = Calc->getType();
// Explicit truncation using cast.
- if (Context.getIntWidth(CastType) < Context.getIntWidth(CalcType))
- return;
+ if (Context.getIntWidth(CastType) < Context.getIntWidth(CalcType)) return;
// If CalcType and CastType have same size then there is no real danger, but
// there can be a portability problem.
-
if (Context.getIntWidth(CastType) == Context.getIntWidth(CalcType)) {
const auto *CastBuiltinType =
dyn_cast<BuiltinType>(CastType->getUnqualifiedDesugaredType());
const auto *CalcBuiltinType =
dyn_cast<BuiltinType>(CalcType->getUnqualifiedDesugaredType());
if (CastBuiltinType && CalcBuiltinType &&
- !isFirstWider(CastBuiltinType->getKind(), CalcBuiltinType->getKind()))
+ !isFirstWider(CastBuiltinType->getKind(), CalcBuiltinType->getKind())) {
return;
+ }
}
// Don't write a warning if we can easily see that the result is not
// truncated.
if (Context.getIntWidth(CalcType) >= getMaxCalculationWidth(Context, Calc))
return;
- diag(Cast->getLocStart(), "either cast from %0 to %1 is ineffective, or "
- "there is loss of precision before the conversion")
+ diag(Cast->getLocStart(),
+ "either cast from %0 to %1 is ineffective, or "
+ "there is loss of precision before the conversion")
<< CalcType << CastType;
}
-} // namespace misc
-} // namespace tidy
-} // namespace clang
+} // namespace misc
+} // namespace tidy
+} // namespace clang
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
