courbet updated this revision to Diff 147508.
courbet marked 2 inline comments as done.
courbet added a comment.
- More explicit documentation.
- Do not trigger in template and macro contexts.
- More tests.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D38455
Files:
clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
Index: test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp
@@ -0,0 +1,103 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-narrowing-conversions %t
+
+float ceil(float);
+namespace std {
+double ceil(double);
+long double floor(long double);
+} // namespace std
+
+namespace floats {
+
+struct ConvertsToFloat {
+ operator float() const { return 0.5; }
+};
+
+float operator "" _Pa(unsigned long long);
+
+void not_ok(double d) {
+ int i = 0;
+ i = d;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
+ i = 0.5f;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
+ i = static_cast<float>(d);
+ // 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;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
+}
+
+void 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]
+ i += 0.5f;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
+ i += d;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [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.
+ i += 2.0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
+ i += 2.0f;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
+
+ i *= 0.5f;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
+ i /= 0.5f;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'float' to 'int' [cppcoreguidelines-narrowing-conversions]
+ i += (double)0.5f;
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: narrowing conversion from 'double' to 'int' [cppcoreguidelines-narrowing-conversions]
+}
+
+void ok(double d) {
+ int i = 0;
+ i = 1;
+ i = static_cast<int>(0.5);
+ i = static_cast<int>(d);
+ i = std::ceil(0.5);
+ i = ::std::floor(0.5);
+ {
+ using std::ceil;
+ i = ceil(0.5f);
+ }
+ i = ceil(0.5f);
+}
+
+void ok_binary_ops(double d) {
+ int i = 0;
+ i += 1;
+ i += static_cast<int>(0.5);
+ i += static_cast<int>(d);
+ i += (int)d;
+ i += std::ceil(0.5);
+ i += ::std::floor(0.5);
+ {
+ using std::ceil;
+ i += ceil(0.5f);
+ }
+ i += ceil(0.5f);
+}
+
+// We're bailing out in templates and macros.
+template <typename T1, typename T2>
+void f(T1 one, T2 two) {
+ one += two;
+}
+
+void template_context() {
+ f(1, 2);
+ f(1, .5);
+}
+
+#define DERP(i, j) (i += j)
+
+void macro_context() {
+ int i = 0;
+ DERP(i, 2);
+ DERP(i, .5);
+}
+
+} // namespace floats
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -78,6 +78,7 @@
cppcoreguidelines-avoid-goto
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) <cppcoreguidelines-c-copy-assignment-signature>
cppcoreguidelines-interfaces-global-init
+ cppcoreguidelines-narrowing-conversions
cppcoreguidelines-no-malloc
cppcoreguidelines-owning-memory
cppcoreguidelines-pro-bounds-array-to-pointer-decay
Index: docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst
@@ -0,0 +1,22 @@
+.. title:: clang-tidy - cppcoreguidelines-narrowing-conversions
+
+cppcoreguidelines-narrowing-conversions
+=======================================
+
+Checks for silent narrowing conversions, e.g: ``int i = 0; i += 0.1;``. While
+the issue is obvious in this former example, it might not be so in the
+following: ``void MyClass::f(double d) { int_member_ += d; }``.
+
+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.
+
+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;``.
+
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -241,6 +241,11 @@
- The 'google-runtime-member-string-references' check was removed.
+- New `cppcoreguidelines-narrowing-conversions
+ <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.html>`_ check
+
+ Checks for narrowing conversions, e.g. ``int i = 0; i += 0.1;``.
+
Improvements to include-fixer
-----------------------------
Index: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h
@@ -0,0 +1,37 @@
+//===--- NarrowingConversionsCheck.h - clang-tidy----------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_NARROWING_CONVERSIONS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_NARROWING_CONVERSIONS_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// Checks for narrowing conversions, e.g:
+/// int i = 0;
+/// i += 0.1;
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.html
+class NarrowingConversionsCheck : public ClangTidyCheck {
+public:
+ NarrowingConversionsCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_NARROWING_CONVERSIONS_H
Index: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
@@ -0,0 +1,70 @@
+//===--- NarrowingConversionsCheck.cpp - clang-tidy------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "NarrowingConversionsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+// FIXME: Check double -> float truncation. Pay attention to casts:
+void NarrowingConversionsCheck::registerMatchers(MatchFinder *Finder) {
+ // ceil() and floor() are guaranteed to return integers, even though the type
+ // is not integral.
+ const auto IsCeilFloorCall = callExpr(callee(functionDecl(
+ hasAnyName("::ceil", "::std::ceil", "::floor", "::std::floor"))));
+
+ const auto IsFloatExpr =
+ expr(hasType(realFloatingPointType()), unless(IsCeilFloorCall));
+
+ // 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);
+
+ // 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"),
+ this);
+}
+
+void NarrowingConversionsCheck::check(const MatchFinder::MatchResult &Result) {
+ if (const auto *Op = Result.Nodes.getNodeAs<BinaryOperator>("op")) {
+ if (Op->getLocStart().isMacroID())
+ return;
+ diag(Op->getOperatorLoc(), "narrowing conversion from %0 to %1")
+ << Op->getRHS()->getType() << Op->getLHS()->getType();
+ return;
+ }
+ const auto *Cast = Result.Nodes.getNodeAs<ImplicitCastExpr>("cast");
+ if (Cast->getLocStart().isMacroID())
+ return;
+ diag(Cast->getExprLoc(), "narrowing conversion from %0 to %1")
+ << Cast->getSubExpr()->getType() << Cast->getType();
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===================================================================
--- clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -13,6 +13,7 @@
#include "../misc/UnconventionalAssignOperatorCheck.h"
#include "AvoidGotoCheck.h"
#include "InterfacesGlobalInitCheck.h"
+#include "NarrowingConversionsCheck.h"
#include "NoMallocCheck.h"
#include "OwningMemoryCheck.h"
#include "ProBoundsArrayToPointerDecayCheck.h"
@@ -40,6 +41,8 @@
"cppcoreguidelines-avoid-goto");
CheckFactories.registerCheck<InterfacesGlobalInitCheck>(
"cppcoreguidelines-interfaces-global-init");
+ CheckFactories.registerCheck<NarrowingConversionsCheck>(
+ "cppcoreguidelines-narrowing-conversions");
CheckFactories.registerCheck<NoMallocCheck>("cppcoreguidelines-no-malloc");
CheckFactories.registerCheck<OwningMemoryCheck>(
"cppcoreguidelines-owning-memory");
Index: clang-tidy/cppcoreguidelines/CMakeLists.txt
===================================================================
--- clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -4,6 +4,7 @@
AvoidGotoCheck.cpp
CppCoreGuidelinesTidyModule.cpp
InterfacesGlobalInitCheck.cpp
+ NarrowingConversionsCheck.cpp
NoMallocCheck.cpp
OwningMemoryCheck.cpp
ProBoundsArrayToPointerDecayCheck.cpp
Index: clang-tidy/bugprone/CMakeLists.txt
===================================================================
--- clang-tidy/bugprone/CMakeLists.txt
+++ clang-tidy/bugprone/CMakeLists.txt
@@ -48,6 +48,7 @@
clangBasic
clangLex
clangTidy
+ clangTidyCppCoreGuidelinesModule
clangTidyUtils
clangTooling
)
Index: clang-tidy/bugprone/BugproneTidyModule.cpp
===================================================================
--- clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -10,6 +10,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "../cppcoreguidelines/NarrowingConversionsCheck.h"
#include "ArgumentCommentCheck.h"
#include "AssertSideEffectCheck.h"
#include "BoolPointerImplicitConversionCheck.h"
@@ -92,6 +93,8 @@
"bugprone-move-forwarding-reference");
CheckFactories.registerCheck<MultipleStatementMacroCheck>(
"bugprone-multiple-statement-macro");
+ CheckFactories.registerCheck<cppcoreguidelines::NarrowingConversionsCheck>(
+ "bugprone-narrowing-conversions");
CheckFactories.registerCheck<ParentVirtualCallCheck>(
"bugprone-parent-virtual-call");
CheckFactories.registerCheck<SizeofContainerCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
