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 WarnOnFloatingPointNarrowingConversion; const bool WarnWithinTemplateInstantiation; const bool WarnOnEquivalentBitWidth; 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 @@ -67,8 +67,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 @@ -217,6 +218,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; @@ -233,6 +250,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) { @@ -304,14 +336,6 @@ return; const BuiltinType *FromType = getBuiltinType(Rhs); - // 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; - } llvm::APSInt IntegerConstant; if (getIntegerConstantExprValue(Context, Rhs, IntegerConstant)) { @@ -320,6 +344,9 @@ Context.getTypeSize(FromType)); return; } + + if (isWarningInhibitedByEquivalentSize(Context, *FromType, *ToType)) + return; if (!isWideEnoughToHold(Context, *FromType, *ToType)) diagNarrowTypeToSignedInt(SourceLoc, Lhs, Rhs); } @@ -344,7 +371,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); } @@ -353,25 +383,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,33 @@ 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 signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions] + // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0. + f = i; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'int' to signed type 'float' is implementation-defined [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 signed type 'long long' is implementation-defined [cppcoreguidelines-narrowing-conversions] + // DISABLED: Warning disabled with WarnOnEquivalentBitWidth=0. + d = ll; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'long long' to signed type 'double' is implementation-defined [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] }