diff options
10 files changed, 246 insertions, 0 deletions
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index b87961e07a1..86eb9d1ba5e 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/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 @@ public: "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>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index 64b1e32deaf..3cce01312ec 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -48,6 +48,7 @@ add_clang_library(clangTidyBugproneModule clangBasic clangLex clangTidy + clangTidyCppCoreGuidelinesModule clangTidyUtils clangTooling ) diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt index 2ce1a0800de..49fedd62203 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt @@ -4,6 +4,7 @@ add_clang_library(clangTidyCppCoreGuidelinesModule AvoidGotoCheck.cpp CppCoreGuidelinesTidyModule.cpp InterfacesGlobalInitCheck.cpp + NarrowingConversionsCheck.cpp NoMallocCheck.cpp OwningMemoryCheck.cpp ProBoundsArrayToPointerDecayCheck.cpp diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp index 51824eca9ac..a0392594696 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ b/clang-tools-extra/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 @@ public: "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"); diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp new file mode 100644 index 00000000000..2c5676e76a2 --- /dev/null +++ b/clang-tools-extra/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 diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h new file mode 100644 index 00000000000..35f58bc220a --- /dev/null +++ b/clang-tools-extra/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 diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 09fa3fafb52..9db42d614c5 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -241,6 +241,11 @@ Improvements to clang-tidy - 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 ----------------------------- diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst new file mode 100644 index 00000000000..b1e82cae8ca --- /dev/null +++ b/clang-tools-extra/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;``. + diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index c90902a3511..50f074fc9c7 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -78,6 +78,7 @@ Clang-Tidy Checks 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 diff --git a/clang-tools-extra/test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp b/clang-tools-extra/test/clang-tidy/cppcoreguidelines-narrowing-conversions.cpp new file mode 100644 index 00000000000..ecffba6b844 --- /dev/null +++ b/clang-tools-extra/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 |
