https://github.com/sstepashka created
https://github.com/llvm/llvm-project/pull/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.
>From 7b7ee29e1b72e0b4de09f8ee40c98c9c2fe090fe 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 | 19 +++++++-----
clang-tools-extra/docs/ReleaseNotes.rst | 6 ++++
...idening-of-multiplication-result-short.cpp | 30 ++++++++++++++-----
3 files changed, 40 insertions(+), 15 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
index 126dc9ba36192..fea7a650b84a6 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,21 @@ 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.");
WideExprTy = Context->getCorrespondingSignedType(Ty);
}
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
