-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[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 3 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,163 @@ | ||
//===--- 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(); | ||
stellar-aria marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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") | ||
<< std::to_string(SourceValue) << Replacement | ||
stellar-aria marked this conversation as resolved.
Show resolved
Hide resolved
|
||
<< FixItHint::CreateReplacement(Range, Replacement) | ||
<< Inserter.createIncludeInsertion( | ||
Result.SourceManager->getFileID(Location), "<limits>"); | ||
}; | ||
|
||
for (const auto &[Value, Replacement] : SignedConstants) { | ||
Fixer(MatchedIntegerConstant.getSExtValue(), Value, Replacement); | ||
} | ||
stellar-aria marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
for (const auto &[Value, Replacement] : UnsignedConstants) { | ||
Fixer(MatchedIntegerConstant.getZExtValue(), Value, Replacement); | ||
} | ||
stellar-aria marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
} // 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 { | ||
|
||
/// Replaces certain integer literals with equivalent calls to | ||
/// ``std::numeric_limits``. | ||
vbvictor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// 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 | ||
============================== | ||
|
||
Replaces certain integer literals with equivalent calls to | ||
``std::numeric_limits<T>::min()`` or ``std::numeric_limits<T>::max()``. | ||
vbvictor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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,90 @@ | ||
// RUN: %check_clang_tidy %s readability-use-numeric-limits %t | ||
#include <stdint.h> | ||
vbvictor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
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.