[clang-tools-extra] [clang-tidy] bugprone-implicit-widening ignores const exprs that fit (PR #98352)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 10 10:14:28 PDT 2024
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
More information about the cfe-commits
mailing list