[clang-tidy] Add new check readability-use-numeric-limits (#127430)

The adds a check that replaces specific numeric literals like `32767`
with the equivalent call to `std::numeric_limits` (such as
`std::numeric_limits<int16_t>::max())`.

Partially addresses #34434, but notably does not handle cases listed in
the title post such as `~0` and `-1`.

clang-tools-extra/clang-tidy/readability/CMakeLists.txt

@@ -58,6 +58,7 @@ add_clang_library(clangTidyReadabilityModule STATIC
   UniqueptrDeleteReleaseCheck.cpp
   UppercaseLiteralSuffixCheck.cpp
   UseAnyOfAllOfCheck.cpp
+  UseNumericLimitsCheck.cpp
   UseStdMinMaxCheck.cpp
 
   LINK_LIBS

clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp

@@ -61,6 +61,7 @@
 #include "UniqueptrDeleteReleaseCheck.h"
 #include "UppercaseLiteralSuffixCheck.h"
 #include "UseAnyOfAllOfCheck.h"
+#include "UseNumericLimitsCheck.h"
 #include "UseStdMinMaxCheck.h"
 
 namespace clang::tidy {
@@ -173,6 +174,8 @@ public:
         "readability-uppercase-literal-suffix");
     CheckFactories.registerCheck<UseAnyOfAllOfCheck>(
         "readability-use-anyofallof");
+    CheckFactories.registerCheck<UseNumericLimitsCheck>(
+        "readability-use-numeric-limits");
     CheckFactories.registerCheck<UseStdMinMaxCheck>(
         "readability-use-std-min-max");
   }

clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp

@@ -0,0 +1,160 @@
+//===--- UseNumericLimitsCheck.cpp - clang-tidy ---------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "UseNumericLimitsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include <cmath>
+#include <limits>
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+UseNumericLimitsCheck::UseNumericLimitsCheck(StringRef Name,
+                                             ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      SignedConstants{
+          {std::numeric_limits<int8_t>::min(),
+           "std::numeric_limits<int8_t>::min()"},
+          {std::numeric_limits<int8_t>::max(),
+           "std::numeric_limits<int8_t>::max()"},
+          {std::numeric_limits<int16_t>::min(),
+           "std::numeric_limits<int16_t>::min()"},
+          {std::numeric_limits<int16_t>::max(),
+           "std::numeric_limits<int16_t>::max()"},
+          {std::numeric_limits<int32_t>::min(),
+           "std::numeric_limits<int32_t>::min()"},
+          {std::numeric_limits<int32_t>::max(),
+           "std::numeric_limits<int32_t>::max()"},
+          {std::numeric_limits<int64_t>::min(),
+           "std::numeric_limits<int64_t>::min()"},
+          {std::numeric_limits<int64_t>::max(),
+           "std::numeric_limits<int64_t>::max()"},
+      },
+      UnsignedConstants{
+          {std::numeric_limits<uint8_t>::max(),
+           "std::numeric_limits<uint8_t>::max()"},
+          {std::numeric_limits<uint16_t>::max(),
+           "std::numeric_limits<uint16_t>::max()"},
+          {std::numeric_limits<uint32_t>::max(),
+           "std::numeric_limits<uint32_t>::max()"},
+          {std::numeric_limits<uint64_t>::max(),
+           "std::numeric_limits<uint64_t>::max()"},
+      },
+      Inserter(Options.getLocalOrGlobal("IncludeStyle",
+                                        utils::IncludeSorter::IS_LLVM),
+               areDiagsSelfContained()) {}
+
+void UseNumericLimitsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+}
+
+void UseNumericLimitsCheck::registerMatchers(MatchFinder *Finder) {
+  auto PositiveIntegerMatcher = [](auto Value) {
+    return unaryOperator(hasOperatorName("+"),
+                         hasUnaryOperand(integerLiteral(equals(Value))
+                                             .bind("positive-integer-literal")))
+        .bind("unary-op");
+  };
+
+  auto NegativeIntegerMatcher = [](auto Value) {
+    return unaryOperator(hasOperatorName("-"),
+                         hasUnaryOperand(integerLiteral(equals(-Value))
+                                             .bind("negative-integer-literal")))
+        .bind("unary-op");
+  };
+
+  auto BareIntegerMatcher = [](auto Value) {
+    return integerLiteral(allOf(unless(hasParent(unaryOperator(
+                                    hasAnyOperatorName("-", "+")))),
+                                equals(Value)))
+        .bind("bare-integer-literal");
+  };
+
+  for (const auto &[Value, _] : SignedConstants) {
+    if (Value < 0) {
+      Finder->addMatcher(NegativeIntegerMatcher(Value), this);
+    } else {
+      Finder->addMatcher(
+          expr(anyOf(PositiveIntegerMatcher(Value), BareIntegerMatcher(Value))),
+          this);
+    }
+  }
+
+  for (const auto &[Value, _] : UnsignedConstants) {
+    Finder->addMatcher(
+        expr(anyOf(PositiveIntegerMatcher(Value), BareIntegerMatcher(Value))),
+        this);
+  }
+}
+
+void UseNumericLimitsCheck::registerPPCallbacks(
+    const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+  Inserter.registerPreprocessor(PP);
+}
+
+void UseNumericLimitsCheck::check(const MatchFinder::MatchResult &Result) {
+  const IntegerLiteral *MatchedDecl = nullptr;
+
+  const IntegerLiteral *NegativeMatchedDecl =
+      Result.Nodes.getNodeAs<IntegerLiteral>("negative-integer-literal");
+  const IntegerLiteral *PositiveMatchedDecl =
+      Result.Nodes.getNodeAs<IntegerLiteral>("positive-integer-literal");
+  const IntegerLiteral *BareMatchedDecl =
+      Result.Nodes.getNodeAs<IntegerLiteral>("bare-integer-literal");
+
+  if (NegativeMatchedDecl != nullptr)
+    MatchedDecl = NegativeMatchedDecl;
+  else if (PositiveMatchedDecl != nullptr)
+    MatchedDecl = PositiveMatchedDecl;
+  else if (BareMatchedDecl != nullptr)
+    MatchedDecl = BareMatchedDecl;
+
+  const llvm::APInt MatchedIntegerConstant = MatchedDecl->getValue();
+
+  auto Fixer = [&](auto SourceValue, auto Value,
+                   const std::string &Replacement) {
+    static_assert(std::is_same_v<decltype(SourceValue), decltype(Value)>,
+                  "The types of SourceValue and Value must match");
+
+    SourceLocation Location = MatchedDecl->getExprLoc();
+    SourceRange Range{MatchedDecl->getBeginLoc(), MatchedDecl->getEndLoc()};
+
+    // Only valid if unary operator is present
+    const UnaryOperator *UnaryOpExpr =
+        Result.Nodes.getNodeAs<UnaryOperator>("unary-op");
+
+    if (MatchedDecl == NegativeMatchedDecl && -SourceValue == Value) {
+      Range = SourceRange(UnaryOpExpr->getBeginLoc(), UnaryOpExpr->getEndLoc());
+      Location = UnaryOpExpr->getExprLoc();
+      SourceValue = -SourceValue;
+    } else if (MatchedDecl == PositiveMatchedDecl && SourceValue == Value) {
+      Range = SourceRange(UnaryOpExpr->getBeginLoc(), UnaryOpExpr->getEndLoc());
+      Location = UnaryOpExpr->getExprLoc();
+    } else if (MatchedDecl != BareMatchedDecl || SourceValue != Value) {
+      return;
+    }
+
+    diag(Location,
+         "the constant '%0' is being utilized; consider using '%1' instead")
+        << SourceValue << Replacement
+        << FixItHint::CreateReplacement(Range, Replacement)
+        << Inserter.createIncludeInsertion(
+               Result.SourceManager->getFileID(Location), "<limits>");
+  };
+
+  for (const auto &[Value, Replacement] : SignedConstants)
+    Fixer(MatchedIntegerConstant.getSExtValue(), Value, Replacement);
+
+  for (const auto &[Value, Replacement] : UnsignedConstants)
+    Fixer(MatchedIntegerConstant.getZExtValue(), Value, Replacement);
+}
+
+} // namespace clang::tidy::readability

clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.h

@@ -0,0 +1,38 @@
+//===--- UseNumericLimitsCheck.h - clang-tidy -------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USENUMERICLIMITSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USENUMERICLIMITSCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+
+namespace clang::tidy::readability {
+
+/// Finds certain integer literals and suggests replacing them with equivalent
+/// ``std::numeric_limits`` calls.
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability/use-numeric-limits.html
+class UseNumericLimitsCheck : public ClangTidyCheck {
+public:
+  UseNumericLimitsCheck(StringRef Name, ClangTidyContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+                           Preprocessor *ModuleExpanderPP) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const llvm::SmallVector<std::pair<int64_t, std::string>> SignedConstants;
+  const llvm::SmallVector<std::pair<uint64_t, std::string>> UnsignedConstants;
+  utils::IncludeInserter Inserter;
+};
+
+} // namespace clang::tidy::readability
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USENUMERICLIMITSCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -154,6 +154,12 @@ New checks
   Finds potentially erroneous calls to ``reset`` method on smart pointers when
   the pointee type also has a ``reset`` method.
 
+- New :doc:`readability-use-numeric-limits
+  <clang-tidy/checks/readability/use-numeric-limits>` check.
+
+  Finds certain integer literals and suggests replacing them with equivalent
+  ``std::numeric_limits`` calls.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -409,6 +409,7 @@ Clang-Tidy Checks
    :doc:`readability-uniqueptr-delete-release <readability/uniqueptr-delete-release>`, "Yes"
    :doc:`readability-uppercase-literal-suffix <readability/uppercase-literal-suffix>`, "Yes"
    :doc:`readability-use-anyofallof <readability/use-anyofallof>`,
+   :doc:`readability-use-numeric-limits <readability/use-numeric-limits>`, "Yes"
    :doc:`readability-use-std-min-max <readability/use-std-min-max>`, "Yes"
    :doc:`zircon-temporary-objects <zircon/temporary-objects>`,
 

clang-tools-extra/docs/clang-tidy/checks/readability/use-numeric-limits.rst

@@ -0,0 +1,31 @@
+.. title:: clang-tidy - readability-use-numeric-limits
+
+readability-use-numeric-limits
+==============================
+
+Finds certain integer literals and suggests replacing them with equivalent
+``std::numeric_limits`` calls.
+
+Before:
+
+.. code-block:: c++
+
+  void foo() {
+    int32_t a = 2147483647;
+  }
+
+After:
+
+.. code-block:: c++
+
+  void foo() {
+    int32_t a = std::numeric_limits<int32_t>::max();
+  }
+
+Options
+-------
+
+.. option:: IncludeStyle
+
+   A string specifying which include-style is used, `llvm` or `google`. Default
+   is `llvm`.

clang-tools-extra/test/clang-tidy/checkers/readability/use-numeric-limits.cpp

@@ -0,0 +1,100 @@
+// RUN: %check_clang_tidy %s readability-use-numeric-limits %t
+// CHECK-FIXES: #include <limits>
+
+using int8_t = signed char;
+using int16_t = short;
+using int32_t = int;
+using int64_t = long long;
+using uint8_t = unsigned char;
+using uint16_t = unsigned short;
+using uint32_t = unsigned int;
+using uint64_t = unsigned long long;
+
+
+void Invalid() {
+  // CHECK-MESSAGES: :[[@LINE+2]]:14: warning: the constant '-128' is being utilized; consider using 'std::numeric_limits<int8_t>::min()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: int8_t a = std::numeric_limits<int8_t>::min();
+  int8_t a = -128;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:14: warning: the constant '127' is being utilized; consider using 'std::numeric_limits<int8_t>::max()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: int8_t b = std::numeric_limits<int8_t>::max();
+  int8_t b = +127;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:14: warning: the constant '127' is being utilized; consider using 'std::numeric_limits<int8_t>::max()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: int8_t c = std::numeric_limits<int8_t>::max();
+  int8_t c = 127;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:15: warning: the constant '-32768' is being utilized; consider using 'std::numeric_limits<int16_t>::min()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: int16_t d = std::numeric_limits<int16_t>::min();
+  int16_t d = -32768;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:15: warning: the constant '32767' is being utilized; consider using 'std::numeric_limits<int16_t>::max()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: int16_t e = std::numeric_limits<int16_t>::max();
+  int16_t e = +32767;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:15: warning: the constant '32767' is being utilized; consider using 'std::numeric_limits<int16_t>::max()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: int16_t f = std::numeric_limits<int16_t>::max();
+  int16_t f = 32767;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:15: warning: the constant '-2147483648' is being utilized; consider using 'std::numeric_limits<int32_t>::min()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: int32_t g = std::numeric_limits<int32_t>::min();
+  int32_t g = -2147483648;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:15: warning: the constant '2147483647' is being utilized; consider using 'std::numeric_limits<int32_t>::max()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: int32_t h = std::numeric_limits<int32_t>::max();
+  int32_t h = +2147483647;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:15: warning: the constant '2147483647' is being utilized; consider using 'std::numeric_limits<int32_t>::max()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: int32_t i = std::numeric_limits<int32_t>::max();
+  int32_t i = 2147483647;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:15: warning: the constant '-9223372036854775808' is being utilized; consider using 'std::numeric_limits<int64_t>::min()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: int64_t j = std::numeric_limits<int64_t>::min();
+  int64_t j = -9223372036854775808;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:15: warning: the constant '9223372036854775807' is being utilized; consider using 'std::numeric_limits<int64_t>::max()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: int64_t k = std::numeric_limits<int64_t>::max();
+  int64_t k = +9223372036854775807;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:15: warning: the constant '9223372036854775807' is being utilized; consider using 'std::numeric_limits<int64_t>::max()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: int64_t l = std::numeric_limits<int64_t>::max();
+  int64_t l = 9223372036854775807;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:15: warning: the constant '255' is being utilized; consider using 'std::numeric_limits<uint8_t>::max()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: uint8_t m = std::numeric_limits<uint8_t>::max();
+  uint8_t m = 255;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:15: warning: the constant '255' is being utilized; consider using 'std::numeric_limits<uint8_t>::max()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: uint8_t n = std::numeric_limits<uint8_t>::max();
+  uint8_t n = +255;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:16: warning: the constant '65535' is being utilized; consider using 'std::numeric_limits<uint16_t>::max()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: uint16_t o = std::numeric_limits<uint16_t>::max();
+  uint16_t o = 65535;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:16: warning: the constant '65535' is being utilized; consider using 'std::numeric_limits<uint16_t>::max()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: uint16_t p = std::numeric_limits<uint16_t>::max();
+  uint16_t p = +65535;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:16: warning: the constant '4294967295' is being utilized; consider using 'std::numeric_limits<uint32_t>::max()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: uint32_t q = std::numeric_limits<uint32_t>::max();
+  uint32_t q = 4294967295;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:16: warning: the constant '4294967295' is being utilized; consider using 'std::numeric_limits<uint32_t>::max()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: uint32_t r = std::numeric_limits<uint32_t>::max();
+  uint32_t r = +4294967295;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:16: warning: the constant '18446744073709551615' is being utilized; consider using 'std::numeric_limits<uint64_t>::max()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: uint64_t s = std::numeric_limits<uint64_t>::max();
+  uint64_t s = 18446744073709551615;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:16: warning: the constant '18446744073709551615' is being utilized; consider using 'std::numeric_limits<uint64_t>::max()' instead [readability-use-numeric-limits]
+  // CHECK-FIXES: uint64_t t = std::numeric_limits<uint64_t>::max();
+  uint64_t t = +18446744073709551615;
+}
+
+void Valid(){
+  int16_t a = +128;
+
+  int16_t b = -127;
+}