[clang-tools-extra] 1a53fb0 - [clang-tidy] NarrowingConversionsCheck should support inhibiting conversions of
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 11 04:03:01 PDT 2021
Author: Haojian Wu
Date: 2021-06-11T13:02:48+02:00
New Revision: 1a53fb0596abf4a8a9d5b4633cd5a8dc04f5e602
URL: https://github.com/llvm/llvm-project/commit/1a53fb0596abf4a8a9d5b4633cd5a8dc04f5e602
DIFF: https://github.com/llvm/llvm-project/commit/1a53fb0596abf4a8a9d5b4633cd5a8dc04f5e602.diff
LOG: [clang-tidy] NarrowingConversionsCheck should support inhibiting conversions of
mixed integer and floating point types with WarnOnEquivalentBitWidth=0.
Also standardize control flow of handleX conversion functions to make it easier to be consistent.
Patch by Stephen Concannon!
Differential Revision: https://reviews.llvm.org/D103894
Added:
Modified:
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
index 41eabb4f5bf92..5e59462b2900b 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -71,8 +71,9 @@ void NarrowingConversionsCheck::registerMatchers(MatchFinder *Finder) {
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 @@ static bool isWideEnoughToHold(const ASTContext &Context,
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 @@ static llvm::SmallString<64> getValueAsString(const llvm::APSInt &Value,
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 @@ void NarrowingConversionsCheck::handleIntegralToFloating(
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 @@ void NarrowingConversionsCheck::handleFloatingToIntegral(
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/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
index 47bb222bd0618..c6d087f867bda 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
@@ -93,6 +93,10 @@ class NarrowingConversionsCheck : public ClangTidyCheck {
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/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-equivalentbitwidth-option.cpp
index 947f1690d79c5..89ef6c01396a6 100644
--- 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]
}
More information about the cfe-commits
mailing list