NarrowingConversionsCheck should support inhibiting conversions of mixed integer and floating point types with WarnOnEquivalentBitWidth=0.

[Stephen]

Summary

Also standardize control flow of handleX conversion functions to make it easier to be consistent.

[Stephen]

Fix the expected error messages.

[Stephen]

Fix the diff.

[Stephen]

Undo that last move, since that didn't work.
I'm absolutely incompetent with this tool.

[Stephen]

Try specifying the commit range.

[Stephen]

Try again.

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

bool handleConditionalOperator(const ASTContext &Context, const Expr &Lhs,
                                  const Expr &Rhs);
 
   void handleImplicitCast(const ASTContext &Context,
                           const ImplicitCastExpr &Cast);
 
   void handleBinaryOperator(const ASTContext &Context,
                             const BinaryOperator &Op);
 
+  bool isWarningInhibitedByEquivalentSize(const ASTContext &Context,
+                                          const BuiltinType &FromType,
+                                          const BuiltinType &ToType) const;
+
   const bool WarnOnIntegerNarrowingConversion;
   const bool WarnOnFloatingPointNarrowingConversion;
   const bool WarnWithinTemplateInstantiation;
   const bool WarnOnEquivalentBitWidth;
   const std::string IgnoreConversionFromTypes;
   const bool PedanticMode;
 };
 
 } // namespace cppcoreguidelines
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_NARROWING_CONVERSIONS_H

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

void NarrowingConversionsCheck::registerMatchers(MatchFinder *Finder) {
 
   // We may want to exclude other types from the checks, such as `size_type`
   // and `difference_type`. These are often used to count elements, represented
   // in 64 bits and assigned to `int`. Rarely are people counting >2B elements.
   const auto IsConversionFromIgnoredType =
       hasType(namedDecl(hasAnyListedName(IgnoreConversionFromTypes)));
 
   // `IsConversionFromIgnoredType` will ignore narrowing calls from those types,
-  // but not expressions that are promoted to `int64` due to a binary expression
-  // with one of those types. For example, it will continue to reject:
+  // but not expressions that are promoted to an ignored type as a result of a
+  // binary expression with one of those types.
+  // For example, it will continue to reject:
   // `int narrowed = int_value + container.size()`.
   // We attempt to address common incidents of compound expressions with
   // `IsIgnoredTypeTwoLevelsDeep`, allowing binary expressions that have one
   // operand of the ignored types and the other operand of another integer type.
   const auto IsIgnoredTypeTwoLevelsDeep =
       anyOf(IsConversionFromIgnoredType,
             binaryOperator(hasOperands(IsConversionFromIgnoredType,
                                        hasType(isInteger()))));

 
 static bool isWideEnoughToHold(const ASTContext &Context,
                                const llvm::APSInt &IntegerConstant,
                                const BuiltinType &ToType) {
   IntegerRange ToIntegerRange = createFromType(Context, ToType);
   return ToIntegerRange.contains(IntegerConstant);
 }
 
+// Returns true iff the floating point constant can be losslessly represented
+// by an integer in the given destination type. eg. 2.0 can be accurately
+// represented by an int32_t, but neither 2^33 nor 2.001 can.
+static bool isFloatExactlyRepresentable(const ASTContext &Context,
+                                        const llvm::APFloat &FloatConstant,
+                                        const QualType &DestType) {
+  unsigned DestWidth = Context.getIntWidth(DestType);
+  bool DestSigned = DestType->isSignedIntegerOrEnumerationType();
+  llvm::APSInt Result = llvm::APSInt(DestWidth, !DestSigned);
+  bool IsExact = false;
+  bool Overflows = FloatConstant.convertToInteger(
+                       Result, llvm::APFloat::rmTowardZero, &IsExact) &
+                   llvm::APFloat::opInvalidOp;
+  return !Overflows && IsExact;
+}
+
 static llvm::SmallString<64> getValueAsString(const llvm::APSInt &Value,
                                               uint64_t HexBits) {
   llvm::SmallString<64> Str;
   Value.toString(Str, 10);
   if (HexBits > 0) {
     Str.append(" (0x");
     llvm::SmallString<32> HexValue;
     Value.toStringUnsigned(HexValue, 16);
     for (size_t I = HexValue.size(); I < (HexBits / 4); ++I)
       Str.append("0");
     Str.append(HexValue);
     Str.append(")");
   }
   return Str;
 }
 
+bool NarrowingConversionsCheck::isWarningInhibitedByEquivalentSize(
+    const ASTContext &Context, const BuiltinType &FromType,
+    const BuiltinType &ToType) const {
+  // With this option, we don't warn on conversions that have equivalent width
+  // in bits. eg. uint32 <-> int32.
+  if (!WarnOnEquivalentBitWidth) {
+    uint64_t FromTypeSize = Context.getTypeSize(&FromType);
+    uint64_t ToTypeSize = Context.getTypeSize(&ToType);
+    if (FromTypeSize == ToTypeSize) {
+      return true;
+    }
+  }
+  return false;
+}
+
 void NarrowingConversionsCheck::diagNarrowType(SourceLocation SourceLoc,
                                                const Expr &Lhs,
                                                const Expr &Rhs) {
   diag(SourceLoc, "narrowing conversion from %0 to %1")
       << getUnqualifiedType(Rhs) << getUnqualifiedType(Lhs);
 }
 
 void NarrowingConversionsCheck::diagNarrowTypeToSignedInt(
void NarrowingConversionsCheck::handleIntegralToFloating(
     const Expr &Rhs) {
   const BuiltinType *ToType = getBuiltinType(Lhs);
   llvm::APSInt IntegerConstant;
   if (getIntegerConstantExprValue(Context, Rhs, IntegerConstant)) {
     if (!isWideEnoughToHold(Context, IntegerConstant, *ToType))
       diagNarrowIntegerConstant(SourceLoc, Lhs, Rhs, IntegerConstant);
     return;
   }
+
   const BuiltinType *FromType = getBuiltinType(Rhs);
+  if (isWarningInhibitedByEquivalentSize(Context, *FromType, *ToType))
+    return;
   if (!isWideEnoughToHold(Context, *FromType, *ToType))
     diagNarrowType(SourceLoc, Lhs, Rhs);
 }
 
 void NarrowingConversionsCheck::handleFloatingToIntegral(
     const ASTContext &Context, SourceLocation SourceLoc, const Expr &Lhs,
     const Expr &Rhs) {
   llvm::APFloat FloatConstant(0.0);
-
-  // We always warn when Rhs is non-constexpr.
-  if (!getFloatingConstantExprValue(Context, Rhs, FloatConstant))
-    return diagNarrowType(SourceLoc, Lhs, Rhs);
-
-  QualType DestType = Lhs.getType();
-  unsigned DestWidth = Context.getIntWidth(DestType);
-  bool DestSigned = DestType->isSignedIntegerOrEnumerationType();
-  llvm::APSInt Result = llvm::APSInt(DestWidth, !DestSigned);
-  bool IsExact = false;
-  bool Overflows = FloatConstant.convertToInteger(
-                       Result, llvm::APFloat::rmTowardZero, &IsExact) &
-                   llvm::APFloat::opInvalidOp;
-  // We warn iff the constant floating point value is not exactly representable.
-  if (Overflows || !IsExact)
+  if (getFloatingConstantExprValue(Context, Rhs, FloatConstant)) {
+    if (!isFloatExactlyRepresentable(Context, FloatConstant, Lhs.getType()))
       return diagNarrowConstant(SourceLoc, Lhs, Rhs);
 
     if (PedanticMode)
       return diagConstantCast(SourceLoc, Lhs, Rhs);
+
+    return;
+  }
+
+  const BuiltinType *FromType = getBuiltinType(Rhs);
+  const BuiltinType *ToType = getBuiltinType(Lhs);
+  if (isWarningInhibitedByEquivalentSize(Context, *FromType, *ToType))
+    return;
+  diagNarrowType(SourceLoc, Lhs, Rhs); // Assumed always lossy.
 }
 
 void NarrowingConversionsCheck::handleFloatingToBoolean(
     const ASTContext &Context, SourceLocation SourceLoc, const Expr &Lhs,
     const Expr &Rhs) {
   return diagNarrowTypeOrConstant(Context, SourceLoc, Lhs, Rhs);
 }
 

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

 // RUN: %check_clang_tidy -check-suffix=DEFAULT %s \
 // RUN: cppcoreguidelines-narrowing-conversions %t -- \
 // RUN: -config='{CheckOptions: [ \
 // RUN: ]}'
 
 // RUN: %check_clang_tidy -check-suffix=DISABLED %s \
 // RUN: cppcoreguidelines-narrowing-conversions %t -- \
 // RUN: -config='{CheckOptions: [ \
 // RUN:   {key: cppcoreguidelines-narrowing-conversions.WarnOnEquivalentBitWidth, value: 0} \
 // RUN: ]}'
 
 void narrowing_equivalent_bitwidth() {
   int i;
-  unsigned int j;
-  i = j;
+  unsigned int ui;
+  i = ui;
   // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'unsigned int' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
   // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
+  float f;
+  i = f;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
+  // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
+  f = i;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'int' to 'float' [cppcoreguidelines-narrowing-conversions]
+  // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
+  long long ll;
+  double d;
+  ll = d;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:8: warning: narrowing conversion from 'double' to 'long long' [cppcoreguidelines-narrowing-conversions]
+  // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
+
+  d = ll;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to 'double' [cppcoreguidelines-narrowing-conversions]
+  // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0.
 }
 
 void most_narrowing_is_not_ok() {
   int i;
-  long long j;
-  i = j;
+  long long ui;
+  i = ui;
   // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
   // CHECK-MESSAGES-DISABLED: :[[@LINE-2]]:7: warning: narrowing conversion from 'long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]
 }