llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: Chris Warner (cwarner-8702) <details> <summary>Changes</summary> Add an option to the `bugprone-implicit-widening-of-multiplication-result` clang-tidy checker to suppress warnings when the expression is made up of all compile-time constants (literals, `constexpr` values or results, etc.) and the result of the multiplication is guaranteed to fit in both the source and destination types. This currently only works for signed integer types: * For unsigned integers, the well-defined rollover behavior on overflow prevents the checker from detecting that the expression does not actually fit in the source expr type, and would produce false negatives. I have a somewhat-tacky stab at addressing this I'd like to submit as a follow-on PR * For floating-point types, there's probably a little additional work to be done to `Expr` to calculate the result at compile time Even still, this seems like a helpful addition just for signed types, and additional types can be added. --- Full diff: https://github.com/llvm/llvm-project/pull/98352.diff 4 Files Affected: - (modified) clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp (+15) - (modified) clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.h (+1) - (modified) clang-tools-extra/docs/clang-tidy/checks/bugprone/implicit-widening-of-multiplication-result.rst (+6) - (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-constants.cpp (+81) ``````````diff diff --git a/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp index f99beac668ce7..46bf20e34ce04 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp @@ -42,6 +42,7 @@ ImplicitWideningOfMultiplicationResultCheck:: UseCXXStaticCastsInCppSources( Options.get("UseCXXStaticCastsInCppSources", true)), UseCXXHeadersInCppSources(Options.get("UseCXXHeadersInCppSources", true)), + IgnoreConstantIntExpr(Options.get("IgnoreConstantIntExpr", false)), IncludeInserter(Options.getLocalOrGlobal("IncludeStyle", utils::IncludeSorter::IS_LLVM), areDiagsSelfContained()) {} @@ -56,6 +57,7 @@ void ImplicitWideningOfMultiplicationResultCheck::storeOptions( Options.store(Opts, "UseCXXStaticCastsInCppSources", UseCXXStaticCastsInCppSources); Options.store(Opts, "UseCXXHeadersInCppSources", UseCXXHeadersInCppSources); + Options.store(Opts, "IgnoreConstantIntExpr", IgnoreConstantIntExpr); Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle()); } @@ -84,6 +86,19 @@ void ImplicitWideningOfMultiplicationResultCheck::handleImplicitCastExpr( if (TgtWidth <= SrcWidth) return; + // Is the expression a compile-time constexpr that we know can fit in the + // source type? + if (IgnoreConstantIntExpr && ETy->isIntegerType() && + !ETy->isUnsignedIntegerType()) { + if (const auto ConstExprResult = E->getIntegerConstantExpr(*Context)) { + const auto TypeSize = Context->getTypeSize(ETy); + llvm::APSInt WidenedResult = ConstExprResult->extOrTrunc(TypeSize); + if (WidenedResult <= llvm::APSInt::getMaxValue(TypeSize, false) && + WidenedResult >= llvm::APSInt::getMinValue(TypeSize, false)) + return; + } + } + // Does the index expression look like it might be unintentionally computed // in a narrower-than-wanted type? const Expr *LHS = getLHSOfMulBinOp(E); diff --git a/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.h b/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.h index 8b99930ae7a89..077a4b847cd9c 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.h @@ -41,6 +41,7 @@ class ImplicitWideningOfMultiplicationResultCheck : public ClangTidyCheck { private: const bool UseCXXStaticCastsInCppSources; const bool UseCXXHeadersInCppSources; + const bool IgnoreConstantIntExpr; utils::IncludeInserter IncludeInserter; }; diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/implicit-widening-of-multiplication-result.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/implicit-widening-of-multiplication-result.rst index c4ddd02602b73..6b857b9b82a21 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/implicit-widening-of-multiplication-result.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/implicit-widening-of-multiplication-result.rst @@ -45,6 +45,12 @@ Options should ``<cstddef>`` header be suggested, or ``<stddef.h>``. Defaults to ``true``. +.. option:: IgnoreConstantIntExpr + + If the multiplication operands are compile-time constants (like literals or + are ``constexpr``) and fit within the source expression type, do not emit a + diagnostic or suggested fix. Only considers expressions where the source + expression is a signed integer type. Examples: diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-constants.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-constants.cpp new file mode 100644 index 0000000000000..5a3dac0255df0 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-constants.cpp @@ -0,0 +1,81 @@ +// RUN: %check_clang_tidy -check-suffixes=ALL,NI %s bugprone-implicit-widening-of-multiplication-result %t -- -- -target x86_64-unknown-unknown -x c++ +// RUN: %check_clang_tidy -check-suffixes=ALL %s bugprone-implicit-widening-of-multiplication-result %t -- \ +// RUN: -config='{CheckOptions: { \ +// RUN: bugprone-implicit-widening-of-multiplication-result.IgnoreConstantIntExpr: true \ +// RUN: }}' -- -target x86_64-unknown-unknown -x c++ + +long t0() { + return 1 * 4; + // CHECK-NOTES-NI: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'long' of a multiplication performed in type 'int' + // CHECK-NOTES-NI: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning + // CHECK-NOTES-NI: static_cast<long>( ) + // CHECK-NOTES-NI: :[[@LINE-4]]:10: note: perform multiplication in a wider type +} + +unsigned long t1() { + const int a = 2; + const int b = 3; + return a * b; + // CHECK-NOTES-NI: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'unsigned long' of a multiplication performed in type 'int' + // CHECK-NOTES-NI: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning + // CHECK-NOTES-NI: static_cast<unsigned long>( ) + // CHECK-NOTES-NI: :[[@LINE-4]]:10: note: perform multiplication in a wider type +} + +long t2() { + constexpr int a = 16383; // ~1/2 of int16_t max + constexpr int b = 2; + return a * b; + // CHECK-NOTES-NI: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'long' of a multiplication performed in type 'int' + // CHECK-NOTES-NI: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning + // CHECK-NOTES-NI: static_cast<long>( ) + // CHECK-NOTES-NI: :[[@LINE-4]]:10: note: perform multiplication in a wider type +} + +constexpr int global_value() { + return 16; +} + +unsigned long t3() { + constexpr int a = 3; + return a * global_value(); + // CHECK-NOTES-NI: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'unsigned long' of a multiplication performed in type 'int' + // CHECK-NOTES-NI: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning + // CHECK-NOTES-NI: static_cast<unsigned long>( ) + // CHECK-NOTES-NI: :[[@LINE-4]]:10: note: perform multiplication in a wider type +} + +long t4() { + const char a = 3; + const short b = 2; + const int c = 5; + return c * b * a; + // CHECK-NOTES-NI: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'long' of a multiplication performed in type 'int' + // CHECK-NOTES-NI: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning + // CHECK-NOTES-NI: static_cast<long>( ) + // CHECK-NOTES-NI: :[[@LINE-4]]:10: note: perform multiplication in a wider type +} + +long t5() { + constexpr int min_int = (-2147483647 - 1); // A literal of -2147483648 evaluates to long + return 1 * min_int; + // CHECK-NOTES-NI: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'long' of a multiplication performed in type 'int' + // CHECK-NOTES-NI: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning + // CHECK-NOTES-NI: static_cast<long>( ) + // CHECK-NOTES-NI: :[[@LINE-4]]:10: note: perform multiplication in a wider type +} + +unsigned long n0() { + const int a = 1073741824; // 1/2 of int32_t max + const int b = 3; + return a * b; + // CHECK-NOTES-ALL: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'unsigned long' of a multiplication performed in type 'int' + // CHECK-NOTES-ALL: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning + // CHECK-NOTES-ALL: static_cast<unsigned long>( ) + // CHECK-NOTES-ALL: :[[@LINE-4]]:10: note: perform multiplication in a wider type +} + +double n1() { + const long a = 100000000; + return a * 400; +} `````````` </details> https://github.com/llvm/llvm-project/pull/98352 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
