[clang-tidy] Add new check readability-use-numeric-limits
#127430
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang-tidy Author: Katherine Whitlock (stellar-aria) ChangesThe adds a check that replaces specific numeric literals like Partially addresses #34434, but notably does not handle cases listed in the title post such as Full diff: https://github.com/llvm/llvm-project/pull/127430.diff 8 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index 8f303c51e1b0d..e06f68bdda73f 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -57,6 +57,7 @@ add_clang_library(clangTidyReadabilityModule STATIC
UniqueptrDeleteReleaseCheck.cpp
UppercaseLiteralSuffixCheck.cpp
UseAnyOfAllOfCheck.cpp
+ UseNumericLimitsCheck.cpp
UseStdMinMaxCheck.cpp
LINK_LIBS
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index d61c0ba39658e..054083d25952a 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -60,6 +60,7 @@
#include "UniqueptrDeleteReleaseCheck.h"
#include "UppercaseLiteralSuffixCheck.h"
#include "UseAnyOfAllOfCheck.h"
+#include "UseNumericLimitsCheck.h"
#include "UseStdMinMaxCheck.h"
namespace clang::tidy {
@@ -170,6 +171,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");
}
diff --git a/clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp
new file mode 100644
index 0000000000000..b26ae008cbdd5
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp
@@ -0,0 +1,135 @@
+//===--- 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::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ expr(anyOf(unaryOperator(hasOperatorName("-"),
+ hasUnaryOperand(integerLiteral().bind(
+ "negative-integer-literal"))),
+ unaryOperator(hasOperatorName("+"),
+ hasUnaryOperand(integerLiteral().bind(
+ "positive-integer-literal"))),
+ integerLiteral(unless(hasParent(unaryOperator(
+ hasAnyOperatorName("-", "+")))))
+ .bind("bare-integer-literal")))
+ .bind("unary-op-exp"),
+ 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;
+ }
+
+ // Only valid if unary operator is present
+ const UnaryOperator *UnaryOpExpr =
+ Result.Nodes.getNodeAs<UnaryOperator>("unary-op-exp");
+
+ const llvm::APInt MatchedIntegerConstant = MatchedDecl->getValue();
+ const auto Language = getLangOpts();
+
+ auto Fixer = [&](auto SourceValue, auto Value,
+ const std::string &Replacement) {
+ SourceLocation Location = MatchedDecl->getExprLoc();
+ SourceRange Range{MatchedDecl->getBeginLoc(), MatchedDecl->getEndLoc()};
+
+ 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 " + std::to_string(SourceValue) +
+ " is being utilized. "
+ "Consider using " +
+ Replacement + " instead")
+ << 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
diff --git a/clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.h b/clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.h
new file mode 100644
index 0000000000000..d7231132ef5dd
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.h
@@ -0,0 +1,41 @@
+//===--- 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"
+#include <vector>
+
+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 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;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus11;
+ }
+
+private:
+ const std::vector<std::tuple<int64_t, std::string>> SignedConstants;
+ const std::vector<std::tuple<uint64_t, std::string>> UnsignedConstants;
+ utils::IncludeInserter Inserter;
+};
+
+} // namespace clang::tidy::readability
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USENUMERICLIMITSCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6b8fe22242417..b2dfb0de6ae68 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -91,6 +91,11 @@ Improvements to clang-tidy
New checks
^^^^^^^^^^
+- 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
^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 7b9b905ef7671..e69ab96cf4bf9 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -403,6 +403,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>`,
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/use-numeric-limits.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/use-numeric-limits.rst
new file mode 100644
index 0000000000000..ed50d00f1085d
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/use-numeric-limits.rst
@@ -0,0 +1,22 @@
+.. title:: clang-tidy - readability-use-numeric-limits
+
+readability-use-numeric-limits
+==============================
+
+Replaces certain integer literals with ``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();
+ }
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/use-numeric-limits.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/use-numeric-limits.cpp
new file mode 100644
index 0000000000000..2f1d5190148b0
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/use-numeric-limits.cpp
@@ -0,0 +1,85 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s readability-use-numeric-limits %t
+#include <stdint.h>
+
+void constants() {
+ // 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 _ = std::numeric_limits<int8_t>::min();
+ int8_t _ = -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 _ = std::numeric_limits<int8_t>::max();
+ int8_t _ = +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 _ = std::numeric_limits<int8_t>::max();
+ int8_t _ = 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 _ = std::numeric_limits<int16_t>::min();
+ int16_t _ = -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 _ = std::numeric_limits<int16_t>::max();
+ int16_t _ = +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 _ = std::numeric_limits<int16_t>::max();
+ int16_t _ = 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 _ = std::numeric_limits<int32_t>::min();
+ int32_t _ = -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 _ = std::numeric_limits<int32_t>::max();
+ int32_t _ = +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 _ = std::numeric_limits<int32_t>::max();
+ int32_t _ = 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 _ = std::numeric_limits<int64_t>::min();
+ int64_t _ = -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 _ = std::numeric_limits<int64_t>::max();
+ int64_t _ = +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 _ = std::numeric_limits<int64_t>::max();
+ int64_t _ = 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 _ = std::numeric_limits<uint8_t>::max();
+ uint8_t _ = 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 _ = std::numeric_limits<uint8_t>::max();
+ uint8_t _ = +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 _ = std::numeric_limits<uint16_t>::max();
+ uint16_t _ = 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 _ = std::numeric_limits<uint16_t>::max();
+ uint16_t _ = +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 _ = std::numeric_limits<uint32_t>::max();
+ uint32_t _ = 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 _ = std::numeric_limits<uint32_t>::max();
+ uint32_t _ = +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 _ = std::numeric_limits<uint64_t>::max();
+ uint64_t _ = 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 _ = std::numeric_limits<uint64_t>::max();
+ uint64_t _ = +18446744073709551615;
+}
+
|
EugeneZelenko
requested review from
PiotrZSL,
5chmidti,
carlosgalvezp and
HerrCai0907
clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/readability/use-numeric-limits.cpp
Show resolved
Hide resolved
vbvictor
reviewed
Feb 18, 2025
clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.h
Outdated
Show resolved
Hide resolved
vbvictor
reviewed
Feb 18, 2025
clang-tools-extra/docs/clang-tidy/checks/readability/use-numeric-limits.rst
Show resolved
Hide resolved
vbvictor
reviewed
Feb 18, 2025
clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp
Outdated
Show resolved
Hide resolved
vbvictor
reviewed
Feb 18, 2025
vbvictor
reviewed
Feb 18, 2025
clang-tools-extra/test/clang-tidy/checkers/readability/use-numeric-limits.cpp
Outdated
Show resolved
Hide resolved
HerrCai0907
requested changes
Feb 18, 2025
There was a problem hiding this comment.
The idea of check is good.
But I think the matcher need to refactor a lot.
- Please place value check in ast matcher part, match all integer will cost lots of cpu and ram to store temporary results.
- Maybe consider to write 2 different matcher, one for positive (include e.g. +255 and 255), the other for negative (-128) since they have different value scopes.
Code in clang-tools-extra\clang-tidy\bugprone\StringConstructorCheck.cpp may be helpful since it shows work with numbers and have matcher |
vbvictor
reviewed
Feb 21, 2025
clang-tools-extra/test/clang-tidy/checkers/readability/use-numeric-limits.cpp
Show resolved
Hide resolved
vbvictor
reviewed
Feb 21, 2025
clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp
Outdated
Show resolved
Hide resolved
vbvictor
reviewed
Feb 21, 2025
clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.h
Outdated
Show resolved
Hide resolved
vbvictor
reviewed
Feb 21, 2025
clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.h
Outdated
Show resolved
Hide resolved
vbvictor
reviewed
Feb 21, 2025
clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.h
Outdated
Show resolved
Hide resolved
HerrCai0907
reviewed
Feb 28, 2025
clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp
Outdated
Show resolved
Hide resolved
HerrCai0907
reviewed
Feb 28, 2025
clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp
Outdated
Show resolved
Hide resolved
HerrCai0907
reviewed
Feb 28, 2025
Comment on lines
+126
to
+124
| auto Fixer = [&](auto SourceValue, auto Value, | ||
| const std::string &Replacement) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
HerrCai0907
reviewed
Feb 28, 2025
I feel like they shouldn't be matched, since with opposite signs they're not really within the set of limits? It just feels unintuitive to me for |
HerrCai0907
approved these changes
Mar 2, 2025
clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp
Outdated
Show resolved
Hide resolved
vbvictor
reviewed
Jun 15, 2025
clang-tools-extra/docs/clang-tidy/checks/readability/use-numeric-limits.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp
Outdated
Show resolved
Hide resolved
vbvictor
reviewed
Jun 15, 2025
clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp
Outdated
Show resolved
Hide resolved
Co-authored-by: Baranov Victor <bar.victor.2002@gmail.com>
vbvictor
approved these changes
Jun 15, 2025
clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/readability/use-numeric-limits.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/readability/UseNumericLimitsCheck.h
Outdated
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/readability/use-numeric-limits.cpp
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/readability/use-numeric-limits.cpp
Outdated
Show resolved
Hide resolved
Co-authored-by: Baranov Victor <bar.victor.2002@gmail.com>
|
@stellar-aria Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/11917 Here is the relevant piece of the build log for the reference |
|
Buildbots failures seems irrelevant |
|
Hi, this PR broken a few build bots. Could you take a look? |
|
I'm assuming the MSAN failure is a result of the ASAN failure, since I don't see any reference to UseNumericLimitsCheck in that output. The ASAN output however... Looks like the |
|
One of the logs also suggests |
|
|
vbvictor
added a commit
that referenced
this pull request
Jun 23, 2025
…145355) Reverts #127430 due to stable asan buildbot failures: https://lab.llvm.org/buildbot/#/builders/169
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jun 23, 2025
…c-limits`" (#145355) Reverts llvm/llvm-project#127430 due to stable asan buildbot failures: https://lab.llvm.org/buildbot/#/builders/169
|
Reverted, thank you qinkunbao for the report! |
miguelcsx
pushed a commit
to miguelcsx/llvm-project
that referenced
this pull request
Jun 23, 2025
…lvm#145355) Reverts llvm#127430 due to stable asan buildbot failures: https://lab.llvm.org/buildbot/#/builders/169
Jaddyen
pushed a commit
to Jaddyen/llvm-project
that referenced
this pull request
Jun 23, 2025
) The adds a check that replaces specific numeric literals like `32767` with the equivalent call to `std::numeric_limits` (such as `std::numeric_limits<int16_t>::max())`. Partially addresses llvm#34434, but notably does not handle cases listed in the title post such as `~0` and `-1`.
Jaddyen
pushed a commit
to Jaddyen/llvm-project
that referenced
this pull request
Jun 23, 2025
…lvm#145355) Reverts llvm#127430 due to stable asan buildbot failures: https://lab.llvm.org/buildbot/#/builders/169
|
I see this is reverted now, but while it was in I noticed that it also triggers for .c files. But the suggested fix is for C++ |
Yes, It should only be limited to C++ code for now. I overlooked |
|
For C the macro constants from limits.h can be used: |
anthonyhatran
pushed a commit
to anthonyhatran/llvm-project
that referenced
this pull request
Jun 26, 2025
…lvm#145355) Reverts llvm#127430 due to stable asan buildbot failures: https://lab.llvm.org/buildbot/#/builders/169
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The adds a check that replaces specific numeric literals like
32767with the equivalent call tostd::numeric_limits(such asstd::numeric_limits<int16_t>::max()).Partially addresses #34434, but notably does not handle cases listed in the title post such as
~0and-1.