[clang-tidy] Fix cppcoreguidelines-narrowing-conversions false positive

[njames93]

Summary

Fix https://llvm.org/PR49498.
The check notices 2 sides of a conditional operator have types with a different constness and so tries to examine the implicit cast.
As one side is infinity, the float narrowing detection sees when its casted to a double(which it already was) it thinks the result is out of range.

I've fixed this by just disregarding expressions where the builtin type(without quals) match as no conversion would take place.

However this has opened a can of worms. Currenty float a = std::numeric_limits<double>::infinity(); is marked as narrowing.
Whats more suspicious is double a = std::numeric_limits<float>::infinity(); is also marked as narrowing.
It could be argued double inf -> float inf is narrowing, but float inf -> double inf definitely isnt.

[aaron.ballman]

However this has opened a can of worms. Currenty float a = std::numeric_limits<double>::infinity(); is marked as narrowing.

I think this is technically not narrowing, per http://eel.is/c++draft/dcl.init.list#7.2, because the source is a constant expression (at least, after C++11) and the value can be represented after conversion. However, I wonder if the same is true for the various NaNs as it is for infinity?

Whats more suspicious is double a = std::numeric_limits<float>::infinity(); is also marked as narrowing.

This is definitely not narrowing.

[carlosgalvezp]

LGTM, is there anything that should be fixed before merging?

[carlosgalvezp]

Took the liberty to land it for you, hope it's ok! :)

clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp

void NarrowingConversionsCheck::handleBinaryOperator(const ASTContext &Context,
                                                      const Expr &Lhs,
                                                      const Expr &Rhs) {
   assert(!Lhs.isInstantiationDependent() && !Rhs.isInstantiationDependent() &&
          "Dependent types must be check before calling this function");
   const BuiltinType *LhsType = getBuiltinType(Lhs);
   const BuiltinType *RhsType = getBuiltinType(Rhs);
   if (RhsType == nullptr || LhsType == nullptr)
     return;
+  if (LhsType == RhsType)
+    return;
   if (RhsType->getKind() == BuiltinType::Bool && LhsType->isSignedInteger())
     return handleBooleanToSignedIntegral(Context, SourceLoc, Lhs, Rhs);
   if (RhsType->isInteger() && LhsType->getKind() == BuiltinType::Bool)
     return handleIntegralToBoolean(Context, SourceLoc, Lhs, Rhs);
   if (RhsType->isInteger() && LhsType->isFloatingPoint())
     return handleIntegralToFloating(Context, SourceLoc, Lhs, Rhs);
   if (RhsType->isInteger() && LhsType->isInteger())
     return handleIntegralCast(Context, SourceLoc, Lhs, Rhs);

 void NarrowingConversionsCheck::handleImplicitCast(
     const ASTContext &Context, const ImplicitCastExpr &Cast) {
   if (Cast.getExprLoc().isMacroID())
     return;
   const Expr &Lhs = Cast;
   const Expr &Rhs = *Cast.getSubExpr();
   if (Lhs.isInstantiationDependent() || Rhs.isInstantiationDependent())
     return;
+  if (getBuiltinType(Lhs) == getBuiltinType(Rhs))
+    return;
   if (handleConditionalOperator(Context, Lhs, Rhs))
     return;
   SourceLocation SourceLoc = Lhs.getExprLoc();
   switch (Cast.getCastKind()) {
   case CK_BooleanToSignedIntegral:
     return handleBooleanToSignedIntegral(Context, SourceLoc, Lhs, Rhs);
   case CK_IntegralToBoolean:
     return handleIntegralToBoolean(Context, SourceLoc, Lhs, Rhs);

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-narrowingfloatingpoint-option.cpp

void narrow_fp_constants() {
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from constant 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
   f = -__builtin_huge_val(); // -max double is not within-range of float.
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from constant 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
   f = __builtin_inf(); // double infinity is not within-range of float.
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from constant 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
   f = __builtin_nan("0"); // double NaN is not narrowing.
 }
 
+double false_positive_const_qualified_cast(bool t) {
+  double b = 1.0;
+  constexpr double a = __builtin_huge_val();
+  // PR49498 The constness difference of 'a' and 'b' results in an implicit cast.
+  return t ? b : a;
+}
+
 } // namespace floats