Author: Katherine Whitlock Date: 2025-06-21T21:10:20+03:00 New Revision: e7dd223ec451d4e8e522aa4f2c2baaa3d027f347
URL: https://github.com/llvm/llvm-project/commit/e7dd223ec451d4e8e522aa4f2c2baaa3d027f347 DIFF: https://github.com/llvm/llvm-project/commit/e7dd223ec451d4e8e522aa4f2c2baaa3d027f347.diff LOG: [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`. Added: clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.h clang-tools-extra/docs/clang-tidy/checks/readability/use-numeric-limits.rst clang-tools-extra/test/clang-tidy/checkers/readability/use-numeric-limits.cpp Modified: clang-tools-extra/clang-tidy/readability/CMakeLists.txt clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 4be1a8f831339..2c40a863c5b7d 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/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 diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index d59b0312673b9..dc47c2fb31937 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/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 @@ class ReadabilityModule : public ClangTidyModule { "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"); } diff --git a/clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp new file mode 100644 index 0000000000000..334b69755db29 --- /dev/null +++ b/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 diff --git a/clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.h b/clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.h new file mode 100644 index 0000000000000..0e7e9abb8562c --- /dev/null +++ b/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 diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 4801dab8c1bd5..9dede347b8c97 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/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 ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index ccb78ee45e9c4..57ae7d330a3ce 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/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>`, diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/use-numeric-limits.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/use-numeric-limits.rst new file mode 100644 index 0000000000000..0f6ca9f0cf2c0 --- /dev/null +++ b/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`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/use-numeric-limits.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/use-numeric-limits.cpp new file mode 100644 index 0000000000000..e02d6f1b7126d --- /dev/null +++ b/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; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
