etienneb updated this revision to Diff 52856. etienneb added a comment. rebased.
http://reviews.llvm.org/D18783 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; - 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; - 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; - 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 cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits