diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h @@ -93,6 +93,10 @@ 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; diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp @@ -71,8 +71,9 @@ 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 @@ -221,6 +222,22 @@ 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; @@ -237,6 +254,21 @@ 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) { @@ -351,7 +383,10 @@ 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); } @@ -360,25 +395,21 @@ const ASTContext &Context, SourceLocation SourceLoc, const Expr &Lhs, const Expr &Rhs) { llvm::APFloat FloatConstant(0.0); + if (getFloatingConstantExprValue(Context, Rhs, FloatConstant)) { + if (!isFloatExactlyRepresentable(Context, FloatConstant, Lhs.getType())) + return diagNarrowConstant(SourceLoc, Lhs, Rhs); - // We always warn when Rhs is non-constexpr. - if (!getFloatingConstantExprValue(Context, Rhs, FloatConstant)) - return diagNarrowType(SourceLoc, Lhs, Rhs); + if (PedanticMode) + return diagConstantCast(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) - return diagNarrowConstant(SourceLoc, Lhs, Rhs); + return; + } - if (PedanticMode) - return diagConstantCast(SourceLoc, Lhs, Rhs); + 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( diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp @@ -11,16 +11,35 @@ 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] }