Author: Dmitrii Kuragin Date: 2026-08-28T12:17:25+08:00 New Revision: a7831dcce98893f3514f889a827f0d38da46621a
URL: https://github.com/llvm/llvm-project/commit/a7831dcce98893f3514f889a827f0d38da46621a DIFF: https://github.com/llvm/llvm-project/commit/a7831dcce98893f3514f889a827f0d38da46621a.diff LOG: [Clang-Tidy] Improve `bugprone-implicit-widening-of-multiplication-result`. (#214501) Implicit integer promotions make it a bit difficult to deduce the correct type in the following expression: ``` std::uint64_t calc_array_size(std::uint16_t width, std::uint16_t height) { return width * height; } ``` Originally, Clang-Tidy suggested to use the following code: ``` return static_cast<long long>(width) * height; ``` It is fully correct according to the C++ rules, but it makes it a bit harder to reason for people. This change adds a more readable "FixIt" taking into account the source type and avoid intermediate representations. Co-authored-by: Dmitrii Kuragin <[email protected]> Added: Modified: clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp clang-tools-extra/docs/ReleaseNotes.md clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-short.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp index 126dc9ba36192..229ad5be223b7 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp @@ -104,6 +104,7 @@ void ImplicitWideningOfMultiplicationResultCheck::handleImplicitCastExpr( const Expr *LHS = getLHSOfMulBinOp(E); if (!LHS) return; + const Expr *RHS = cast<BinaryOperator>(E)->getRHS()->IgnoreParens(); // Ok, looks like we should diagnose this. diag(E->getBeginLoc(), "performing an implicit widening conversion to type " @@ -131,17 +132,24 @@ void ImplicitWideningOfMultiplicationResultCheck::handleImplicitCastExpr( QualType WideExprTy; // Get Ty of the same signedness as ExprTy, because we only want to suggest // to widen the computation, but not change it's signedness domain. - if (Ty->isSignedIntegerType() == ETy->isSignedIntegerType()) { + // However, if ETy is only signed because both operands were of an + // unsigned type narrower than int (and thus got integer-promoted to the + // signed type int), the multiplication was never really operating in a + // signed domain to begin with, so don't force a signed widened type in + // that case either. + const bool BothOperandsWereUnsigned = + LHS->IgnoreImpCasts()->getType()->isUnsignedIntegerType() && + RHS->IgnoreImpCasts()->getType()->isUnsignedIntegerType(); + const bool EffectiveETyIsSigned = + ETy->isSignedIntegerType() && !BothOperandsWereUnsigned; + if (Ty->isSignedIntegerType() == EffectiveETyIsSigned) { WideExprTy = Ty; } else if (Ty->isSignedIntegerType()) { - assert(ETy->isUnsignedIntegerType() && - "Expected source type to be signed."); WideExprTy = Context->getCorrespondingUnsignedType(Ty); } else { assert(Ty->isUnsignedIntegerType() && "Expected target type to be unsigned."); - assert(ETy->isSignedIntegerType() && - "Expected source type to be unsigned."); + assert(ETy->isSignedIntegerType() && "Expected source type to be signed."); WideExprTy = Context->getCorrespondingSignedType(Ty); } diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index 6fe497e5f6eaf..420b7ddce20e6 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -129,6 +129,12 @@ infrastructure are described first, followed by tool-specific sections. #### Changes in existing checks +- Improved {doc}`bugprone-implicit-widening-of-multiplication-result + <clang-tidy/checks/bugprone/implicit-widening-of-multiplication-result>` check + by suggesting a wider type of the same signedness as the original operands, + instead of forcing a signed type, when a multiplication of two unsigned + operands narrower than `int` is only signed due to integer promotion. + - Fixed a crash in {doc}`bugprone-misplaced-operator-in-strlen-in-alloc <clang-tidy/checks/bugprone/misplaced-operator-in-strlen-in-alloc>` when checking an array new expression without a size expression. @@ -144,7 +150,7 @@ infrastructure are described first, followed by tool-specific sections. - Improved {doc}`cppcoreguidelines-pro-type-member-init <clang-tidy/checks/cppcoreguidelines/pro-type-member-init>` check by treating `std::array` the same as built-in arrays when `IgnoreArrays` option is enabled. - + - Improved {doc}`cppcoreguidelines-use-enum-class <clang-tidy/checks/cppcoreguidelines/use-enum-class>` check by omitting unnamed enums from the `enum class` requirement, as previously the check suggested users an ill-formed fix. diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-short.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-short.cpp index d824ea3b818d7..a6c59ee1e9017 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-short.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-short.cpp @@ -1,15 +1,41 @@ -// RUN: %check_clang_tidy -std=c99 %s bugprone-implicit-widening-of-multiplication-result %t -- -- -target x86_64-unknown-unknown -x c -// RUN: %check_clang_tidy %s bugprone-implicit-widening-of-multiplication-result %t -- -- -target x86_64-unknown-unknown -x c++ +// RUN: %check_clang_tidy -check-suffixes=ALL,C -std=c99 %s bugprone-implicit-widening-of-multiplication-result %t -- -- -target x86_64-unknown-unknown -x c +// RUN: %check_clang_tidy -check-suffixes=ALL,CXX %s bugprone-implicit-widening-of-multiplication-result %t -- -- -target x86_64-unknown-unknown -x c++ long t0(short a, int b) { return a * b; - // CHECK-NOTES: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'long' of a multiplication performed in type 'int' - // CHECK-NOTES: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning - // CHECK-NOTES: :[[@LINE-3]]:10: note: perform multiplication in a wider type + // CHECK-NOTES-ALL: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type '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: :[[@LINE-3]]:10: note: perform multiplication in a wider type } long t1(short a, short b) { return a * b; - // CHECK-NOTES: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'long' of a multiplication performed in type 'int' - // CHECK-NOTES: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning - // CHECK-NOTES: :[[@LINE-3]]:10: note: perform multiplication in a wider type + // CHECK-NOTES-ALL: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type '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: :[[@LINE-3]]:10: note: perform multiplication in a wider type } + +// Both operands are unsigned, and the multiplication only became a signed +// 'int' due to integer promotion; the suggested wider type should stay +// unsigned instead of switching signedness domains. +unsigned long t2(unsigned short a, unsigned short b) { + 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-C: (unsigned long)( ) + // CHECK-NOTES-CXX: static_cast<unsigned long>( ) + // CHECK-NOTES-ALL: :[[@LINE-5]]:10: note: perform multiplication in a wider type + // CHECK-NOTES-C: (unsigned long) + // CHECK-NOTES-CXX: static_cast<unsigned long>( ) +} + +long t3(unsigned short a, unsigned short b) { + return a * b; + // CHECK-NOTES-ALL: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'long' of a multiplication performed in type 'int' + // CHECK-NOTES-ALL: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning + // CHECK-NOTES-C: (long)( ) + // CHECK-NOTES-CXX: static_cast<long>( ) + // CHECK-NOTES-ALL: :[[@LINE-5]]:10: note: perform multiplication in a wider type + // CHECK-NOTES-C: (unsigned long) + // CHECK-NOTES-CXX: static_cast<unsigned long>( ) +} + _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
