Skip to content

[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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/readability/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ add_clang_library(clangTidyReadabilityModule STATIC
UniqueptrDeleteReleaseCheck.cpp
UppercaseLiteralSuffixCheck.cpp
UseAnyOfAllOfCheck.cpp
UseNumericLimitsCheck.cpp
UseStdMinMaxCheck.cpp

LINK_LIBS
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
#include "UniqueptrDeleteReleaseCheck.h"
#include "UppercaseLiteralSuffixCheck.h"
#include "UseAnyOfAllOfCheck.h"
#include "UseNumericLimitsCheck.h"
#include "UseStdMinMaxCheck.h"

namespace clang::tidy {
Expand Down Expand Up @@ -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");
}
Expand Down
160 changes: 160 additions & 0 deletions clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp
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
Copy link
Contributor

Choose a reason for hiding this comment

The 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?
use 2 auto ignore this relationship.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
38 changes: 38 additions & 0 deletions clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.h
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
6 changes: 6 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,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
^^^^^^^^^^^^^^^^^

Expand Down
1 change: 1 addition & 0 deletions clang-tools-extra/docs/clang-tidy/checks/list.rst
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,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>`,

Expand Down
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();
}

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;


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;
}