subject:"\[llvm\] \[clang\-tools\-extra\] \[clang\] Add clang\-tidy check to suggest replacement of conditional statement with std\:\:min\/std\:\:max \(PR #77816\)"

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-02-05 Thread Piotr Zegar via cfe-commits


PiotrZSL wrote:

Unless you want this to be merged as 
"76656712+11ha...@users.noreply.github.com", change your email privacy settings.

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-02-04 Thread via cfe-commits



@@ -0,0 +1,188 @@
+//===--- 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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-28 Thread Piotr Zegar via cfe-commits



@@ -0,0 +1,240 @@
+//===--- 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:
+  case clang::CK_UserDefinedConversion:
+return true;
+  default:
+return false;
+  }
+}
+
+class ExprVisitor : public clang::RecursiveASTVisitor {
+public:
+  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.
+  }
+};
+
+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 SourceManager ,
+ const LangOptions ,
+ StringRef FunctionName, const IfStmt *If) 
{
+  const llvm::StringRef CondLhsStr = Lexer::getSourceText(
+  Source.getExpansionRange(CondLhs->getSourceRange()), Source, LO);
+  const llvm::StringRef CondRhsStr =

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-28 Thread Piotr Zegar via cfe-commits



@@ -0,0 +1,240 @@
+//===--- 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:
+  case clang::CK_UserDefinedConversion:
+return true;
+  default:
+return false;
+  }
+}
+
+class ExprVisitor : public clang::RecursiveASTVisitor {
+public:
+  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.
+  }
+};
+
+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 SourceManager ,
+ const LangOptions ,
+ StringRef FunctionName, const IfStmt *If) 
{
+  const llvm::StringRef CondLhsStr = Lexer::getSourceText(
+  Source.getExpansionRange(CondLhs->getSourceRange()), Source, LO);
+  const llvm::StringRef CondRhsStr =

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-28 Thread Piotr Zegar via cfe-commits



@@ -0,0 +1,240 @@
+//===--- 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:
+  case clang::CK_UserDefinedConversion:
+return true;
+  default:
+return false;
+  }
+}
+
+class ExprVisitor : public clang::RecursiveASTVisitor {
+public:
+  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.
+  }
+};
+
+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 SourceManager ,
+ const LangOptions ,
+ StringRef FunctionName, const IfStmt *If) 
{
+  const llvm::StringRef CondLhsStr = Lexer::getSourceText(
+  Source.getExpansionRange(CondLhs->getSourceRange()), Source, LO);
+  const llvm::StringRef CondRhsStr =

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-28 Thread Piotr Zegar via cfe-commits



@@ -0,0 +1,240 @@
+//===--- 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) {

PiotrZSL wrote:

this function shouldn't exist, as cast kind doesn't mean that this is implicit 
cast.

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-27 Thread Bhuminjay Soni via cfe-commits



@@ -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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-27 Thread Bhuminjay Soni via cfe-commits



@@ -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;

11happy wrote:

![Screenshot from 2024-01-27 
23-13-22](https://github.com/llvm/llvm-project/assets/76656712/09978a9d-e45b-4a24-b296-a3c71ec47640)
In the definition, Context is used in the constructor as if it were a member of 
the class, so if it's removed it will not declared anywhere in the class.


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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-27 Thread Félix-Antoine Constantin via cfe-commits


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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-27 Thread Félix-Antoine Constantin via cfe-commits



@@ -0,0 +1,195 @@
+//===--- 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) {

felix642 wrote:

You are missing `CK_UserDefinedConversion`

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-26 Thread Bhuminjay Soni via cfe-commits


11happy wrote:

```
 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;
  }
}
  ```
  these two Ifs can be clubbed into single one , as suggested insome previous 
style suggestions, but I have kept them to ensure better readability. should I 
club them?

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-26 Thread Félix-Antoine Constantin via cfe-commits


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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-26 Thread Félix-Antoine Constantin via cfe-commits



@@ -0,0 +1,195 @@
+//===--- 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) {

felix642 wrote:

You might want to (ignore/filter) some implicit cast types otherwise code like 
this one might give you some unexpected results:

```
#include 

class Foo
{};

int bar()
{
std::vector> a;
unsigned int b = a[0].size();
if(a[0].size() > b)
b = a[0].size();
}
```
https://godbolt.org/z/M67o5hxrG

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-23 Thread Félix-Antoine Constantin via cfe-commits


https://github.com/felix642 requested changes to this pull request.

Looks good! I was able run your code on llvm's project and I'm happy with the 
changes that I see. There is maybe one more use case that we should check 
before I am ready to approve (Except what Piotr has already highlighted) 

The following code generates an invalid fix :

```
#include 

int main()
{
  std::vector a;
  int b;
  if(a[0] < b)
  {
a[0] = b;
  }
```
```
#include 

int main()
{
  std::vector a;
  int b;
  a[0] = std::max(a[0], b);
}
```
The type of `a` and `b` are considered different by your check. `a` is resolved 
has value_type instead of int. 

If we are able to fix that I would be ready accept this PR!

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-22 Thread Piotr Zegar via cfe-commits


https://github.com/PiotrZSL approved this pull request.

Except reported nits, looks fine.

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-22 Thread Piotr Zegar via cfe-commits



@@ -0,0 +1,151 @@
+//===--- 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 , const Expr *CondLhs,

PiotrZSL wrote:

rename all free standing functions into camelCast per codding standard:

_Function names should be verb phrases (as they represent actions), and 
command-like function should be imperative. The name should be camel case, and 
start with a lower case letter (e.g. openFile() or isFoo())._


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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-22 Thread Piotr Zegar via cfe-commits



@@ -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(
+ hasOperatorName("="),
+ hasLHS(expr().bind("AssignLhs")),
+ hasRHS(expr().bind("AssignRhs"))),
+  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 *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");

PiotrZSL wrote:

Move behind line 78, it's not used until that place.

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-22 Thread Piotr Zegar via cfe-commits



@@ -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(
+ hasOperatorName("="),
+ hasLHS(expr().bind("AssignLhs")),
+ hasRHS(expr().bind("AssignRhs"))),
+  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 *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  = *Result.Context;
+  const auto  = Context.getLangOpts();
+  const SourceManager  = Context.getSourceManager();
+
+  const auto *BinaryOp = dyn_cast(If->getCond());

PiotrZSL wrote:

Betetr would be to bind that binary operator to some name, otherwise if some 
implicit cast is in getCond, then this cast may fail and then we going to crash 
at line 98

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-21 Thread Félix-Antoine Constantin via cfe-commits



@@ -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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-20 Thread Piotr Zegar via cfe-commits


https://github.com/PiotrZSL commented:

Just few nits after quick look

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-20 Thread Bhuminjay Soni via cfe-commits



@@ -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),

11happy wrote:

I am sorry , I think I misunderstood you, can you please suggest me where do i 
need to initialise it and how, sorry for such silly doubt. 

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-20 Thread Félix-Antoine Constantin via cfe-commits



@@ -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:

The variable `AlgorithmHeader` is not even present in the snippet that you 
provided. So I highly doubt that it's initialized

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-20 Thread Bhuminjay Soni via cfe-commits



@@ -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),

11happy wrote:

```
void UseStdMinMaxCheck::storeOptions(ClangTidyOptions::OptionMap ) {
  Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
  Options.store(Opts, "AlgorithmHeader",
Options.get("AlgorithmHeader", ""));
}
```
Here, the AlgorithmHeader field is initialized in the member initializer list 
of the constructor using Options.get("AlgorithmHeader", "")
is this not ok??

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-20 Thread Félix-Antoine Constantin via cfe-commits



@@ -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:

You are missing the initialization of the field `AlgorithmHeader`

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-20 Thread Félix-Antoine Constantin via cfe-commits



@@ -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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-19 Thread Julian Schmidt via cfe-commits



@@ -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),
+  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");
+  const auto  = *Result.Context;
+  const auto  = Context.getLangOpts();
+  const SourceManager  = Context.getSourceManager();
+
+  const auto *BinaryOp = dyn_cast(If->getCond());
+  if (!BinaryOp || If->hasElseStorage())
+return;
+
+  if (Compound) {
+if (Compound->size() > 1)
+  return;
+  }
+
+  const SourceLocation IfLocation = If->getIfLoc();
+  const SourceLocation ThenLocation = If->getEndLoc();
+
+  if (IfLocation.isMacroID() || ThenLocation.isMacroID())
+return;
+
+  const auto CreateString = [&](int index) -> llvm::StringRef {
+switch (index) {
+case 1:
+  return Lexer::getSourceText(
+  Source.getExpansionRange(CondLhs->getSourceRange()), Source, LO);
+case 2:
+  return Lexer::getSourceText(
+  Source.getExpansionRange(CondRhs->getSourceRange()), Source, LO);
+case 3:
+  return Lexer::getSourceText(
+  Source.getExpansionRange(AssignLhs->getSourceRange()), Source, LO);
+default:
+  return "Invalid index";
+}
+  };
+
+  const auto CreateReplacement = [&](bool useMax) {
+std::string functionName = useMax ? "std::max" : "std::min";
+return CreateString(/* AssignLhs */ 3).str() + " = " + functionName + "(" +
+   CreateString(/* CondLhs */ 1).str() + ", " +
+   CreateString(/* CondRhs */ 2).str() + ");";
+  };
+  const auto OperatorStr = BinaryOp->getOpcodeStr();
+  if (((BinaryOp->getOpcode() == BO_LT || BinaryOp->getOpcode() == BO_LE) &&
+   (tidy::utils::areStatementsIdentical(CondLhs, AssignRhs, Context) &&
+tidy::utils::areStatementsIdentical(CondRhs, AssignLhs, Context))) ||
+  ((BinaryOp->getOpcode() == BO_GT || BinaryOp->getOpcode() == BO_GE) &&
+   (tidy::utils::areStatementsIdentical(CondLhs, AssignLhs, Context) &&
+tidy::utils::areStatementsIdentical(CondRhs, AssignRhs, Context {
+diag(IfLocation, "use `std::min` instead of `%0`")
+<< OperatorStr
+<< FixItHint::CreateReplacement(SourceRange(IfLocation, ThenLocation),
+CreateReplacement(/*useMax = false*/ 
0))

5chmidti wrote:

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-19 Thread Julian Schmidt via cfe-commits



@@ -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),
+  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");
+  const auto  = *Result.Context;
+  const auto  = Context.getLangOpts();
+  const SourceManager  = Context.getSourceManager();
+
+  const auto *BinaryOp = dyn_cast(If->getCond());
+  if (!BinaryOp || If->hasElseStorage())
+return;
+
+  if (Compound) {
+if (Compound->size() > 1)
+  return;
+  }
+
+  const SourceLocation IfLocation = If->getIfLoc();
+  const SourceLocation ThenLocation = If->getEndLoc();
+
+  if (IfLocation.isMacroID() || ThenLocation.isMacroID())
+return;
+
+  const auto CreateString = [&](int index) -> llvm::StringRef {
+switch (index) {
+case 1:
+  return Lexer::getSourceText(
+  Source.getExpansionRange(CondLhs->getSourceRange()), Source, LO);
+case 2:
+  return Lexer::getSourceText(
+  Source.getExpansionRange(CondRhs->getSourceRange()), Source, LO);
+case 3:
+  return Lexer::getSourceText(
+  Source.getExpansionRange(AssignLhs->getSourceRange()), Source, LO);
+default:
+  return "Invalid index";
+}
+  };
+
+  const auto CreateReplacement = [&](bool useMax) {
+std::string functionName = useMax ? "std::max" : "std::min";
+return CreateString(/* AssignLhs */ 3).str() + " = " + functionName + "(" +
+   CreateString(/* CondLhs */ 1).str() + ", " +
+   CreateString(/* CondRhs */ 2).str() + ");";
+  };

5chmidti wrote:

I think that the `switch` and `index` are hurting readability.
Because you merged the creation for min and max into a single 
`CreateReplacement` lambda I camek up with this:
create the strings for `AssignLhs`, `CondLhs` and `CondRhs` inside the 
`CreateReplacement` lambda.
E.g.:
```c++
  const auto CreateReplacement = [&](bool UseMax) {
const auto *FunctionName = UseMax ? "std::max" : "std::min";
const auto AssignLhsStr = Lexer::...;
const auto CondLhsStr = Lexer::...;
const auto CondRhsStr = Lexer::...;
return (AssignLhsStr + " = " + FunctionName + "(" +
   CondLhsStr + ", " +
   CondRhsStr + ");").str();
  };
```
Then there is only one lambda that handles the whole replacement string 
generation.
Also, `UseMax`,

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-19 Thread Julian Schmidt via cfe-commits


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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-19 Thread Bhuminjay Soni via cfe-commits

11happy wrote:

> I found some things that should be addressed.
> 
> @PiotrZSL you had a comment about "improving readability and promoting the 
> use of standard library functions." in `ReleaseNotes.rst`, I just want to 
> mention that this sentence is also present in the header and check 
> documentation. I don't know if your comment is meant to be scoped to 
> `ReleaseNotes.rst` or this (partial) sentence itself.
> 
> As @felix642 noticed in a review comment, and @PiotrZSL wrote in the linked 
> issue, the check should also detect
> 
> ```c++
> if (value1 < value2)
>   value = value2;
> else
>   value = value1;
> ```
> 
> and `value = (value1 < value2) ? value1 : value2;`.
> 
> I would want to see these cases be handled by this check, the question I have 
> is if this functionality should be added in this pr or in a follow-up pr (the 
> issue should not be closed in this case). Thoughts? IMO, a follow-up pr is 
> fine.

It currently handles this case:
```
if (value1 < value2)
  value = value2;
else
  value = value1;
  ```
  where it is not flagging/warning this type of code as it is two way 
assignment changing both variables to max,basically current implementation 
follows along the line of sinlge check referenced by 
https://pylint.pycqa.org/en/latest/user_guide/messages/refactor/consider-using-max-builtin.html
  also @felix642 suggested yesterday to modify code so that it should not flag 
this type which earlier used to.

  further about below one its a matter of discussion:
  ```
  and `value = (value1 < value2) ? value1 : value2;`.
  ```

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-19 Thread Julian Schmidt via cfe-commits



@@ -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());
+
+  auto CreateMaxReplacement = [&]() {
+return AssignLhsStr.str() + " = std::max(" + CondLhsStr.str() + ", " +
+   CondRhsStr.str() + ");";
+  };
+
+  auto CreateMinReplacement = [&]() {
+return AssignLhsStr.str() + " = std::min(" + CondLhsStr.str() + ", " +
+   CondRhsStr.str() + ");";
+  };
+  const auto OperatorStr = BinaryOp->getOpcodeStr();
+  if (((BinaryOp->getOpcode() == BO_LT || BinaryOp->getOpcode() == BO_LE) &&
+   (tidy::utils::areStatementsIdentical(CondLhs, AssignRhs, Context) &&
+tidy::utils::areStatementsIdentical(CondRhs, AssignLhs, Context))) ||
+  ((BinaryOp->getOpcode() == BO_GT || BinaryOp->getOpcode() == BO_GE) &&
+   (tidy::utils::areStatementsIdentical(CondLhs, AssignLhs, Context) &&
+tidy::utils::areStatementsIdentical(CondRhs, AssignRhs, Context {
+diag(IfLocation, "use `std::min` instead of `%0`")
+<< OperatorStr
+<<

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-19 Thread Julian Schmidt via cfe-commits



@@ -56,13 +56,16 @@
 #include "UniqueptrDeleteReleaseCheck.h"
 #include "UppercaseLiteralSuffixCheck.h"
 #include "UseAnyOfAllOfCheck.h"
+#include "UseStdMinMaxCheck.h"
 
 namespace clang::tidy {
 namespace readability {
 
 class ReadabilityModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories ) override {
+CheckFactories.registerCheck(
+"readability-use-std-min-max");

5chmidti wrote:

Please sort this registration into the list of checks alphabetically (check the 
`CMakeLists.txt`, it is listed in the correct position there, right after 
`use-anyofallof`).

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-19 Thread Julian Schmidt via cfe-commits


https://github.com/5chmidti requested changes to this pull request.

I found some things that should be addressed.

@PiotrZSL you had a comment about "improving readability and promoting the use 
of standard library functions." in `ReleaseNotes.rst`, I just want to mention 
that this sentence is also present in the header and check documentation. I 
don't know if your comment is meant to be scoped to `ReleaseNotes.rst` or this 
(partial) sentence itself. 

As @felix642 noticed in a review comment, and @PiotrZSL wrote in the linked 
issue, the check should also detect
```c++
if (value1 < value2)
  value = value2;
else
  value = value1;
```
and `value = (value1 < value2) ? value1 : value2;`.

I would want to see these cases be handled by this check, the question I have 
is if this functionality should be added in this pr or in a follow-up pr (the 
issue should not be closed in this case). Thoughts? IMO, a follow-up pr is fine.

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Piotr Zegar via cfe-commits



@@ -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");
+  auto  = *Result.Context;
+  const SourceManager  = Context.getSourceManager();
+
+  if (!lhsVar1 || !rhsVar1 || !lhsVar2 || !rhsVar2 || !ifStmt)
+return;
+
+  const auto *binaryOp = dyn_cast(ifStmt->getCond());
+  if (!binaryOp)
+return;
+
+  SourceLocation ifLocation = ifStmt->getIfLoc();
+  SourceLocation thenLocation = ifStmt->getEndLoc();
+
+  auto lhsVar1Str =
+  Lexer::getSourceText(Source.getExpansionRange(lhsVar1->getSourceRange()),
+   Context.getSourceManager(), Context.getLangOpts());
+
+  auto lhsVar2Str =
+  Lexer::getSourceText(Source.getExpansionRange(lhsVar2->getSourceRange()),
+   Context.getSourceManager(), Context.getLangOpts());
+
+  auto rhsVar1Str =
+  Lexer::getSourceText(Source.getExpansionRange(rhsVar1->getSourceRange()),
+   Context.getSourceManager(), Context.getLangOpts());
+
+  auto replacementMax = lhsVar2Str.str() + " = std::max(" + lhsVar1Str.str() +
+", " + rhsVar1Str.str() + ");";
+  auto replacementMin = lhsVar2Str.str() + " = std::min(" + lhsVar1Str.str() +
+", " + rhsVar1Str.str() + ");";
+  auto *operatorStr = binaryOp->getOpcodeStr().data();
+
+  if (binaryOp->getOpcode() == BO_LT || binaryOp->getOpcode() == BO_LE) {
+if (tidy::utils::areStatementsIdentical(lhsVar1, lhsVar2, Context) &&
+tidy::utils::areStatementsIdentical(rhsVar1, rhsVar2, Context)) {
+
+  diag(ifStmt->getIfLoc(), "use `std::max` instead of `%0`")
+  << operatorStr
+  << FixItHint::CreateReplacement(SourceRange(ifLocation, 
thenLocation),
+  std::move(replacementMax))
+  << IncludeInserter.createIncludeInsertion(
+ Source.getFileID(ifStmt->getBeginLoc()), AlgorithmHeader);
+} else if (tidy::utils::areStatementsIdentical(lhsVar1, rhsVar2, Context) 
&&
+   tidy::utils::areStatementsIdentical(rhsVar1, lhsVar2, Context)) 
{
+  diag(ifStmt->getIfLoc(), "use `std::min` instead of `%0`")
+  << operatorStr
+  << FixItHint::CreateReplacement(SourceRange(ifLocation, 
thenLocation),
+  std::move(replacementMin))
+  << IncludeInserter.createIncludeInsertion(
+

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Bhuminjay Soni via cfe-commits


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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Bhuminjay Soni via cfe-commits



@@ -0,0 +1,149 @@
+// 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;
+}
+
+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 `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member2, value1);
+  if (obj.member2 >= value1)
+obj.member2 =

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Félix-Antoine Constantin via cfe-commits



@@ -0,0 +1,149 @@
+// 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;
+}
+
+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 `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member2, value1);
+  if (obj.member2 >= value1)
+obj.member2 =

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Bhuminjay Soni via cfe-commits



@@ -0,0 +1,149 @@
+// 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;
+}
+
+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 `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member2, value1);
+  if (obj.member2 >= value1)
+obj.member2 =

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Bhuminjay Soni via cfe-commits



@@ -0,0 +1,149 @@
+// 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;
+}
+
+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 `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member2, value1);
+  if (obj.member2 >= value1)
+obj.member2 =

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Bhuminjay Soni via cfe-commits



@@ -0,0 +1,149 @@
+// 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;
+}
+
+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 `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member2, value1);
+  if (obj.member2 >= value1)
+obj.member2 =

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Félix-Antoine Constantin via cfe-commits



@@ -0,0 +1,149 @@
+// 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;
+}
+
+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 `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member2, value1);
+  if (obj.member2 >= value1)
+obj.member2 =

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Félix-Antoine Constantin via cfe-commits


https://github.com/felix642 commented:

Hey @11happy I left 2 more comments related to some tests that we should maybe 
add. 

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Julian Schmidt via cfe-commits


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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Julian Schmidt via cfe-commits



@@ -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");
+  auto  = *Result.Context;
+  const SourceManager  = Context.getSourceManager();
+
+  if (!lhsVar1 || !rhsVar1 || !lhsVar2 || !rhsVar2 || !ifStmt)

5chmidti wrote:

This if statement will never fail, because you have a single matcher that 
contains no `optional` matcher, thus all nodes that you have bound will be 
bound.

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Julian Schmidt via cfe-commits



@@ -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(

5chmidti wrote:

IMO this `has` should be `hasCondition` for better readability/stating what you 
mean.

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-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Julian Schmidt via cfe-commits



@@ -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:

I think a clearer name for these bound nodes (and these variables) could be 
`condLhs`, `condRhs`. The names for the assignment are fine, but `assignLhs` 
and `assignRhs` could be used too.

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

45 matches

Mail list logo