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 3 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
163 changes: 163 additions & 0 deletions clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp
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();

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")
<< std::to_string(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 {

/// 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 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
4 changes: 4 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,10 @@ 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 to replace certain
integer literals with ``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
==============================

Replaces certain integer literals with equivalent calls to
``std::numeric_limits<T>::min()`` or ``std::numeric_limits<T>::max()``.

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,90 @@
// RUN: %check_clang_tidy %s readability-use-numeric-limits %t
#include <stdint.h>

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