[clang-tools-extra] 4530c3b - [clang-tidy] Fix cppcoreguidelines-narrowing-conversions false positive
Carlos Galvez via cfe-commits
cfe-commits at lists.llvm.org
Sat Apr 15 08:07:53 PDT 2023
Author: Nathan James
Date: 2023-04-15T15:07:44Z
New Revision: 4530c3bc4897f6633577de07b61ceb1bf7e79f50
URL: https://github.com/llvm/llvm-project/commit/4530c3bc4897f6633577de07b61ceb1bf7e79f50
DIFF: https://github.com/llvm/llvm-project/commit/4530c3bc4897f6633577de07b61ceb1bf7e79f50.diff
LOG: [clang-tidy] Fix cppcoreguidelines-narrowing-conversions false positive
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.
Reviewed By: carlosgalvezp
Differential Revision: https://reviews.llvm.org/D98416
Added:
Modified:
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-narrowingfloatingpoint-option.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
index 08157603aab89..f22c9f599a262 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -510,6 +510,8 @@ void NarrowingConversionsCheck::handleBinaryOperator(const ASTContext &Context,
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)
@@ -549,6 +551,8 @@ void NarrowingConversionsCheck::handleImplicitCast(
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();
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-narrowingfloatingpoint-option.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-narrowingfloatingpoint-option.cpp
index 14840df18d038..6cad3204c18e4 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-narrowingfloatingpoint-option.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-narrowingfloatingpoint-option.cpp
@@ -54,4 +54,11 @@ void narrow_fp_constants() {
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
diff erence of 'a' and 'b' results in an implicit cast.
+ return t ? b : a;
+}
+
} // namespace floats
More information about the cfe-commits
mailing list