-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[clang-tidy] Add new check readability-use-numeric-limits
#127430
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
Changes from all commits
4d60b54
c40ef8d
332ceb9
a500961
7fd0a21
ab21a77
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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) { | ||
Comment on lines
+122
to
+123
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer to make it as template function since SourceValue and Value maybe be the same in this cases? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if I use a template parameter list as in auto Fixer = [&]<typename ValueType>(ValueType SourceValue, ValueType Value, ... I get a warning that explicit template parameter lists for lambdas are a C++20 extension? Using a static_assert like static_assert(std::is_same_v<decltype(SourceValue), decltype(Value)>); and/or an enable_if as in auto Fixer = [&](auto SourceValue, auto Value, const std::string &Replacement)
-> std::enable_if_t<
std::is_same_v<decltype(SourceValue), decltype(Value)>> { seems to work, however |
||
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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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(); | ||
} | ||
stellar-aria marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Options | ||
------- | ||
|
||
.. option:: IncludeStyle | ||
|
||
A string specifying which include-style is used, `llvm` or `google`. Default | ||
is `llvm`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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; | ||
|
||
|
||
vbvictor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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; | ||
vbvictor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
stellar-aria marked this conversation as resolved.
Show resolved
Hide resolved
|
||
void Valid(){ | ||
int16_t a = +128; | ||
|
||
int16_t b = -127; | ||
} |
Uh oh!
There was an error while loading. Please reload this page.