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..7b16a8b82497d --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/NumericlimitmaxcheckCheck.cpp @@ -0,0 +1,176 @@ +//===----------------------------------------------------------------------===// +// +// 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; + + 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())); + + // To handle ternary branch case + auto TernaryBranch = + expr(anyOf(NegOneExpr, BitNotZero), + hasAncestor(conditionalOperator( + hasAncestor(implicitCastExpr(hasType(isUnsignedInteger() + )) + .bind("outerCast"))))) + .bind("unsignedMaxExpr"); + + auto Combined = + expr(anyOf( + ExplicitCastOfNegOrBitnot, + ImplicitNegOneToUnsigned, + UnsignedBitNotZero, + UnsignedLiteralNegOne + )).bind("unsignedMaxExpr"); + + Finder->addMatcher(Combined, this); + Finder->addMatcher(TernaryBranch, this); +} + + +void NumericlimitmaxcheckCheck::check(const MatchFinder::MatchResult &Result) { + const auto *E = Result.Nodes.getNodeAs("unsignedMaxExpr"); + const auto *OuterCast = Result.Nodes.getNodeAs("outerCast"); + + if (!E) + return; + + ASTContext &Ctx = *Result.Context; // Get context before first use + + QualType QT; + if (OuterCast) { + // This is ternary matcher. Get type from the cast. + QT = OuterCast->getType(); + } else { + // Get type from the bound expression. + QT = E->getType(); + } + + if (E->getBeginLoc().isInvalid() || E->getBeginLoc().isMacroID()) + return; + + const SourceManager &SM = Ctx.getSourceManager(); + + if (!OuterCast) { + auto Parents = Ctx.getParents(*E); + if (!Parents.empty()) { + for (const auto &Parent : Parents) { + // Check if parent is an explicit cast to unsigned + if (const auto *ParentCast = Parent.get()) { + if (ParentCast->getType()->isUnsignedIntegerType()) { + // Skip this match, avoids double reporting + return; + } + } + // Also check if parent is an implicit cast that's part of an explicit cast chain + if (const auto *ImplicitCast = Parent.get()) { + auto GrandParents = Ctx.getParents(*ImplicitCast); + for (const auto &GP : GrandParents) { + if (const auto *GPCast = GP.get()) { + if (GPCast->getType()->isUnsignedIntegerType()) { + return; + } + } + } + } + } + } + } + + if (QT.isNull() || !QT->isUnsignedIntegerType()) + return; + + std::string TypeStr = QT.getUnqualifiedType().getAsString(); + if (const auto *Typedef = QT->getAs()) { + TypeStr = Typedef->getDecl()->getName().str(); + } + + // Get original source text for diagnostic message + StringRef OriginalText = + Lexer::getSourceText(CharSourceRange::getTokenRange(E->getSourceRange()), + SM, getLangOpts()); + + // Suggestion to the user regarding the usage of unsigned ~0 or -1 + std::string Replacement = "std::numeric_limits<" + TypeStr + ">::max()"; + + //gives warning message if unsigned ~0 or -1 are used instead of numeric_limit::max() + auto Diag = diag(E->getBeginLoc(), + "use 'std::numeric_limits<%0>::max()' instead of '%1'") + << TypeStr << OriginalText; + + //suggest hint for fixing it + Diag << FixItHint::CreateReplacement(E->getSourceRange(), Replacement); + + //includes the which contains the numeric_limits::max() + FileID FID = SM.getFileID(E->getBeginLoc()); + if (auto IncludeHint = Inserter.createIncludeInsertion(FID, "")) { + Diag << *IncludeHint; + } +} + +} // namespace clang::tidy::readability 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::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..9230f40f6343b --- /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 + + +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::max()' instead of '-1' [readability-NumericLimitMaxCheck] + // CHECK-FIXES: unsigned int a = std::numeric_limits::max(); + + unsigned int b = ~0; + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'std::numeric_limits::max()' instead of '~0' [readability-NumericLimitMaxCheck] + // CHECK-FIXES: unsigned int b = std::numeric_limits::max(); + + unsigned long c = (unsigned long)(-1); + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: use 'std::numeric_limits::max()' instead of '(unsigned long)(-1)' [readability-NumericLimitMaxCheck] + // CHECK-FIXES: unsigned long c = std::numeric_limits::max(); + + unsigned long d = (unsigned long)(~0); + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: use 'std::numeric_limits::max()' instead of '(unsigned long)(~0)' [readability-NumericLimitMaxCheck] + // CHECK-FIXES: unsigned long d = std::numeric_limits::max(); + +} + +void test_comparisons(unsigned x) { + if (x == -1) { + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use 'std::numeric_limits::max()' instead of '-1' [readability-NumericLimitMaxCheck] + // CHECK-FIXES: if (x == std::numeric_limits::max()) { + ; + } + + if (x == (unsigned)(~0)); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use 'std::numeric_limits::max()' instead of '(unsigned)(~0)' [readability-NumericLimitMaxCheck] + // CHECK-FIXES: if (x == std::numeric_limits::max()); + +} + +void test_cpp_casts_and_other_types() { + unsigned int e = static_cast(-1); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'std::numeric_limits::max()' instead of 'static_cast(-1)' [readability-NumericLimitMaxCheck] + // CHECK-FIXES: unsigned int e = std::numeric_limits::max(); + + unsigned int f = unsigned(-1); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'std::numeric_limits::max()' instead of 'unsigned(-1)' [readability-NumericLimitMaxCheck] + // CHECK-FIXES: unsigned int f = std::numeric_limits::max(); + + unsigned short s = -1; + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: use 'std::numeric_limits::max()' instead of '-1' [readability-NumericLimitMaxCheck] + // CHECK-FIXES: unsigned short s = std::numeric_limits::max(); + + unsigned char c = ~0; + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: use 'std::numeric_limits::max()' instead of '~0' [readability-NumericLimitMaxCheck] + // CHECK-FIXES: unsigned char c = std::numeric_limits::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::max()' instead of '-1' [readability-NumericLimitMaxCheck] + // CHECK-FIXES: my_uint_t u = std::numeric_limits::max(); +} + +unsigned test_other_contexts(bool x) { + // Test function arguments + test_arg(-1); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use 'std::numeric_limits::max()' instead of '-1' [readability-NumericLimitMaxCheck] + // CHECK-FIXES: test_arg(std::numeric_limits::max()); + + // Test ternary operator + unsigned k = x ? -1 : 0; + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'std::numeric_limits::max()' instead of '-1' [readability-NumericLimitMaxCheck] + // CHECK-FIXES: unsigned k = x ? std::numeric_limits::max() : 0; + + unsigned k2 = x ? 0 : ~0; + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use 'std::numeric_limits::max()' instead of '~0' [readability-NumericLimitMaxCheck] + // CHECK-FIXES: unsigned k2 = x ? 0 : std::numeric_limits::max(); + + // Test return values + return -1; + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use 'std::numeric_limits::max()' instead of '-1' [readability-NumericLimitMaxCheck] + // CHECK-FIXES: return std::numeric_limits::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; +}