njames93 created this revision.
njames93 added reviewers: alexfh, aaron.ballman.
Herald added subscribers: shchenz, kbarton, xazax.hun, nemanjai.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D98416
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-narrowingfloatingpoint-option.cpp
Index:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-narrowingfloatingpoint-option.cpp
===================================================================
---
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-narrowingfloatingpoint-option.cpp
+++
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-narrowingfloatingpoint-option.cpp
@@ -54,4 +54,11 @@
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 difference of 'a' and 'b' results in an implicit
cast.
+ return t ? b : a;
+}
+
} // namespace floats
Index:
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -369,6 +369,8 @@
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)
@@ -408,6 +410,8 @@
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();
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-narrowingfloatingpoint-option.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-narrowingfloatingpoint-option.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-narrowingfloatingpoint-option.cpp
@@ -54,4 +54,11 @@
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 difference of 'a' and 'b' results in an implicit cast.
+ return t ? b : a;
+}
+
} // namespace floats
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -369,6 +369,8 @@
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)
@@ -408,6 +410,8 @@
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();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
