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

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-hacky 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.

Author: cwarner-8702

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);

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;
 };
 

clang-tools-extra/docs/ReleaseNotes.rst

@@ -245,6 +245,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``.

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.  Defaults to ``false``.
 
 Examples:
 

clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-constants.cpp

@@ -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>( )
+  // CHECK-MESSAGES: :[[@LINE-4]]:10: note: perform multiplication in a wider type
+}
+
+double n1() {
+  const long a = 100000000;
+  return a * 400;
+}