Skip to content
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;
}