[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,189 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+namespace {
+
+// Ignore if statements that are inside macros.
+AST_MATCHER(IfStmt, isIfInMacro) {
+ return Node.getIfLoc().isMacroID() || Node.getEndLoc().isMacroID();
+}
+
+} // namespace
+
+static const llvm::StringRef AlgorithmHeader("");
+
+static bool minCondition(const BinaryOperator::Opcode Op, const Expr *CondLhs,
+ const Expr *CondRhs, const Expr *AssignLhs,
+ const Expr *AssignRhs, const ASTContext ) {
+ if ((Op == BO_LT || Op == BO_LE) &&
+ (tidy::utils::areStatementsIdentical(CondLhs, AssignRhs, Context) &&
+ tidy::utils::areStatementsIdentical(CondRhs, AssignLhs, Context)))
+return true;
+
+ if ((Op == BO_GT || Op == BO_GE) &&
+ (tidy::utils::areStatementsIdentical(CondLhs, AssignLhs, Context) &&
+ tidy::utils::areStatementsIdentical(CondRhs, AssignRhs, Context)))
+return true;
+
+ return false;
+}
+
+static bool maxCondition(const BinaryOperator::Opcode Op, const Expr *CondLhs,
+ const Expr *CondRhs, const Expr *AssignLhs,
+ const Expr *AssignRhs, const ASTContext ) {
+ if ((Op == BO_LT || Op == BO_LE) &&
+ (tidy::utils::areStatementsIdentical(CondLhs, AssignLhs, Context) &&
+ tidy::utils::areStatementsIdentical(CondRhs, AssignRhs, Context)))
+return true;
+
+ if ((Op == BO_GT || Op == BO_GE) &&
+ (tidy::utils::areStatementsIdentical(CondLhs, AssignRhs, Context) &&
+ tidy::utils::areStatementsIdentical(CondRhs, AssignLhs, Context)))
+return true;
+
+ return false;
+}
+
+QualType getNonTemplateAlias(QualType QT) {
+ while (true) {
+// cast to a TypedefType
+if (const TypedefType *TT = dyn_cast(QT)) {
+ // check if the typedef is a template and if it is dependent
+ if (!TT->getDecl()->getDescribedTemplate() &&
+ !TT->getDecl()->getDeclContext()->isDependentContext())
+return QT;
+ QT = TT->getDecl()->getUnderlyingType();
+}
+// cast to elaborated type
+else if (const ElaboratedType *ET = dyn_cast(QT)) {
+ QT = ET->getNamedType();
+} else {
+ break;
+}
+ }
+ return QT;
+}
+
+static std::string createReplacement(const Expr *CondLhs, const Expr *CondRhs,
+ const Expr *AssignLhs,
+ const SourceManager ,
+ const LangOptions ,
+ StringRef FunctionName,
+ const BinaryOperator *BO) {
+ const llvm::StringRef CondLhsStr = Lexer::getSourceText(
+ Source.getExpansionRange(CondLhs->getSourceRange()), Source, LO);
+ const llvm::StringRef CondRhsStr = Lexer::getSourceText(
+ Source.getExpansionRange(CondRhs->getSourceRange()), Source, LO);
+ const llvm::StringRef AssignLhsStr = Lexer::getSourceText(
+ Source.getExpansionRange(AssignLhs->getSourceRange()), Source, LO);
+
+ clang::QualType GlobalImplicitCastType;
+ clang::QualType LhsType = CondLhs->getType()
+.getCanonicalType()
+.getNonReferenceType()
+.getUnqualifiedType();
+ clang::QualType RhsType = CondRhs->getType()
+.getCanonicalType()
+.getNonReferenceType()
+.getUnqualifiedType();
+ if (LhsType != RhsType) {
+GlobalImplicitCastType = getNonTemplateAlias(BO->getLHS()->getType());
+ }
+
+ return (AssignLhsStr + " = " + FunctionName +
+ (!GlobalImplicitCastType.isNull()
+ ? "<" + GlobalImplicitCastType.getAsString() + ">("
+ : "(") +
+ CondLhsStr + ", " + CondRhsStr + ");")
+ .str();
+}
+
+UseStdMinMaxCheck::UseStdMinMaxCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+ utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+
+void UseStdMinMaxCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
+}
+
+void UseStdMinMaxCheck::registerMatchers(MatchFinder *Finder) {
+ auto
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/felix642 approved this pull request. LGTM, There is still some work to do to support more cases like ternary operators and if/else statements, but I think we have a decent foundation to work on and we can add these cases later in other PRs. https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,168 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+static const llvm::StringRef AlgorithmHeader("");
+
+static bool minCondition(const BinaryOperator::Opcode Op, const Expr *CondLhs,
+ const Expr *CondRhs, const Expr *AssignLhs,
+ const Expr *AssignRhs, const ASTContext ) {
+ if ((Op == BO_LT || Op == BO_LE) &&
+ (tidy::utils::areStatementsIdentical(CondLhs, AssignRhs, Context) &&
+ tidy::utils::areStatementsIdentical(CondRhs, AssignLhs, Context)))
+return true;
+
+ if ((Op == BO_GT || Op == BO_GE) &&
+ (tidy::utils::areStatementsIdentical(CondLhs, AssignLhs, Context) &&
+ tidy::utils::areStatementsIdentical(CondRhs, AssignRhs, Context)))
+return true;
+
+ return false;
+}
+
+static bool maxCondition(const BinaryOperator::Opcode Op, const Expr *CondLhs,
+ const Expr *CondRhs, const Expr *AssignLhs,
+ const Expr *AssignRhs, const ASTContext ) {
+ if ((Op == BO_LT || Op == BO_LE) &&
+ (tidy::utils::areStatementsIdentical(CondLhs, AssignLhs, Context) &&
+ tidy::utils::areStatementsIdentical(CondRhs, AssignRhs, Context)))
+return true;
+
+ if ((Op == BO_GT || Op == BO_GE) &&
+ (tidy::utils::areStatementsIdentical(CondLhs, AssignRhs, Context) &&
+ tidy::utils::areStatementsIdentical(CondRhs, AssignLhs, Context)))
+return true;
+
+ return false;
+}
+
+static std::string createReplacement(const Expr *CondLhs, const Expr *CondRhs,
+ const Expr *AssignLhs,
+ const SourceManager ,
+ const LangOptions ,
+ StringRef FunctionName,
+ const BinaryOperator *BO) {
+ const llvm::StringRef CondLhsStr = Lexer::getSourceText(
+ Source.getExpansionRange(CondLhs->getSourceRange()), Source, LO);
+ const llvm::StringRef CondRhsStr = Lexer::getSourceText(
+ Source.getExpansionRange(CondRhs->getSourceRange()), Source, LO);
+ const llvm::StringRef AssignLhsStr = Lexer::getSourceText(
+ Source.getExpansionRange(AssignLhs->getSourceRange()), Source, LO);
+
+ clang::QualType GlobalImplicitCastType;
+ if (CondLhs->getType()
+ .getCanonicalType()
+ .getNonReferenceType()
+ .getUnqualifiedType() != CondRhs->getType()
+ .getCanonicalType()
+ .getNonReferenceType()
+ .getUnqualifiedType()) {
+GlobalImplicitCastType = BO->getLHS()->getType();
+ }
+
+ return (AssignLhsStr + " = " + FunctionName +
+ (!GlobalImplicitCastType.isNull()
+ ? "<" + GlobalImplicitCastType.getCanonicalType().getAsString()
+
+ ">("
+ : "(") +
+ CondLhsStr + ", " + CondRhsStr + ");")
+ .str();
+}
+
+UseStdMinMaxCheck::UseStdMinMaxCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+ utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+
+void UseStdMinMaxCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
+}
+
+// Ignore if statements that are inside macros.
+AST_MATCHER(IfStmt, isIfInMacro) {
+ return Node.getIfLoc().isMacroID() || Node.getEndLoc().isMacroID();
+}
+
+// Ignore expressions that are of dependent types.
+AST_MATCHER(Expr, isExprDependentType) {
+ return Node.getType()->isDependentType();
+}
+
+void UseStdMinMaxCheck::registerMatchers(MatchFinder *Finder) {
+ auto AssignOperator =
+ binaryOperator(hasOperatorName("="), hasLHS(expr().bind("AssignLhs")),
+ hasRHS(expr().bind("AssignRhs")));
+ auto BinaryOperator =
+ binaryOperator(
PiotrZSL wrote:
check if using `unless(isTypeDependent())` don't do a trick:
https://clang.llvm.org/docs/LibASTMatchersReference.html
https://github.com/llvm/llvm-project/pull/77816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/PiotrZSL edited https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,189 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include
+using namespace std;
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+class ExprVisitor : public clang::RecursiveASTVisitor {
+public:
+ explicit ExprVisitor(clang::ASTContext *Context) : Context(Context) {}
+ bool visitStmt(const clang::Stmt *S, bool ,
+ clang::QualType ) {
+
+if (isa(S) && !found) {
+ found = true;
+ const clang::ImplicitCastExpr *ImplicitCast =
+ cast(S);
+ GlobalImplicitCastType = ImplicitCast->getType();
+ // Stop visiting children.
+ return false;
+}
+// Continue visiting children.
+for (const clang::Stmt *Child : S->children()) {
+ if (Child) {
+this->visitStmt(Child, found, GlobalImplicitCastType);
+ }
+}
+
+return true; // Continue visiting other nodes.
+ }
+
+private:
+ clang::ASTContext *Context;
felix642 wrote:
Would you be able to tell me where are you using this ASTContext variable? I'm
not seeing any usage and I feel like I'm missing something
https://github.com/llvm/llvm-project/pull/77816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,238 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+static bool isImplicitCastType(const clang::CastKind castKind) {
+ switch (castKind) {
+ case clang::CK_CPointerToObjCPointerCast:
+ case clang::CK_BlockPointerToObjCPointerCast:
+ case clang::CK_BitCast:
+ case clang::CK_AnyPointerToBlockPointerCast:
+ case clang::CK_NullToMemberPointer:
+ case clang::CK_NullToPointer:
+ case clang::CK_IntegralToPointer:
+ case clang::CK_PointerToIntegral:
+ case clang::CK_IntegralCast:
+ case clang::CK_BooleanToSignedIntegral:
+ case clang::CK_IntegralToFloating:
+ case clang::CK_FloatingToIntegral:
+ case clang::CK_FloatingCast:
+ case clang::CK_ObjCObjectLValueCast:
+ case clang::CK_FloatingRealToComplex:
+ case clang::CK_FloatingComplexToReal:
+ case clang::CK_FloatingComplexCast:
+ case clang::CK_FloatingComplexToIntegralComplex:
+ case clang::CK_IntegralRealToComplex:
+ case clang::CK_IntegralComplexToReal:
+ case clang::CK_IntegralComplexCast:
+ case clang::CK_IntegralComplexToFloatingComplex:
+ case clang::CK_FloatingToFixedPoint:
+ case clang::CK_FixedPointToFloating:
+ case clang::CK_FixedPointCast:
+ case clang::CK_FixedPointToIntegral:
+ case clang::CK_IntegralToFixedPoint:
+ case clang::CK_MatrixCast:
+ case clang::CK_PointerToBoolean:
+ case clang::CK_IntegralToBoolean:
+ case clang::CK_FloatingToBoolean:
+ case clang::CK_MemberPointerToBoolean:
+ case clang::CK_FloatingComplexToBoolean:
+ case clang::CK_IntegralComplexToBoolean:
+return true;
+ default:
+return false;
+ }
+}
+
+class ExprVisitor : public clang::RecursiveASTVisitor {
+public:
+ explicit ExprVisitor(clang::ASTContext *Context) : Context(Context) {}
+ bool visitStmt(const clang::Stmt *S, bool ,
+ clang::QualType ) {
+
+if (isa(S) && !found) {
+ const auto CastKind = cast(S)->getCastKind();
+ if (isImplicitCastType(CastKind)) {
+found = true;
+const clang::ImplicitCastExpr *ImplicitCast =
+cast(S);
+GlobalImplicitCastType = ImplicitCast->getType();
+// Stop visiting children.
+return false;
+ }
+}
+// Continue visiting children.
+for (const clang::Stmt *Child : S->children()) {
+ if (Child) {
+this->visitStmt(Child, found, GlobalImplicitCastType);
+ }
+}
+
+return true; // Continue visiting other nodes.
+ }
+
+private:
+ clang::ASTContext *Context;
+};
+
+static const llvm::StringRef AlgorithmHeader("");
+
+static bool minCondition(const BinaryOperator::Opcode Op, const Expr *CondLhs,
+ const Expr *CondRhs, const Expr *AssignLhs,
+ const Expr *AssignRhs, const ASTContext ) {
+ if ((Op == BO_LT || Op == BO_LE) &&
+ (tidy::utils::areStatementsIdentical(CondLhs, AssignRhs, Context) &&
+ tidy::utils::areStatementsIdentical(CondRhs, AssignLhs, Context)))
+return true;
+
+ if ((Op == BO_GT || Op == BO_GE) &&
+ (tidy::utils::areStatementsIdentical(CondLhs, AssignLhs, Context) &&
+ tidy::utils::areStatementsIdentical(CondRhs, AssignRhs, Context)))
+return true;
+
+ return false;
+}
+
+static bool maxCondition(const BinaryOperator::Opcode Op, const Expr *CondLhs,
+ const Expr *CondRhs, const Expr *AssignLhs,
+ const Expr *AssignRhs, const ASTContext ) {
+ if ((Op == BO_LT || Op == BO_LE) &&
+ (tidy::utils::areStatementsIdentical(CondLhs, AssignLhs, Context) &&
+ tidy::utils::areStatementsIdentical(CondRhs, AssignRhs, Context)))
+return true;
+
+ if ((Op == BO_GT || Op == BO_GE) &&
+ (tidy::utils::areStatementsIdentical(CondLhs, AssignRhs, Context) &&
+ tidy::utils::areStatementsIdentical(CondRhs, AssignLhs, Context)))
+return true;
+
+ return false;
+}
+
+static std::string
+createReplacement(const BinaryOperator::Opcode Op, const Expr *CondLhs,
+ const Expr *CondRhs, const Expr *AssignLhs,
+ const ASTContext , const SourceManager ,
+ const LangOptions , StringRef FunctionName,
+ QualType GlobalImplicitCastType) {
+ const llvm::StringRef CondLhsStr = Lexer::getSourceText(
+ Source.getExpansionRange(CondLhs->getSourceRange()), Source, LO);
+ const llvm::StringRef CondRhsStr =
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/felix642 requested changes to this pull request. Added a few more comments regarding some nits but we're almost there https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
felix642 wrote: @11happy I was testing with `d5b8dc25598` but this seems to be working properly now. I would assume that this has been fixed in your latest commits https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/PiotrZSL edited https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,130 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+static const llvm::StringRef AlgorithmHeader("");
+
+UseStdMinMaxCheck::UseStdMinMaxCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+ utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+
+void UseStdMinMaxCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
+}
+
+void UseStdMinMaxCheck::registerMatchers(MatchFinder *Finder) {
+ auto AssignOperator =
+ binaryOperator(hasOperatorName("="), hasLHS(expr().bind("AssignLhs")),
+ hasRHS(expr().bind("AssignRhs")));
+
+ Finder->addMatcher(
+ ifStmt(
+ stmt().bind("if"),
+ unless(hasElse(stmt())), // Ensure `if` has no `else`
+ hasCondition(binaryOperator(hasAnyOperatorName("<", ">", "<=", ">="),
+ hasLHS(expr().bind("CondLhs")),
+ hasRHS(expr().bind("CondRhs")))
+ .bind("binaryOp")),
+ hasThen(
+ anyOf(stmt(AssignOperator),
+compoundStmt(statementCountIs(1), has(AssignOperator,
+ hasParent(stmt(unless(ifStmt(hasElse(
+ equalsBoundNode("if"))), // Ensure `if` has no `else if`
+ this);
+}
+
+void UseStdMinMaxCheck::registerPPCallbacks(const SourceManager ,
+Preprocessor *PP,
+Preprocessor *ModuleExpanderPP) {
+ IncludeInserter.registerPreprocessor(PP);
+}
+
+void UseStdMinMaxCheck::check(const MatchFinder::MatchResult ) {
+ const auto *If = Result.Nodes.getNodeAs("if");
+ const auto = *Result.Context;
+ const auto = Context.getLangOpts();
+ const SourceManager = Context.getSourceManager();
+
+ const SourceLocation IfLocation = If->getIfLoc();
+ const SourceLocation ThenLocation = If->getEndLoc();
+
+ // Ignore Macros
+ if (IfLocation.isMacroID() || ThenLocation.isMacroID())
+return;
+
+ const auto *CondLhs = Result.Nodes.getNodeAs("CondLhs");
+ const auto *CondRhs = Result.Nodes.getNodeAs("CondRhs");
+ const auto *AssignLhs = Result.Nodes.getNodeAs("AssignLhs");
+ const auto *AssignRhs = Result.Nodes.getNodeAs("AssignRhs");
+
+ const auto CreateReplacement = [&](bool UseMax) {
+const auto *FunctionName = UseMax ? "std::max" : "std::min";
+const auto CondLhsStr = Lexer::getSourceText(
+Source.getExpansionRange(CondLhs->getSourceRange()), Source, LO);
+const auto CondRhsStr = Lexer::getSourceText(
+Source.getExpansionRange(CondRhs->getSourceRange()), Source, LO);
+const auto AssignLhsStr = Lexer::getSourceText(
+Source.getExpansionRange(AssignLhs->getSourceRange()), Source, LO);
+return (AssignLhsStr + " = " + FunctionName +
+((CondLhs->getType() != CondRhs->getType())
+ ? "<" + AssignLhs->getType().getAsString() + ">("
+ : "(") +
+CondLhsStr + ", " + CondRhsStr + ");")
+.str();
+ };
+ const auto *BinaryOp = Result.Nodes.getNodeAs("binaryOp");
+ const auto BinaryOpcode = BinaryOp->getOpcode();
+ const auto OperatorStr = BinaryOp->getOpcodeStr();
+ if (((BinaryOpcode == BO_LT || BinaryOpcode == BO_LE) &&
+ (tidy::utils::areStatementsIdentical(CondLhs, AssignRhs, Context) &&
+tidy::utils::areStatementsIdentical(CondRhs, AssignLhs, Context))) ||
+ ((BinaryOpcode == BO_GT || BinaryOpcode == BO_GE) &&
+ (tidy::utils::areStatementsIdentical(CondLhs, AssignLhs, Context) &&
+tidy::utils::areStatementsIdentical(CondRhs, AssignRhs, Context {
PiotrZSL wrote:
consider extracting those 2 big if's conditions into separate static functions,
same for CreateReplacement lambda.
It would make check method smaller and therefore more readable.
https://github.com/llvm/llvm-project/pull/77816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/PiotrZSL edited https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,137 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+static const llvm::StringRef AlgorithmHeader("");
+
+UseStdMinMaxCheck::UseStdMinMaxCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+ utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+
+void UseStdMinMaxCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
+}
+
+void UseStdMinMaxCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ ifStmt(
+ stmt().bind("if"),
+ hasCondition(binaryOperator(hasAnyOperatorName("<", ">", "<=", ">="),
+ hasLHS(expr().bind("CondLhs")),
+ hasRHS(expr().bind("CondRhs",
+ hasThen(anyOf(stmt(binaryOperator(hasOperatorName("="),
+hasLHS(expr().bind("AssignLhs")),
+hasRHS(expr().bind("AssignRhs",
+compoundStmt(statementCountIs(1),
+ has(binaryOperator(
PiotrZSL wrote:
duplicated binaryOperator, maybe extract it:
```
auto AssingOperator = binaryOperator(
hasOperatorName("="),
hasLHS(expr().bind("AssignLhs")),
hasRHS(expr().bind("AssignRhs")));
```
and then use AssingOperator in both places.
https://github.com/llvm/llvm-project/pull/77816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,175 @@
+// RUN: %check_clang_tidy %s readability-use-std-min-max %t
+
+#define MY_MACRO_MIN(a, b) ((a) < (b) ? (a) : (b))
+
+constexpr int myConstexprMin(int a, int b) {
+ return a < b ? a : b;
+}
+
+constexpr int myConstexprMax(int a, int b) {
+ return a > b ? a : b;
+}
+
+#define MY_IF_MACRO(condition, statement) \
+ if (condition) {\
+statement \
+ }
+
+class MyClass {
+public:
+ int member1;
+ int member2;
+};
+
+void foo() {
+ int value1,value2,value3;
+ short value4;
+ MyClass obj;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<`
[readability-use-std-min-max]
+ // CHECK-FIXES: value1 = std::max(value1, value2);
+ if (value1 < value2)
+value1 = value2;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<`
[readability-use-std-min-max]
+ // CHECK-FIXES: value2 = std::min(value1, value2);
+ if (value1 < value2)
+value2 = value1;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>`
[readability-use-std-min-max]
+ // CHECK-FIXES: value2 = std::min(value2, value1);
+ if (value2 > value1)
+value2 = value1;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>`
[readability-use-std-min-max
+ // CHECK-FIXES: value1 = std::max(value2, value1);
+ if (value2 > value1)
+value1 = value2;
+
+ // No suggestion needed here
+ if (value1 == value2)
+value1 = value2;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<`
[readability-use-std-min-max]
+ // CHECK-FIXES: value1 = std::max(value1, value4);
+ if(value1`
[readability-use-std-min-max]
+ // CHECK-FIXES: value1 = std::min(value1, myConstexprMax(value2, value3));
+ if (value1 > myConstexprMax(value2, value3))
+value1 = myConstexprMax(value2, value3);
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=`
[readability-use-std-min-max]
+ // CHECK-FIXES: value2 = std::min(value1, value2);
+ if (value1 <= value2)
+value2 = value1;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=`
[readability-use-std-min-max]
+ // CHECK-FIXES: value1 = std::max(value1, value2);
+ if (value1 <= value2)
+value1 = value2;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=`
[readability-use-std-min-max]
+ // CHECK-FIXES: value1 = std::max(value2, value1);
+ if (value2 >= value1)
+value1 = value2;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=`
[readability-use-std-min-max]
+ // CHECK-FIXES: value2 = std::min(value2, value1);
+ if (value2 >= value1)
+value2 = value1;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<`
[readability-use-std-min-max]
+ // CHECK-FIXES: obj.member1 = std::max(obj.member1, obj.member2);
+ if (obj.member1 < obj.member2)
+obj.member1 = obj.member2;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<`
[readability-use-std-min-max]
+ // CHECK-FIXES: obj.member2 = std::min(obj.member1, obj.member2);
+ if (obj.member1 < obj.member2)
+obj.member2 = obj.member1;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>`
[readability-use-std-min-max]
+ // CHECK-FIXES: obj.member2 = std::min(obj.member2, obj.member1);
+ if (obj.member2 > obj.member1)
+obj.member2 = obj.member1;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>`
[readability-use-std-min-max]
+ // CHECK-FIXES: obj.member1 = std::max(obj.member2, obj.member1);
+ if (obj.member2 > obj.member1)
+obj.member1 = obj.member2;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<`
[readability-use-std-min-max]
+ // CHECK-FIXES: obj.member1 = std::max(obj.member1, value4);
+ if (obj.member1 < value4)
+obj.member1 = value4;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<`
[readability-use-std-min-max]
+ // CHECK-FIXES: value3 = std::min(obj.member1 + value2, value3);
+ if (obj.member1 + value2 < value3)
+value3 = obj.member1 + value2;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=`
[readability-use-std-min-max]
+ // CHECK-FIXES: obj.member2 = std::min(value1, obj.member2);
+ if (value1 <= obj.member2)
+obj.member2 = value1;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=`
[readability-use-std-min-max]
+ // CHECK-FIXES: value1 = std::max(value1, obj.member2);
+ if (value1 <= obj.member2)
+value1 = obj.member2;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=`
[readability-use-std-min-max]
+ // CHECK-FIXES: value1 = std::max(obj.member2, value1);
+ if (obj.member2 >= value1)
+value1 = obj.member2;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=`
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/11happy edited https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,175 @@
+// RUN: %check_clang_tidy %s readability-use-std-min-max %t
+
+#define MY_MACRO_MIN(a, b) ((a) < (b) ? (a) : (b))
+
+constexpr int myConstexprMin(int a, int b) {
+ return a < b ? a : b;
+}
+
+constexpr int myConstexprMax(int a, int b) {
+ return a > b ? a : b;
+}
+
+#define MY_IF_MACRO(condition, statement) \
+ if (condition) {\
+statement \
+ }
+
+class MyClass {
+public:
+ int member1;
+ int member2;
+};
+
+void foo() {
+ int value1,value2,value3;
+ short value4;
+ MyClass obj;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<`
[readability-use-std-min-max]
+ // CHECK-FIXES: value1 = std::max(value1, value2);
+ if (value1 < value2)
+value1 = value2;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<`
[readability-use-std-min-max]
+ // CHECK-FIXES: value2 = std::min(value1, value2);
+ if (value1 < value2)
+value2 = value1;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>`
[readability-use-std-min-max]
+ // CHECK-FIXES: value2 = std::min(value2, value1);
+ if (value2 > value1)
+value2 = value1;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>`
[readability-use-std-min-max
+ // CHECK-FIXES: value1 = std::max(value2, value1);
+ if (value2 > value1)
+value1 = value2;
+
+ // No suggestion needed here
+ if (value1 == value2)
+value1 = value2;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<`
[readability-use-std-min-max]
+ // CHECK-FIXES: value1 = std::max(value1, value4);
+ if(value1`
[readability-use-std-min-max]
+ // CHECK-FIXES: value1 = std::min(value1, myConstexprMax(value2, value3));
+ if (value1 > myConstexprMax(value2, value3))
+value1 = myConstexprMax(value2, value3);
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=`
[readability-use-std-min-max]
+ // CHECK-FIXES: value2 = std::min(value1, value2);
+ if (value1 <= value2)
+value2 = value1;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=`
[readability-use-std-min-max]
+ // CHECK-FIXES: value1 = std::max(value1, value2);
+ if (value1 <= value2)
+value1 = value2;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=`
[readability-use-std-min-max]
+ // CHECK-FIXES: value1 = std::max(value2, value1);
+ if (value2 >= value1)
+value1 = value2;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=`
[readability-use-std-min-max]
+ // CHECK-FIXES: value2 = std::min(value2, value1);
+ if (value2 >= value1)
+value2 = value1;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<`
[readability-use-std-min-max]
+ // CHECK-FIXES: obj.member1 = std::max(obj.member1, obj.member2);
+ if (obj.member1 < obj.member2)
+obj.member1 = obj.member2;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<`
[readability-use-std-min-max]
+ // CHECK-FIXES: obj.member2 = std::min(obj.member1, obj.member2);
+ if (obj.member1 < obj.member2)
+obj.member2 = obj.member1;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>`
[readability-use-std-min-max]
+ // CHECK-FIXES: obj.member2 = std::min(obj.member2, obj.member1);
+ if (obj.member2 > obj.member1)
+obj.member2 = obj.member1;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>`
[readability-use-std-min-max]
+ // CHECK-FIXES: obj.member1 = std::max(obj.member2, obj.member1);
+ if (obj.member2 > obj.member1)
+obj.member1 = obj.member2;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<`
[readability-use-std-min-max]
+ // CHECK-FIXES: obj.member1 = std::max(obj.member1, value4);
+ if (obj.member1 < value4)
+obj.member1 = value4;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<`
[readability-use-std-min-max]
+ // CHECK-FIXES: value3 = std::min(obj.member1 + value2, value3);
+ if (obj.member1 + value2 < value3)
+value3 = obj.member1 + value2;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=`
[readability-use-std-min-max]
+ // CHECK-FIXES: obj.member2 = std::min(value1, obj.member2);
+ if (value1 <= obj.member2)
+obj.member2 = value1;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=`
[readability-use-std-min-max]
+ // CHECK-FIXES: value1 = std::max(value1, obj.member2);
+ if (value1 <= obj.member2)
+value1 = obj.member2;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=`
[readability-use-std-min-max]
+ // CHECK-FIXES: value1 = std::max(obj.member2, value1);
+ if (obj.member2 >= value1)
+value1 = obj.member2;
+
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=`
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/felix642 requested changes to this pull request. https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/5chmidti commented: @11happy, regarding the statement count: The tests work when I add `statementCountIs(1),` right after `compoundStmt(` -> `compoundStmt(statementCountIs(1),`. I removed the bound node, `Compound` and the `Compoind->size() > 1` check. But the tests should still succeed even if you leave the rest as is. https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,139 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+UseStdMinMaxCheck::UseStdMinMaxCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+ utils::IncludeSorter::IS_LLVM),
felix642 wrote:
No worries, in the constructor of the check you should initialize the variable
```
AlgorithmHeader(Options.get(...));
```
and then store the value of the variable in the `Store` method
```
Options.store(Opts, "AlgorithmHeader", AlgorithmHeader);
```
The logic is the same has `IncludeInserter`, the only difference is that you
should use `Options.get` rather than `Options.getLocalOrGlobal`.
(To be transparent, I still think this option shouldn't exist since there is no
reason to overwrite the header)
https://github.com/llvm/llvm-project/pull/77816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/PiotrZSL edited https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/PiotrZSL edited https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,129 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+UseStdMinMaxCheck::UseStdMinMaxCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+ utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+
+void UseStdMinMaxCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
+ Options.store(Opts, "AlgorithmHeader",
+Options.get("AlgorithmHeader", ""));
PiotrZSL wrote:
remove, not needed
https://github.com/llvm/llvm-project/pull/77816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,143 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+UseStdMinMaxCheck::UseStdMinMaxCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+ utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+
+void UseStdMinMaxCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
+ Options.store(Opts, "AlgorithmHeader",
+Options.get("AlgorithmHeader", ""));
+}
+
+void UseStdMinMaxCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ ifStmt(
+ hasCondition(binaryOperator(hasAnyOperatorName("<", ">", "<=", ">="),
+ hasLHS(expr().bind("CondLhs")),
+ hasRHS(expr().bind("CondRhs",
+ hasThen(anyOf(stmt(binaryOperator(hasOperatorName("="),
+hasLHS(expr().bind("AssignLhs")),
+hasRHS(expr().bind("AssignRhs",
+compoundStmt(has(binaryOperator(
+ hasOperatorName("="),
+ hasLHS(expr().bind("AssignLhs")),
+ hasRHS(expr().bind("AssignRhs")
+.bind("compound"
+ .bind("if"),
+ this);
+}
+
+void UseStdMinMaxCheck::registerPPCallbacks(const SourceManager ,
+Preprocessor *PP,
+Preprocessor *ModuleExpanderPP) {
+ IncludeInserter.registerPreprocessor(PP);
+}
+
+void UseStdMinMaxCheck::check(const MatchFinder::MatchResult ) {
+ const auto *CondLhs = Result.Nodes.getNodeAs("CondLhs");
+ const auto *CondRhs = Result.Nodes.getNodeAs("CondRhs");
+ const auto *AssignLhs = Result.Nodes.getNodeAs("AssignLhs");
+ const auto *AssignRhs = Result.Nodes.getNodeAs("AssignRhs");
+ const auto *If = Result.Nodes.getNodeAs("if");
+ const auto *Compound = Result.Nodes.getNodeAs("compound");
+ auto = *Result.Context;
+ const SourceManager = Context.getSourceManager();
+
+ const auto *BinaryOp = dyn_cast(If->getCond());
+ if (!BinaryOp || If->hasElseStorage())
+return;
+
+ if (Compound) {
+int count = 0;
+clang::Stmt::const_child_iterator I = Compound->child_begin();
+clang::Stmt::const_child_iterator E = Compound->child_end();
+for (; I != E; ++I) {
+ count++;
+}
+if (count > 1)
+ return;
+ }
+
+ SourceLocation IfLocation = If->getIfLoc();
+ SourceLocation ThenLocation = If->getEndLoc();
+
+ if (IfLocation.isMacroID() || ThenLocation.isMacroID())
+return;
+
+ const auto CondLhsStr =
+ Lexer::getSourceText(Source.getExpansionRange(CondLhs->getSourceRange()),
+ Context.getSourceManager(), Context.getLangOpts());
+
+ const auto AssignLhsStr = Lexer::getSourceText(
+ Source.getExpansionRange(AssignLhs->getSourceRange()),
+ Context.getSourceManager(), Context.getLangOpts());
+
+ const auto CondRhsStr =
+ Lexer::getSourceText(Source.getExpansionRange(CondRhs->getSourceRange()),
+ Context.getSourceManager(), Context.getLangOpts());
5chmidti wrote:
IMO, these should be lamdbas for performance reasons as well.
https://github.com/llvm/llvm-project/pull/77816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
11happy wrote: **Updates of Recent Commit:** - Incorporated all the naming and formatting suggestions by @5chmidti - Thanks @felix642 for such a great insight on testcases, actually those two you suggested were actually getting flagged when they shouldn't ,added changes so that they don't get flagged. - Added a check which currently dont flag anything related to MACRO functions - Incorporated suggestions by @PiotrZSL Thank you https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -224,6 +224,13 @@ New checks Recommends the smallest possible underlying type for an ``enum`` or ``enum`` class based on the range of its enumerators. +- New :doc:`readability-use-std-min-max + ` check. + + Replaces certain conditional statements with equivalent ``std::min`` or + ``std::max`` expressions, improving readability and promoting the use of + standard library functions. PiotrZSL wrote: "improving readability and promoting the use of standard library functions." that part is not needed here. https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
https://github.com/felix642 edited https://github.com/llvm/llvm-project/pull/77816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)
@@ -0,0 +1,132 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+UseStdMinMaxCheck::UseStdMinMaxCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+ utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()),
+ AlgorithmHeader(Options.get("AlgorithmHeader", "")) {}
+
+void UseStdMinMaxCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
+}
+
+void UseStdMinMaxCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ ifStmt(
+ has(binaryOperator(
+ anyOf(hasOperatorName("<"), hasOperatorName(">"),
+hasOperatorName("<="), hasOperatorName(">=")),
+ hasLHS(expr().bind("lhsVar1")), hasRHS(expr().bind("rhsVar1",
+ hasThen(
+ anyOf(stmt(binaryOperator(hasOperatorName("="),
+hasLHS(expr().bind("lhsVar2")),
+hasRHS(expr().bind("rhsVar2",
+compoundStmt(has(binaryOperator(
+hasOperatorName("="), hasLHS(expr().bind("lhsVar2")),
+hasRHS(expr().bind("rhsVar2"
+ .bind("ifStmt"),
+ this);
+}
+
+void UseStdMinMaxCheck::registerPPCallbacks(const SourceManager ,
+Preprocessor *PP,
+Preprocessor *ModuleExpanderPP) {
+ IncludeInserter.registerPreprocessor(PP);
+}
+
+void UseStdMinMaxCheck::check(const MatchFinder::MatchResult ) {
+ const auto *lhsVar1 = Result.Nodes.getNodeAs("lhsVar1");
+ const auto *rhsVar1 = Result.Nodes.getNodeAs("rhsVar1");
+ const auto *lhsVar2 = Result.Nodes.getNodeAs("lhsVar2");
+ const auto *rhsVar2 = Result.Nodes.getNodeAs("rhsVar2");
+ const auto *ifStmt = Result.Nodes.getNodeAs("ifStmt");
5chmidti wrote:
Please run `clang-tidy --checks="-*,readability-identifier-naming" -fix -p
clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp` to let
clang-tidy change the variable names to `CamelCase` instead of `camelBack`
(this comes from .clang-tidy file at the top of the tree).
There are a few more variables named this way inside this function.
The `IfStmt` won't be changed, maybe change the bound name to `if` and name the
variable `If`?
https://github.com/llvm/llvm-project/pull/77816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
