From 89c41c335dac288d991d1e99ad19493bc89439e4 Mon Sep 17 00:00:00 2001 From: Guillaume Chatelet Date: Thu, 10 Jun 2021 12:41:57 +0000 Subject: [PATCH] [clang-tidy] Allow disabling integer narrowing conversions for cppcoreguidelines-narrowing-conversions Differential Revision: https://reviews.llvm.org/D104018 --- .../NarrowingConversionsCheck.cpp | 57 ++++++++++++---------- .../cppcoreguidelines/NarrowingConversionsCheck.h | 1 + .../cppcoreguidelines-narrowing-conversions.rst | 8 ++- ...rrowing-conversions-narrowinginteger-option.cpp | 23 +++++++++ 4 files changed, 63 insertions(+), 26 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-narrowinginteger-option.cpp diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp index 8ce9afc8..41eabb4 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp @@ -35,6 +35,8 @@ auto hasAnyListedName(const std::string &Names) { NarrowingConversionsCheck::NarrowingConversionsCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), + WarnOnIntegerNarrowingConversion( + Options.get("WarnOnIntegerNarrowingConversion", true)), WarnOnFloatingPointNarrowingConversion( Options.get("WarnOnFloatingPointNarrowingConversion", true)), WarnWithinTemplateInstantiation( @@ -45,6 +47,8 @@ NarrowingConversionsCheck::NarrowingConversionsCheck(StringRef Name, void NarrowingConversionsCheck::storeOptions( ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "WarnOnIntegerNarrowingConversion", + WarnOnIntegerNarrowingConversion); Options.store(Opts, "WarnOnFloatingPointNarrowingConversion", WarnOnFloatingPointNarrowingConversion); Options.store(Opts, "WarnWithinTemplateInstantiation", @@ -294,34 +298,37 @@ void NarrowingConversionsCheck::handleIntegralCast(const ASTContext &Context, SourceLocation SourceLoc, const Expr &Lhs, const Expr &Rhs) { - const BuiltinType *ToType = getBuiltinType(Lhs); - // From [conv.integral]p7.3.8: - // Conversions to unsigned integer is well defined so no warning is issued. - // "The resulting value is the smallest unsigned value equal to the source - // value modulo 2^n where n is the number of bits used to represent the - // destination type." - if (ToType->isUnsignedInteger()) - return; - const BuiltinType *FromType = getBuiltinType(Rhs); - - // With this option, we don't warn on conversions that have equivalent width - // in bits. eg. uint32 <-> int32. - if (!WarnOnEquivalentBitWidth) { - uint64_t FromTypeSize = Context.getTypeSize(FromType); - uint64_t ToTypeSize = Context.getTypeSize(ToType); - if (FromTypeSize == ToTypeSize) + if (WarnOnIntegerNarrowingConversion) { + const BuiltinType *ToType = getBuiltinType(Lhs); + // From [conv.integral]p7.3.8: + // Conversions to unsigned integer is well defined so no warning is issued. + // "The resulting value is the smallest unsigned value equal to the source + // value modulo 2^n where n is the number of bits used to represent the + // destination type." + if (ToType->isUnsignedInteger()) return; - } + const BuiltinType *FromType = getBuiltinType(Rhs); - llvm::APSInt IntegerConstant; - if (getIntegerConstantExprValue(Context, Rhs, IntegerConstant)) { - if (!isWideEnoughToHold(Context, IntegerConstant, *ToType)) - diagNarrowIntegerConstantToSignedInt(SourceLoc, Lhs, Rhs, IntegerConstant, - Context.getTypeSize(FromType)); - return; + // With this option, we don't warn on conversions that have equivalent width + // in bits. eg. uint32 <-> int32. + if (!WarnOnEquivalentBitWidth) { + uint64_t FromTypeSize = Context.getTypeSize(FromType); + uint64_t ToTypeSize = Context.getTypeSize(ToType); + if (FromTypeSize == ToTypeSize) + return; + } + + llvm::APSInt IntegerConstant; + if (getIntegerConstantExprValue(Context, Rhs, IntegerConstant)) { + if (!isWideEnoughToHold(Context, IntegerConstant, *ToType)) + diagNarrowIntegerConstantToSignedInt(SourceLoc, Lhs, Rhs, + IntegerConstant, + Context.getTypeSize(FromType)); + return; + } + if (!isWideEnoughToHold(Context, *FromType, *ToType)) + diagNarrowTypeToSignedInt(SourceLoc, Lhs, Rhs); } - if (!isWideEnoughToHold(Context, *FromType, *ToType)) - diagNarrowTypeToSignedInt(SourceLoc, Lhs, Rhs); } void NarrowingConversionsCheck::handleIntegralToBoolean( diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h index 12a6bb8..47bb222 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.h @@ -93,6 +93,7 @@ private: void handleBinaryOperator(const ASTContext &Context, const BinaryOperator &Op); + const bool WarnOnIntegerNarrowingConversion; const bool WarnOnFloatingPointNarrowingConversion; const bool WarnWithinTemplateInstantiation; const bool WarnOnEquivalentBitWidth; 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 index bfc7669..3502fb9 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-narrowing-conversions.rst @@ -13,7 +13,8 @@ Guidelines, corresponding to rule ES.46. See 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 narrowing conversions from: - - an integer to a narrower integer (e.g. ``char`` to ``unsigned char``), + - an integer to a narrower integer (e.g. ``char`` to ``unsigned char``) + if WarnOnIntegerNarrowingConversion Option is set, - an integer to a narrower floating-point (e.g. ``uint64_t`` to ``float``), - a floating-point to an integer (e.g. ``double`` to ``int``), - a floating-point to a narrower floating-point (e.g. ``double`` to ``float``) @@ -30,6 +31,11 @@ This check will flag: Options ------- +.. option:: WarnOnIntegerNarrowingConversion + + When `true`, the check will warn on narrowing integer conversion + (e.g. ``int`` to ``size_t``). `true` by default. + .. option:: WarnOnFloatingPointNarrowingConversion When `true`, the check will warn on narrowing floating point conversion diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-narrowinginteger-option.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-narrowinginteger-option.cpp new file mode 100644 index 0000000..a3944bc --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions-narrowinginteger-option.cpp @@ -0,0 +1,23 @@ +// RUN: %check_clang_tidy -check-suffix=DEFAULT %s \ +// RUN: cppcoreguidelines-narrowing-conversions %t -- \ +// RUN: -config='{CheckOptions: [{key: cppcoreguidelines-narrowing-conversions.WarnOnIntegerNarrowingConversion, value: true}]}' + +// RUN: %check_clang_tidy -check-suffix=DISABLED %s \ +// RUN: cppcoreguidelines-narrowing-conversions %t -- \ +// RUN: -config='{CheckOptions: [{key: cppcoreguidelines-narrowing-conversions.WarnOnIntegerNarrowingConversion, value: false}]}' + +void foo(unsigned long long value) { + int a = value; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:11: warning: narrowing conversion from 'unsigned long long' to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions] + // DISABLED: No warning for integer narrowing conversions when WarnOnIntegerNarrowingConversion = false. + long long b = value; + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:17: warning: narrowing conversion from 'unsigned long long' to signed type 'long long' is implementation-defined [cppcoreguidelines-narrowing-conversions] + // DISABLED: No warning for integer narrowing conversions when WarnOnIntegerNarrowingConversion = false. +} + +void casting_float_to_bool_is_still_operational_when_integer_narrowing_is_disabled(float f) { + if (f) { + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:7: warning: narrowing conversion from 'float' to 'bool' [cppcoreguidelines-narrowing-conversions] + // CHECK-MESSAGES-DISABLED: :[[@LINE-2]]:7: warning: narrowing conversion from 'float' to 'bool' [cppcoreguidelines-narrowing-conversions] + } +} -- 2.7.4