llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang-tidy Author: Katherine Whitlock (stellar-aria) <details> <summary>Changes</summary> 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`. --- Full diff: https://github.com/llvm/llvm-project/pull/127430.diff 8 Files Affected: - (modified) clang-tools-extra/clang-tidy/readability/CMakeLists.txt (+1) - (modified) clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp (+3) - (added) clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp (+135) - (added) clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.h (+41) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5) - (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1) - (added) clang-tools-extra/docs/clang-tidy/checks/readability/use-numeric-limits.rst (+22) - (added) clang-tools-extra/test/clang-tidy/checkers/readability/use-numeric-limits.cpp (+85) ``````````diff diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 8f303c51e1b0d..e06f68bdda73f 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -57,6 +57,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 d61c0ba39658e..054083d25952a 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -60,6 +60,7 @@ #include "UniqueptrDeleteReleaseCheck.h" #include "UppercaseLiteralSuffixCheck.h" #include "UseAnyOfAllOfCheck.h" +#include "UseNumericLimitsCheck.h" #include "UseStdMinMaxCheck.h" namespace clang::tidy { @@ -170,6 +171,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..b26ae008cbdd5 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp @@ -0,0 +1,135 @@ +//===--- 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::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + expr(anyOf(unaryOperator(hasOperatorName("-"), + hasUnaryOperand(integerLiteral().bind( + "negative-integer-literal"))), + unaryOperator(hasOperatorName("+"), + hasUnaryOperand(integerLiteral().bind( + "positive-integer-literal"))), + integerLiteral(unless(hasParent(unaryOperator( + hasAnyOperatorName("-", "+"))))) + .bind("bare-integer-literal"))) + .bind("unary-op-exp"), + 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; + } + + // Only valid if unary operator is present + const UnaryOperator *UnaryOpExpr = + Result.Nodes.getNodeAs<UnaryOperator>("unary-op-exp"); + + const llvm::APInt MatchedIntegerConstant = MatchedDecl->getValue(); + const auto Language = getLangOpts(); + + auto Fixer = [&](auto SourceValue, auto Value, + const std::string &Replacement) { + SourceLocation Location = MatchedDecl->getExprLoc(); + SourceRange Range{MatchedDecl->getBeginLoc(), MatchedDecl->getEndLoc()}; + + 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 " + std::to_string(SourceValue) + + " is being utilized. " + "Consider using " + + Replacement + " instead") + << 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..d7231132ef5dd --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.h @@ -0,0 +1,41 @@ +//===--- 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" +#include <vector> + +namespace clang::tidy::readability { + +/// Replaces certain integer literals with equivalent calls to +/// ``std::numeric_limits``. +/// 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 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; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus11; + } + +private: + const std::vector<std::tuple<int64_t, std::string>> SignedConstants; + const std::vector<std::tuple<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 6b8fe22242417..b2dfb0de6ae68 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -91,6 +91,11 @@ Improvements to clang-tidy New checks ^^^^^^^^^^ +- New :doc:`readability-use-numeric-limits + <clang-tidy/checks/readability/use-numeric-limits>` check to replace certain + integer literals with ``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 7b9b905ef7671..e69ab96cf4bf9 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -403,6 +403,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..ed50d00f1085d --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/use-numeric-limits.rst @@ -0,0 +1,22 @@ +.. title:: clang-tidy - readability-use-numeric-limits + +readability-use-numeric-limits +============================== + +Replaces certain integer literals with ``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(); + } 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..2f1d5190148b0 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/use-numeric-limits.cpp @@ -0,0 +1,85 @@ +// RUN: %check_clang_tidy -std=c++11-or-later %s readability-use-numeric-limits %t +#include <stdint.h> + +void constants() { + // 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 _ = std::numeric_limits<int8_t>::min(); + int8_t _ = -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 _ = std::numeric_limits<int8_t>::max(); + int8_t _ = +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 _ = std::numeric_limits<int8_t>::max(); + int8_t _ = 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 _ = std::numeric_limits<int16_t>::min(); + int16_t _ = -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 _ = std::numeric_limits<int16_t>::max(); + int16_t _ = +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 _ = std::numeric_limits<int16_t>::max(); + int16_t _ = 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 _ = std::numeric_limits<int32_t>::min(); + int32_t _ = -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 _ = std::numeric_limits<int32_t>::max(); + int32_t _ = +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 _ = std::numeric_limits<int32_t>::max(); + int32_t _ = 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 _ = std::numeric_limits<int64_t>::min(); + int64_t _ = -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 _ = std::numeric_limits<int64_t>::max(); + int64_t _ = +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 _ = std::numeric_limits<int64_t>::max(); + int64_t _ = 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 _ = std::numeric_limits<uint8_t>::max(); + uint8_t _ = 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 _ = std::numeric_limits<uint8_t>::max(); + uint8_t _ = +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 _ = std::numeric_limits<uint16_t>::max(); + uint16_t _ = 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 _ = std::numeric_limits<uint16_t>::max(); + uint16_t _ = +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 _ = std::numeric_limits<uint32_t>::max(); + uint32_t _ = 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 _ = std::numeric_limits<uint32_t>::max(); + uint32_t _ = +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 _ = std::numeric_limits<uint64_t>::max(); + uint64_t _ = 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 _ = std::numeric_limits<uint64_t>::max(); + uint64_t _ = +18446744073709551615; +} + `````````` </details> https://github.com/llvm/llvm-project/pull/127430 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
