https://github.com/sstepashka updated https://github.com/llvm/llvm-project/pull/214501
>From a1aea69f83c711ace14f6b5085dcb998661f2276 Mon Sep 17 00:00:00 2001 From: Dmitrii Kuragin <[email protected]> Date: Thu, 6 Aug 2026 07:32:29 -0700 Subject: [PATCH] [Clang-Tidy] Improve `bugprone-implicit-widening-of-multiplication-result`. 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. --- ...citWideningOfMultiplicationResultCheck.cpp | 24 ++++++++------- clang-tools-extra/docs/ReleaseNotes.rst | 6 ++++ ...idening-of-multiplication-result-short.cpp | 30 ++++++++++++++----- 3 files changed, 42 insertions(+), 18 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp index 126dc9ba36192..887326672b974 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,19 +132,22 @@ 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."); + else if (Ty->isSignedIntegerType()) WideExprTy = Context->getCorrespondingUnsignedType(Ty); - } else { - assert(Ty->isUnsignedIntegerType() && - "Expected target type to be unsigned."); - assert(ETy->isSignedIntegerType() && - "Expected source type to be unsigned."); + else WideExprTy = Context->getCorrespondingSignedType(Ty); - } { const auto Diag = diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 0dcf2ea1f21c7..82daa2db06f8c 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -113,6 +113,12 @@ New check aliases 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. + - 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. 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..12ed22de28e71 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,29 @@ -// 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>( ) } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
