Skip to content

[clang-tidy] bugprone-implicit-widening ignores const exprs that fit #98352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {}
Expand All @@ -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());
}

Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class ImplicitWideningOfMultiplicationResultCheck : public ClangTidyCheck {
private:
const bool UseCXXStaticCastsInCppSources;
const bool UseCXXHeadersInCppSources;
const bool IgnoreConstantIntExpr;
utils::IncludeInserter IncludeInserter;
};

Expand Down
5 changes: 5 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,11 @@ Changes in existing checks
<clang-tidy/checks/bugprone/forwarding-reference-overload>`
check to ignore deleted constructors which won't hide other overloads.

- Improved :doc:`bugprone-implicit-widening-of-multiplication-result
<clang-tidy/checks/bugprone/implicit-widening-of-multiplication-result>` check
by adding an option to ignore constant expressions of signed integer types
that fit in the source expression type.

- Improved :doc:`bugprone-inc-dec-in-conditions
<clang-tidy/checks/bugprone/inc-dec-in-conditions>` check to ignore code
within unevaluated contexts, such as ``decltype``.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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. Defaults to ``false``.

Examples:

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// RUN: %check_clang_tidy %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;
}

unsigned long t1() {
const int a = 2;
const int b = 3;
return a * b;
}

long t2() {
constexpr int a = 16383; // ~1/2 of int16_t max
constexpr int b = 2;
return a * b;
}

constexpr int global_value() {
return 16;
}

unsigned long t3() {
constexpr int a = 3;
return a * global_value();
}

long t4() {
const char a = 3;
const short b = 2;
const int c = 5;
return c * b * a;
}

long t5() {
constexpr int min_int = (-2147483647 - 1); // A literal of -2147483648 evaluates to long
return 1 * min_int;
}

unsigned long n0() {
const int a = 1073741824; // 1/2 of int32_t max
const int b = 3;
return a * b;
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: performing an implicit widening conversion to type 'unsigned long' of a multiplication performed in type 'int'
// CHECK-MESSAGES: :[[@LINE-2]]:10: note: make conversion explicit to silence this warning
// CHECK-MESSAGES: static_cast<unsigned long>( )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually you should use CHECK-FIXES for this.

// CHECK-MESSAGES: :[[@LINE-4]]:10: note: perform multiplication in a wider type
}

double n1() {
const long a = 100000000;
return a * 400;
}
Loading