gchatelet updated this revision to Diff 170878.
gchatelet marked an inline comment as done.
gchatelet added a comment.
- Join the two parts of the ReleaseNotes update
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53488
Files:
clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
Index: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
===================================================================
--- test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
+++ test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
@@ -9,12 +9,12 @@
namespace floats {
struct ConvertsToFloat {
- operator float() const { return 0.5; }
+ operator float() const { return 0.5f; }
};
-float operator "" _Pa(unsigned long long);
+float operator"" _float(unsigned long long);
-void not_ok(double d) {
+void narrow_double_to_int_not_ok(double d) {
int i = 0;
i = d;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
@@ -24,11 +24,11 @@
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
i = ConvertsToFloat();
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
- i = 15_Pa;
+ i = 15_float;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
}
-void not_ok_binary_ops(double d) {
+void narrow_double_to_int_not_ok_binary_ops(double d) {
int i = 0;
i += 0.5;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
@@ -52,6 +52,43 @@
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
}
+double operator"" _double(unsigned long long);
+
+float narrow_double_to_float_return() {
+ return 0.5;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_double_to_float_not_ok(double d) {
+ float f = 0;
+ f = d;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+ f = 0.5;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+ f = 15_double;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+}
+
+void narrow_double_to_float_not_ok_binary_ops(double d) {
+ float f = 0;
+ f += 0.5;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+ f += d;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+ // We warn on the following even though it's not dangerous because there is no
+ // reason to use a double literal here.
+ // TODO(courbet): Provide an automatic fix.
+ f += 2.0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+
+ f *= 0.5;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+ f /= 0.5;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+ f += (double)0.5f;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'float' [cppcoreguidelines-narrowing-conversions]
+}
+
void ok(double d) {
int i = 0;
i = 1;
@@ -89,15 +126,19 @@
void template_context() {
f(1, 2);
+ f(1, .5f);
f(1, .5);
+ f(1, .5l);
}
#define DERP(i, j) (i += j)
void macro_context() {
int i = 0;
DERP(i, 2);
+ DERP(i, .5f);
DERP(i, .5);
+ DERP(i, .5l);
}
-} // namespace floats
+} // namespace floats
Index: docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
===================================================================
--- docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
+++ docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
@@ -10,13 +10,15 @@
This rule is part of the "Expressions and statements" profile of the C++ Core
Guidelines, corresponding to rule ES.46. See
-https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Res-narrowing.
+https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es46-avoid-lossy-narrowing-truncating-arithmetic-conversions.
-We enforce only part of the guideline, more specifically, we flag:
- - All floating-point to integer conversions that are not marked by an explicit
- cast (c-style or ``static_cast``). For example: ``int i = 0; i += 0.1;``,
- ``void f(int); f(0.1);``,
- - All applications of binary operators where the left-hand-side is an integer
- and the right-hand-size is a floating-point. For example:
- ``int i; i+= 0.1;``.
+A narrowing conversion is currently defined as a conversion from:
+ - a floating-point to an integer (e.g. ``double`` to ``int``),
+ - a floating-point to a narrower floating-point (e.g. ``double`` to ``float``).
+This check will flag:
+ - All narrowing conversions that are not marked by an explicit cast (c-style or
+ ``static_cast``). For example: ``int i = 0; i += 0.1;``,
+ ``void f(int); f(0.1);``,
+ - All applications of binary operators with a narrowing conversions.
+ For example: ``int i; i+= 0.1;``.
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -163,6 +163,11 @@
<clang-tidy/checks/readability-redundant-smartptr-get>` check does not warn
about calls inside macros anymore by default.
+- The :doc:`cppcoreguidelines-narrowing-conversions
+ <clang-tidy/checks/cppcoreguidelines-narrowing-conversions>` check now
+ detects narrowing conversions between floating-point types
+ (e.g. ``double`` to ``float``).
+
Improvements to include-fixer
-----------------------------
Index: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
===================================================================
--- clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
+++ clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
@@ -28,6 +28,13 @@
: ClangTidyCheck(Name, Context) {}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ void checkBinaryOperator(const ASTContext *const Context,
+ const BinaryOperator &Op);
+
+ void checkCastOperator(const ASTContext *const Context,
+ const ImplicitCastExpr &Cast);
};
} // namespace cppcoreguidelines
Index: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
===================================================================
--- clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
+++ clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -27,42 +27,68 @@
const auto IsFloatExpr =
expr(hasType(realFloatingPointType()), unless(IsCeilFloorCall));
- // casts:
+ // Casts:
// i = 0.5;
// void f(int); f(0.5);
- Finder->addMatcher(implicitCastExpr(hasImplicitDestinationType(isInteger()),
- hasSourceExpression(IsFloatExpr),
- unless(hasParent(castExpr())),
- unless(isInTemplateInstantiation()))
- .bind("cast"),
- this);
+ Finder->addMatcher(
+ implicitCastExpr(
+ anyOf(hasImplicitDestinationType(isInteger()),
+ hasImplicitDestinationType(realFloatingPointType())),
+ hasSourceExpression(IsFloatExpr), unless(hasParent(castExpr())),
+ unless(isInTemplateInstantiation()))
+ .bind("cast"),
+ this);
// Binary operators:
// i += 0.5;
Finder->addMatcher(
- binaryOperator(isAssignmentOperator(),
- // The `=` case generates an implicit cast which is covered
- // by the previous matcher.
- unless(hasOperatorName("=")),
- hasLHS(hasType(isInteger())), hasRHS(IsFloatExpr),
- unless(isInTemplateInstantiation()))
- .bind("op"),
+ binaryOperator(
+ isAssignmentOperator(),
+ // The `=` case generates an implicit cast which is covered
+ // by the previous matcher.
+ unless(hasOperatorName("=")),
+ hasLHS(anyOf(hasType(isInteger()), hasType(realFloatingPointType()))),
+ hasRHS(IsFloatExpr), unless(isInTemplateInstantiation()))
+ .bind("binary_op"),
this);
}
-void NarrowingConversionsCheck::check(const MatchFinder::MatchResult &Result) {
- if (const auto *Op = Result.Nodes.getNodeAs<BinaryOperator>("op")) {
- if (Op->getBeginLoc().isMacroID())
- return;
- diag(Op->getOperatorLoc(), "narrowing conversion from %0 to %1")
- << Op->getRHS()->getType() << Op->getLHS()->getType();
+static bool isNarrower(const ASTContext *const Context, const QualType Lhs,
+ const QualType Rhs) {
+ assert(Lhs->isRealType()); // Either integer or floating point.
+ assert(Rhs->isFloatingType()); // Floating point only.
+ return Lhs->isIntegerType() ||
+ (Context->getTypeSize(Rhs) > Context->getTypeSize(Lhs));
+}
+
+void NarrowingConversionsCheck::checkBinaryOperator(
+ const ASTContext *const Context, const BinaryOperator &Op) {
+ if (Op.getBeginLoc().isMacroID())
return;
- }
- const auto *Cast = Result.Nodes.getNodeAs<ImplicitCastExpr>("cast");
- if (Cast->getBeginLoc().isMacroID())
+ const auto LhsType = Op.getLHS()->getType();
+ const auto RhsType = Op.getRHS()->getType();
+ if (isNarrower(Context, LhsType, RhsType))
+ diag(Op.getOperatorLoc(), "narrowing conversion from %0 to %1")
+ << RhsType << LhsType;
+}
+
+void NarrowingConversionsCheck::checkCastOperator(
+ const ASTContext *const Context, const ImplicitCastExpr &Cast) {
+ if (Cast.getBeginLoc().isMacroID())
return;
- diag(Cast->getExprLoc(), "narrowing conversion from %0 to %1")
- << Cast->getSubExpr()->getType() << Cast->getType();
+ const auto LhsType = Cast.getType();
+ const auto RhsType = Cast.getSubExpr()->getType();
+ if (isNarrower(Context, LhsType, RhsType))
+ diag(Cast.getExprLoc(), "narrowing conversion from %0 to %1")
+ << RhsType << LhsType;
+}
+
+void NarrowingConversionsCheck::check(const MatchFinder::MatchResult &Result) {
+ if (const auto *Op = Result.Nodes.getNodeAs<BinaryOperator>("binary_op"))
+ return checkBinaryOperator(Result.Context, *Op);
+ if (const auto *Cast = Result.Nodes.getNodeAs<ImplicitCastExpr>("cast"))
+ return checkCastOperator(Result.Context, *Cast);
+ llvm_unreachable("must be binary operator or cast expression");
}
} // namespace cppcoreguidelines
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
