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

Reply via email to