[clang-tidy] improve misc-misplaced-widening-cast so it also detects portability problems

[danielmarjamaki]

Summary

The misc-misplaced-widening-cast currently only looks at the size to determine if a cast is widening.

With this patch it will also warn when there are portability problems.

The test file should now be target independant.

[alexfh]

LG

clang-tidy/misc/MisplacedWideningCastCheck.cpp

auto Calc = expr(anyOf(binaryOperator(anyOf(
                              hasOperatorName("+"), hasOperatorName("-"),
                              hasOperatorName("*"), hasOperatorName("<<"))),
                          unaryOperator(hasOperatorName("~"))),
                    hasType(isInteger()))
                   .bind("Calc");
 
   auto Cast = explicitCastExpr(anyOf(cStyleCastExpr(), cxxStaticCastExpr(),
                                      cxxReinterpretCastExpr()),
-                               hasDestinationType(isInteger()),
-                               has(Calc))
+                               hasDestinationType(isInteger()), has(Calc))
                   .bind("Cast");
 
   Finder->addMatcher(varDecl(has(Cast)), this);
   Finder->addMatcher(returnStmt(has(Cast)), this);
   Finder->addMatcher(binaryOperator(hasOperatorName("="), hasRHS(Cast)), this);
 }
 
 static unsigned getMaxCalculationWidth(ASTContext &Context, const Expr *E) {
void MisplacedWideningCastCheck::check(const MatchFinder::MatchResult &Result) {
   if (Calc->getLocStart().isMacroID())
     return;
 
   ASTContext &Context = *Result.Context;
 
   QualType CastType = Cast->getType();
   QualType CalcType = Calc->getType();
 
-  if (Context.getIntWidth(CastType) <= Context.getIntWidth(CalcType))
+  // Explicit truncation using cast.
+  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)) {
+    if (CalcType->isSpecificBuiltinType(BuiltinType::Int)) {
+      // There should be a warning when casting from int to long or long long.
+      if (!CastType->isSpecificBuiltinType(BuiltinType::Long) &&
+          !CastType->isSpecificBuiltinType(BuiltinType::LongLong))
+        return;
+    } else if (CalcType->isSpecificBuiltinType(BuiltinType::Long)) {
+      // There should be a warning when casting from long to long long.
+      if (!CastType->isSpecificBuiltinType(BuiltinType::LongLong))
+        return;
+    } else {
+      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")
       << CalcType << CastType;
 }
 
 } // namespace misc
 } // namespace tidy
 } // namespace clang

test/clang-tidy/misc-misplaced-widening-cast.cpp

-// RUN: %check_clang_tidy %s misc-misplaced-widening-cast %t -- -- -target x86_64-unknown-unknown
+// RUN: %check_clang_tidy %s misc-misplaced-widening-cast %t
 
 void assign(int a, int b) {
   long l;
 
   l = a * b;
   l = (long)(a * b);
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' is ineffective, or there is loss of precision before the conversion [misc-misplaced-widening-cast]
   l = (long)a * b;