[clang-tidy] Fix infinite loop in MisplacedWideningCastCheck.

[etienneb]

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.

[alexfh]

Formatting is broken. clang-format -style=file, please.

[etienneb]

alexfh comments.

[alexfh]

LG

[etienneb]

nits, lower case names.

[alexfh]

Missed a couple of nits.

clang-tidy/misc/MisplacedWideningCastCheck.cpp

98–130

s/RelativeIntSizes/relativeIntSize/
s/kind/Kind/

[etienneb]

fix nits.

clang-tidy/misc/MisplacedWideningCastCheck.cpp

 //===--- MisplacedWideningCastCheck.cpp - clang-tidy-----------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "MisplacedWideningCastCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "llvm/ADT/DenseMap.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace misc {
 
 MisplacedWideningCastCheck::MisplacedWideningCastCheck(
     StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       CheckImplicitCasts(Options.get("CheckImplicitCasts", true)) {}
 
 void MisplacedWideningCastCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "CheckImplicitCasts", CheckImplicitCasts);
 }
 
 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("-"),
 
-  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);
   Finder->addMatcher(callExpr(hasAnyArgument(Cast)), this);
   Finder->addMatcher(binaryOperator(hasOperatorName("="), hasRHS(Cast)), this);
   Finder->addMatcher(
       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)
if (const auto *Bop = dyn_cast<BinaryOperator>(E)) {
     return T->isIntegerType() ? Context.getIntWidth(T) : 1024U;
   } else if (const auto *I = dyn_cast<IntegerLiteral>(E)) {
     return I->getValue().getActiveBits();
   }
 
   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 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) {
-  static const llvm::SmallDenseMap<int, int> RelativeIntSizes(
-      createRelativeIntSizesMap());
-  static const llvm::SmallDenseMap<int, int> RelativeCharSizes(
-      createRelativeCharSizesMap());
-  static const llvm::SmallDenseMap<int, int> RelativeCharSizesW(
-      createRelativeCharSizesWMap());
-
   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;