"readability check to convert numerical constants to std::numeric_limits" #167145
+328
−0
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. |
Member
|
@llvm/pr-subscribers-clang-tools-extra Author: Divvela Rakesh (divvelarakesh1) ChangesFull diff: https://github.com/llvm/llvm-project/pull/167145.diff 3 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/readability/NumericlimitmaxcheckCheck.cpp b/clang-tools-extra/clang-tidy/readability/NumericlimitmaxcheckCheck.cpp
new file mode 100644
index 0000000000000..fd68564fc0c23
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/NumericlimitmaxcheckCheck.cpp
@@ -0,0 +1,192 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 "NumericlimitmaxcheckCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Expr.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+NumericlimitmaxcheckCheck::NumericlimitmaxcheckCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ Inserter(Options.getLocalOrGlobal("IncludeStyle", utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+
+void NumericlimitmaxcheckCheck::registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+ Inserter.registerPreprocessor(PP);
+}
+
+void NumericlimitmaxcheckCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+}
+
+bool NumericlimitmaxcheckCheck::isLanguageVersionSupported(const LangOptions &LangOpts) const {
+ return LangOpts.CPlusPlus;
+}
+
+void NumericlimitmaxcheckCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ // ... inside registerMatchers() ...
+
+ auto NegOneLiteral = integerLiteral(equals(-1));
+ auto ZeroLiteral = integerLiteral(equals(0));
+
+ auto NegOneExpr = anyOf(
+ NegOneLiteral,
+ unaryOperator(hasOperatorName("-"),
+ hasUnaryOperand(integerLiteral(equals(1)))));
+
+ auto BitNotZero = unaryOperator(hasOperatorName("~"),
+ hasUnaryOperand(ZeroLiteral));
+
+ // Match implicit cast of -1 to unsigned
+ auto ImplicitNegOneToUnsigned =
+ implicitCastExpr(
+ hasSourceExpression(ignoringParenImpCasts(anyOf(NegOneExpr, BitNotZero))),
+ hasType(isUnsignedInteger()));
+
+ // Match explicit cast to unsigned of either -1 or ~0
+ auto ExplicitCastOfNegOrBitnot =
+ explicitCastExpr(
+ hasDestinationType(isUnsignedInteger()),
+ hasSourceExpression(ignoringParenImpCasts(anyOf(NegOneExpr, BitNotZero))));
+
+ // Match ~0 with unsigned type
+ auto UnsignedBitNotZero =
+ unaryOperator(
+ hasOperatorName("~"),
+ hasUnaryOperand(ZeroLiteral),
+ hasType(isUnsignedInteger()));
+
+ auto UnsignedLiteralNegOne =
+ integerLiteral(equals(-1), hasType(isUnsignedInteger()));
+
+ // *** ADD THIS NEW MATCHER ***
+ // Matches -1 or ~0 when they are a branch of a ternary operator
+ // that is itself being implicitly cast to unsigned.
+ auto TernaryBranch =
+ expr(anyOf(NegOneExpr, BitNotZero),
+ hasAncestor( // <-- Use hasAncestor to look up the tree
+ conditionalOperator(
+ // Check that the conditional operator itself has an ancestor
+ // which is the implicit cast to unsigned
+ hasAncestor(implicitCastExpr(hasType(isUnsignedInteger()))
+ .bind("outerCast")))))
+ .bind("unsignedMaxExpr");
+
+ // *** MODIFY THIS PART ***
+ auto OldCombined =
+ expr(anyOf(
+ ExplicitCastOfNegOrBitnot,
+ ImplicitNegOneToUnsigned,
+ UnsignedBitNotZero,
+ UnsignedLiteralNegOne
+ )).bind("unsignedMaxExpr");
+
+ Finder->addMatcher(OldCombined, this);
+ Finder->addMatcher(TernaryBranch, this); // Add the new matcher
+}
+
+
+void NumericlimitmaxcheckCheck::check(const MatchFinder::MatchResult &Result) {
+const auto *E = Result.Nodes.getNodeAs<Expr>("unsignedMaxExpr");
+ const auto *OuterCast = Result.Nodes.getNodeAs<CastExpr>("outerCast"); // Get the cast
+
+ if (!E)
+ return;
+
+ ASTContext &Ctx = *Result.Context; // Get context *before* first use
+
+ QualType QT;
+ if (OuterCast) {
+ // This is our new ternary matcher. Get type from the *cast*.
+ QT = OuterCast->getType();
+ } else {
+ // This is the old logic. Get type from the bound expression.
+ QT = E->getType();
+ }
+
+ if (E->getBeginLoc().isInvalid() || E->getBeginLoc().isMacroID())
+ return;
+
+ const SourceManager &SM = Ctx.getSourceManager();
+
+ // *** ADD THIS LOGIC BLOCK ***
+ // This logic prevents double-reporting for the *old* matchers.
+ // We skip it for the new ternary matcher (when OuterCast is not null)
+ // because the ternary matcher binds the *inner* expression, and we
+ // *do* want to report it.
+ if (!OuterCast) {
+ auto Parents = Ctx.getParents(*E); // This fixes the [-Wunused] warning
+ if (!Parents.empty()) {
+ for (const auto &Parent : Parents) {
+ // Check if parent is an explicit cast to unsigned
+ if (const auto *ParentCast = Parent.get<ExplicitCastExpr>()) {
+ if (ParentCast->getType()->isUnsignedIntegerType()) {
+ // Skip this match - the cast itself will be (or was) reported
+ return;
+ }
+ }
+ // Also check if parent is an implicit cast that's part of an explicit cast chain
+ if (const auto *ImplicitCast = Parent.get<ImplicitCastExpr>()) {
+ auto GrandParents = Ctx.getParents(*ImplicitCast);
+ for (const auto &GP : GrandParents) {
+ if (const auto *GPCast = GP.get<ExplicitCastExpr>()) {
+ if (GPCast->getType()->isUnsignedIntegerType()) {
+ return;
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ // ... (rest of parent checking logic) ...
+ // ...
+ // Determine the unsigned destination type
+ // QualType QT = E->getType(); // This line is moved up and modified
+ if (QT.isNull() || !QT->isUnsignedIntegerType())
+ return;
+
+ // Get a printable type string
+ std::string TypeStr = QT.getUnqualifiedType().getAsString();
+ if (const auto *Typedef = QT->getAs<TypedefType>()) {
+ TypeStr = Typedef->getDecl()->getName().str();
+ }
+
+ // Get original source text for diagnostic message
+ StringRef OriginalText =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(E->getSourceRange()),
+ SM, getLangOpts());
+
+ // Build replacement text
+ std::string Replacement = "std::numeric_limits<" + TypeStr + ">::max()";
+
+ // Create diagnostic
+ auto Diag = diag(E->getBeginLoc(),
+ "use 'std::numeric_limits<%0>::max()' instead of '%1'")
+ << TypeStr << OriginalText;
+
+ // Add fix-it hints
+ Diag << FixItHint::CreateReplacement(E->getSourceRange(), Replacement);
+
+ // Add include for <limits>
+ FileID FID = SM.getFileID(E->getBeginLoc());
+ if (auto IncludeHint = Inserter.createIncludeInsertion(FID, "<limits>")) {
+ Diag << *IncludeHint;
+ }
+}
+
+} // namespace clang::tidy::readability
\ No newline at end of file
diff --git a/clang-tools-extra/clang-tidy/readability/NumericlimitmaxcheckCheck.h b/clang-tools-extra/clang-tidy/readability/NumericlimitmaxcheckCheck.h
new file mode 100644
index 0000000000000..1a3ed29e7d5fa
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/NumericlimitmaxcheckCheck.h
@@ -0,0 +1,35 @@
+//===----------------------------------------------------------------------===//
+//
+// 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_NUMERICLIMITMAXCHECKCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_NUMERICLIMITMAXCHECKCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+
+namespace clang::tidy::readability {
+
+/// Suggest replacement of values like -1 / `~0` (when used as unsigned)
+/// with std::numeric_limits<T>::max() for unsigned integer types.
+class NumericlimitmaxcheckCheck : public ClangTidyCheck {
+public:
+ NumericlimitmaxcheckCheck(StringRef Name, ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override;
+
+ void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpander) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+ utils::IncludeInserter Inserter;
+};
+
+} // namespace clang::tidy::readability
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_NUMERICLIMITMAXCHECKCHECK_H
\ No newline at end of file
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/NumericLimitMaxCheck.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/NumericLimitMaxCheck.cpp
new file mode 100644
index 0000000000000..06dce9422fc52
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/NumericLimitMaxCheck.cpp
@@ -0,0 +1,101 @@
+// RUN: %check_clang_tidy %s readability-NumericLimitMaxCheck %t
+
+// Defines a typedef, which the check should respect and use in its fix.
+typedef unsigned long long my_uint_t;
+
+void test_arg(unsigned int);
+
+void test_unsigned_literals() {
+
+ unsigned int a = -1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'std::numeric_limits<unsigned int>::max()' instead of '-1' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned int a = std::numeric_limits<unsigned int>::max();
+
+ unsigned int b = ~0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'std::numeric_limits<unsigned int>::max()' instead of '~0' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned int b = std::numeric_limits<unsigned int>::max();
+
+ unsigned long c = (unsigned long)(-1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: use 'std::numeric_limits<unsigned long>::max()' instead of '(unsigned long)(-1)' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned long c = std::numeric_limits<unsigned long>::max();
+
+ unsigned long d = (unsigned long)(~0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: use 'std::numeric_limits<unsigned long>::max()' instead of '(unsigned long)(~0)' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned long d = std::numeric_limits<unsigned long>::max();
+
+}
+
+void test_comparisons(unsigned x) {
+ if (x == -1) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use 'std::numeric_limits<unsigned int>::max()' instead of '-1' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: if (x == std::numeric_limits<unsigned int>::max()) {
+ ;
+ }
+
+ if (x == (unsigned)(~0));
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use 'std::numeric_limits<unsigned int>::max()' instead of '(unsigned)(~0)' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: if (x == std::numeric_limits<unsigned int>::max());
+
+}
+
+void test_cpp_casts_and_other_types() {
+ unsigned int e = static_cast<unsigned int>(-1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'std::numeric_limits<unsigned int>::max()' instead of 'static_cast<unsigned int>(-1)' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned int e = std::numeric_limits<unsigned int>::max();
+
+ unsigned int f = unsigned(-1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'std::numeric_limits<unsigned int>::max()' instead of 'unsigned(-1)' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned int f = std::numeric_limits<unsigned int>::max();
+
+ unsigned short s = -1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: use 'std::numeric_limits<unsigned short>::max()' instead of '-1' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned short s = std::numeric_limits<unsigned short>::max();
+
+ unsigned char c = ~0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: use 'std::numeric_limits<unsigned char>::max()' instead of '~0' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned char c = std::numeric_limits<unsigned char>::max();
+
+ // Tests that the check correctly uses the typedef'd name "my_uint_t"
+ my_uint_t u = -1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use 'std::numeric_limits<my_uint_t>::max()' instead of '-1' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: my_uint_t u = std::numeric_limits<my_uint_t>::max();
+}
+
+unsigned test_other_contexts(bool x) {
+ // Test function arguments
+ test_arg(-1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use 'std::numeric_limits<unsigned int>::max()' instead of '-1' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: test_arg(std::numeric_limits<unsigned int>::max());
+
+ // Test ternary operator
+ unsigned k = x ? -1 : 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'std::numeric_limits<unsigned int>::max()' instead of '-1' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned k = x ? std::numeric_limits<unsigned int>::max() : 0;
+
+ unsigned k2 = x ? 0 : ~0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use 'std::numeric_limits<unsigned int>::max()' instead of '~0' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned k2 = x ? 0 : std::numeric_limits<unsigned int>::max();
+
+ // Test return values
+ return -1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::numeric_limits<unsigned int>::max()' instead of '-1' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: return std::numeric_limits<unsigned int>::max();
+}
+
+
+#define MY_MAX_MACRO -1
+#define MY_MAX_MACRO_TILDE ~0
+
+void test_no_warning() {
+ int a = -1;
+ unsigned u = 0;
+ int b = ~0;
+ unsigned c = 42;
+
+ // This is (max - 1), not max
+ unsigned d = (unsigned)-2;
+
+ // The check should ignore code from macros
+ unsigned m = MY_MAX_MACRO;
+ unsigned m2 = MY_MAX_MACRO_TILDE;
+}
\ No newline at end of file
|
Member
|
@llvm/pr-subscribers-clang-tidy Author: Divvela Rakesh (divvelarakesh1) ChangesFull diff: https://github.com/llvm/llvm-project/pull/167145.diff 3 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/readability/NumericlimitmaxcheckCheck.cpp b/clang-tools-extra/clang-tidy/readability/NumericlimitmaxcheckCheck.cpp
new file mode 100644
index 0000000000000..fd68564fc0c23
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/NumericlimitmaxcheckCheck.cpp
@@ -0,0 +1,192 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 "NumericlimitmaxcheckCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Expr.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+NumericlimitmaxcheckCheck::NumericlimitmaxcheckCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ Inserter(Options.getLocalOrGlobal("IncludeStyle", utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+
+void NumericlimitmaxcheckCheck::registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+ Inserter.registerPreprocessor(PP);
+}
+
+void NumericlimitmaxcheckCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+}
+
+bool NumericlimitmaxcheckCheck::isLanguageVersionSupported(const LangOptions &LangOpts) const {
+ return LangOpts.CPlusPlus;
+}
+
+void NumericlimitmaxcheckCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ // ... inside registerMatchers() ...
+
+ auto NegOneLiteral = integerLiteral(equals(-1));
+ auto ZeroLiteral = integerLiteral(equals(0));
+
+ auto NegOneExpr = anyOf(
+ NegOneLiteral,
+ unaryOperator(hasOperatorName("-"),
+ hasUnaryOperand(integerLiteral(equals(1)))));
+
+ auto BitNotZero = unaryOperator(hasOperatorName("~"),
+ hasUnaryOperand(ZeroLiteral));
+
+ // Match implicit cast of -1 to unsigned
+ auto ImplicitNegOneToUnsigned =
+ implicitCastExpr(
+ hasSourceExpression(ignoringParenImpCasts(anyOf(NegOneExpr, BitNotZero))),
+ hasType(isUnsignedInteger()));
+
+ // Match explicit cast to unsigned of either -1 or ~0
+ auto ExplicitCastOfNegOrBitnot =
+ explicitCastExpr(
+ hasDestinationType(isUnsignedInteger()),
+ hasSourceExpression(ignoringParenImpCasts(anyOf(NegOneExpr, BitNotZero))));
+
+ // Match ~0 with unsigned type
+ auto UnsignedBitNotZero =
+ unaryOperator(
+ hasOperatorName("~"),
+ hasUnaryOperand(ZeroLiteral),
+ hasType(isUnsignedInteger()));
+
+ auto UnsignedLiteralNegOne =
+ integerLiteral(equals(-1), hasType(isUnsignedInteger()));
+
+ // *** ADD THIS NEW MATCHER ***
+ // Matches -1 or ~0 when they are a branch of a ternary operator
+ // that is itself being implicitly cast to unsigned.
+ auto TernaryBranch =
+ expr(anyOf(NegOneExpr, BitNotZero),
+ hasAncestor( // <-- Use hasAncestor to look up the tree
+ conditionalOperator(
+ // Check that the conditional operator itself has an ancestor
+ // which is the implicit cast to unsigned
+ hasAncestor(implicitCastExpr(hasType(isUnsignedInteger()))
+ .bind("outerCast")))))
+ .bind("unsignedMaxExpr");
+
+ // *** MODIFY THIS PART ***
+ auto OldCombined =
+ expr(anyOf(
+ ExplicitCastOfNegOrBitnot,
+ ImplicitNegOneToUnsigned,
+ UnsignedBitNotZero,
+ UnsignedLiteralNegOne
+ )).bind("unsignedMaxExpr");
+
+ Finder->addMatcher(OldCombined, this);
+ Finder->addMatcher(TernaryBranch, this); // Add the new matcher
+}
+
+
+void NumericlimitmaxcheckCheck::check(const MatchFinder::MatchResult &Result) {
+const auto *E = Result.Nodes.getNodeAs<Expr>("unsignedMaxExpr");
+ const auto *OuterCast = Result.Nodes.getNodeAs<CastExpr>("outerCast"); // Get the cast
+
+ if (!E)
+ return;
+
+ ASTContext &Ctx = *Result.Context; // Get context *before* first use
+
+ QualType QT;
+ if (OuterCast) {
+ // This is our new ternary matcher. Get type from the *cast*.
+ QT = OuterCast->getType();
+ } else {
+ // This is the old logic. Get type from the bound expression.
+ QT = E->getType();
+ }
+
+ if (E->getBeginLoc().isInvalid() || E->getBeginLoc().isMacroID())
+ return;
+
+ const SourceManager &SM = Ctx.getSourceManager();
+
+ // *** ADD THIS LOGIC BLOCK ***
+ // This logic prevents double-reporting for the *old* matchers.
+ // We skip it for the new ternary matcher (when OuterCast is not null)
+ // because the ternary matcher binds the *inner* expression, and we
+ // *do* want to report it.
+ if (!OuterCast) {
+ auto Parents = Ctx.getParents(*E); // This fixes the [-Wunused] warning
+ if (!Parents.empty()) {
+ for (const auto &Parent : Parents) {
+ // Check if parent is an explicit cast to unsigned
+ if (const auto *ParentCast = Parent.get<ExplicitCastExpr>()) {
+ if (ParentCast->getType()->isUnsignedIntegerType()) {
+ // Skip this match - the cast itself will be (or was) reported
+ return;
+ }
+ }
+ // Also check if parent is an implicit cast that's part of an explicit cast chain
+ if (const auto *ImplicitCast = Parent.get<ImplicitCastExpr>()) {
+ auto GrandParents = Ctx.getParents(*ImplicitCast);
+ for (const auto &GP : GrandParents) {
+ if (const auto *GPCast = GP.get<ExplicitCastExpr>()) {
+ if (GPCast->getType()->isUnsignedIntegerType()) {
+ return;
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ // ... (rest of parent checking logic) ...
+ // ...
+ // Determine the unsigned destination type
+ // QualType QT = E->getType(); // This line is moved up and modified
+ if (QT.isNull() || !QT->isUnsignedIntegerType())
+ return;
+
+ // Get a printable type string
+ std::string TypeStr = QT.getUnqualifiedType().getAsString();
+ if (const auto *Typedef = QT->getAs<TypedefType>()) {
+ TypeStr = Typedef->getDecl()->getName().str();
+ }
+
+ // Get original source text for diagnostic message
+ StringRef OriginalText =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(E->getSourceRange()),
+ SM, getLangOpts());
+
+ // Build replacement text
+ std::string Replacement = "std::numeric_limits<" + TypeStr + ">::max()";
+
+ // Create diagnostic
+ auto Diag = diag(E->getBeginLoc(),
+ "use 'std::numeric_limits<%0>::max()' instead of '%1'")
+ << TypeStr << OriginalText;
+
+ // Add fix-it hints
+ Diag << FixItHint::CreateReplacement(E->getSourceRange(), Replacement);
+
+ // Add include for <limits>
+ FileID FID = SM.getFileID(E->getBeginLoc());
+ if (auto IncludeHint = Inserter.createIncludeInsertion(FID, "<limits>")) {
+ Diag << *IncludeHint;
+ }
+}
+
+} // namespace clang::tidy::readability
\ No newline at end of file
diff --git a/clang-tools-extra/clang-tidy/readability/NumericlimitmaxcheckCheck.h b/clang-tools-extra/clang-tidy/readability/NumericlimitmaxcheckCheck.h
new file mode 100644
index 0000000000000..1a3ed29e7d5fa
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/NumericlimitmaxcheckCheck.h
@@ -0,0 +1,35 @@
+//===----------------------------------------------------------------------===//
+//
+// 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_NUMERICLIMITMAXCHECKCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_NUMERICLIMITMAXCHECKCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+
+namespace clang::tidy::readability {
+
+/// Suggest replacement of values like -1 / `~0` (when used as unsigned)
+/// with std::numeric_limits<T>::max() for unsigned integer types.
+class NumericlimitmaxcheckCheck : public ClangTidyCheck {
+public:
+ NumericlimitmaxcheckCheck(StringRef Name, ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override;
+
+ void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpander) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+ utils::IncludeInserter Inserter;
+};
+
+} // namespace clang::tidy::readability
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_NUMERICLIMITMAXCHECKCHECK_H
\ No newline at end of file
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/NumericLimitMaxCheck.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/NumericLimitMaxCheck.cpp
new file mode 100644
index 0000000000000..06dce9422fc52
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/NumericLimitMaxCheck.cpp
@@ -0,0 +1,101 @@
+// RUN: %check_clang_tidy %s readability-NumericLimitMaxCheck %t
+
+// Defines a typedef, which the check should respect and use in its fix.
+typedef unsigned long long my_uint_t;
+
+void test_arg(unsigned int);
+
+void test_unsigned_literals() {
+
+ unsigned int a = -1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'std::numeric_limits<unsigned int>::max()' instead of '-1' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned int a = std::numeric_limits<unsigned int>::max();
+
+ unsigned int b = ~0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'std::numeric_limits<unsigned int>::max()' instead of '~0' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned int b = std::numeric_limits<unsigned int>::max();
+
+ unsigned long c = (unsigned long)(-1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: use 'std::numeric_limits<unsigned long>::max()' instead of '(unsigned long)(-1)' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned long c = std::numeric_limits<unsigned long>::max();
+
+ unsigned long d = (unsigned long)(~0);
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: use 'std::numeric_limits<unsigned long>::max()' instead of '(unsigned long)(~0)' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned long d = std::numeric_limits<unsigned long>::max();
+
+}
+
+void test_comparisons(unsigned x) {
+ if (x == -1) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use 'std::numeric_limits<unsigned int>::max()' instead of '-1' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: if (x == std::numeric_limits<unsigned int>::max()) {
+ ;
+ }
+
+ if (x == (unsigned)(~0));
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use 'std::numeric_limits<unsigned int>::max()' instead of '(unsigned)(~0)' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: if (x == std::numeric_limits<unsigned int>::max());
+
+}
+
+void test_cpp_casts_and_other_types() {
+ unsigned int e = static_cast<unsigned int>(-1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'std::numeric_limits<unsigned int>::max()' instead of 'static_cast<unsigned int>(-1)' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned int e = std::numeric_limits<unsigned int>::max();
+
+ unsigned int f = unsigned(-1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'std::numeric_limits<unsigned int>::max()' instead of 'unsigned(-1)' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned int f = std::numeric_limits<unsigned int>::max();
+
+ unsigned short s = -1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: use 'std::numeric_limits<unsigned short>::max()' instead of '-1' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned short s = std::numeric_limits<unsigned short>::max();
+
+ unsigned char c = ~0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: use 'std::numeric_limits<unsigned char>::max()' instead of '~0' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned char c = std::numeric_limits<unsigned char>::max();
+
+ // Tests that the check correctly uses the typedef'd name "my_uint_t"
+ my_uint_t u = -1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use 'std::numeric_limits<my_uint_t>::max()' instead of '-1' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: my_uint_t u = std::numeric_limits<my_uint_t>::max();
+}
+
+unsigned test_other_contexts(bool x) {
+ // Test function arguments
+ test_arg(-1);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use 'std::numeric_limits<unsigned int>::max()' instead of '-1' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: test_arg(std::numeric_limits<unsigned int>::max());
+
+ // Test ternary operator
+ unsigned k = x ? -1 : 0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'std::numeric_limits<unsigned int>::max()' instead of '-1' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned k = x ? std::numeric_limits<unsigned int>::max() : 0;
+
+ unsigned k2 = x ? 0 : ~0;
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use 'std::numeric_limits<unsigned int>::max()' instead of '~0' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: unsigned k2 = x ? 0 : std::numeric_limits<unsigned int>::max();
+
+ // Test return values
+ return -1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::numeric_limits<unsigned int>::max()' instead of '-1' [readability-NumericLimitMaxCheck]
+ // CHECK-FIXES: return std::numeric_limits<unsigned int>::max();
+}
+
+
+#define MY_MAX_MACRO -1
+#define MY_MAX_MACRO_TILDE ~0
+
+void test_no_warning() {
+ int a = -1;
+ unsigned u = 0;
+ int b = ~0;
+ unsigned c = 42;
+
+ // This is (max - 1), not max
+ unsigned d = (unsigned)-2;
+
+ // The check should ignore code from macros
+ unsigned m = MY_MAX_MACRO;
+ unsigned m2 = MY_MAX_MACRO_TILDE;
+}
\ No newline at end of file
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
No description provided.