https://github.com/11happy updated https://github.com/llvm/llvm-project/pull/77816
>From 1883d987b2f83adaef05fdb47ae25c7b06582a64 Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Fri, 12 Jan 2024 00:02:46 +0530 Subject: [PATCH 01/17] Add readability check to suggest replacement of conditional statement with std::min/std::max Signed-off-by: 11happy <soni5ha...@gmail.com> --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../ConditionaltostdminmaxCheck.cpp | 86 +++++++++++++++++++ .../readability/ConditionaltostdminmaxCheck.h | 30 +++++++ .../readability/ReadabilityTidyModule.cpp | 3 + clang-tools-extra/docs/ReleaseNotes.rst | 6 ++ .../docs/clang-tidy/checks/list.rst | 1 + .../readability/ConditionalToStdMinMax.rst | 29 +++++++ .../readability/ConditionalToStdMinMax.cpp | 27 ++++++ 8 files changed, 183 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/ConditionalToStdMinMax.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 408c822b861c5fe..4bc373bb69bb84a 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -8,6 +8,7 @@ add_clang_library(clangTidyReadabilityModule AvoidReturnWithVoidValueCheck.cpp AvoidUnconditionalPreprocessorIfCheck.cpp BracesAroundStatementsCheck.cpp + ConditionaltostdminmaxCheck.cpp ConstReturnTypeCheck.cpp ContainerContainsCheck.cpp ContainerDataPointerCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp new file mode 100644 index 000000000000000..fba8c68f737450f --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp @@ -0,0 +1,86 @@ +//===--- ConditionaltostdminmaxCheck.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 "ConditionaltostdminmaxCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +void ConditionaltostdminmaxCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + ifStmt( + has( + binaryOperator( + anyOf(hasOperatorName("<"),hasOperatorName(">")), + hasLHS(ignoringImpCasts(declRefExpr().bind("lhsVar1"))), + hasRHS(ignoringImpCasts(declRefExpr().bind("rhsVar1"))) + ) + ) + , + hasThen( + stmt( + binaryOperator( + hasOperatorName("="), + hasLHS(ignoringImpCasts(declRefExpr().bind("lhsVar2"))), + hasRHS(ignoringImpCasts(declRefExpr().bind("rhsVar2"))) + ) + ) + ) + ).bind("ifStmt"),this); + + + +} + +void ConditionaltostdminmaxCheck::check(const MatchFinder::MatchResult &Result) { + const DeclRefExpr *lhsVar1 = Result.Nodes.getNodeAs<DeclRefExpr>("lhsVar1"); + const DeclRefExpr *rhsVar1 = Result.Nodes.getNodeAs<DeclRefExpr>("rhsVar1"); + const DeclRefExpr *lhsVar2 = Result.Nodes.getNodeAs<DeclRefExpr>("lhsVar2"); + const DeclRefExpr *rhsVar2 = Result.Nodes.getNodeAs<DeclRefExpr>("rhsVar2"); + const IfStmt *ifStmt = Result.Nodes.getNodeAs<IfStmt>("ifStmt"); + + if (!lhsVar1 || !rhsVar1 || !lhsVar2 || !rhsVar2 || !ifStmt) + return; + + const BinaryOperator *binaryOp = dyn_cast<BinaryOperator>(ifStmt->getCond()); + if (!binaryOp) + return; + + SourceLocation ifLocation = ifStmt->getIfLoc(); + SourceLocation thenLocation = ifStmt->getEndLoc(); + + if(binaryOp->getOpcode() == BO_LT){ + if(lhsVar1->getDecl() == lhsVar2->getDecl() && rhsVar1->getDecl() == rhsVar2->getDecl()){ + diag(ifStmt->getIfLoc(), "use std::max instead of <")<<FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), + lhsVar2->getNameInfo().getAsString() + " = std::max(" + lhsVar1->getNameInfo().getAsString() + ", " + + rhsVar1->getNameInfo().getAsString() + ")"); + } + else if(lhsVar1->getDecl() == rhsVar2->getDecl() && rhsVar1->getDecl() == lhsVar2->getDecl()){ + diag(ifStmt->getIfLoc(), "use std::min instead of <")<<FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), + lhsVar2->getNameInfo().getAsString() + " = std::min(" + lhsVar1->getNameInfo().getAsString() + ", " + + rhsVar1->getNameInfo().getAsString() + ")"); + } + } + else if(binaryOp->getOpcode() == BO_GT){ + if(lhsVar1->getDecl() == lhsVar2->getDecl() && rhsVar1->getDecl() == rhsVar2->getDecl()){ + diag(ifStmt->getIfLoc(), "use std::min instead of >")<<FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), + lhsVar2->getNameInfo().getAsString() + " = std::min(" + lhsVar1->getNameInfo().getAsString() + ", " + + rhsVar1->getNameInfo().getAsString() + ")"); + } + else if(lhsVar1->getDecl() == rhsVar2->getDecl() && rhsVar1->getDecl() == lhsVar2->getDecl()){ + diag(ifStmt->getIfLoc(), "use std::max instead of >")<<FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), + lhsVar2->getNameInfo().getAsString() + " = std::max(" + lhsVar1->getNameInfo().getAsString() + ", " + + rhsVar1->getNameInfo().getAsString() + ")"); + } + } +} + +} // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h b/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h new file mode 100644 index 000000000000000..b7eabcc7d16d416 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h @@ -0,0 +1,30 @@ +//===--- ConditionaltostdminmaxCheck.h - clang-tidy -------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONDITIONALTOSTDMINMAXCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONDITIONALTOSTDMINMAXCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::readability { + +/// FIXME: replaces certain conditional statements with equivalent std::min or std::max expressions, improving readability and promoting the use of standard library functions." +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability/ConditionalToStdMinMax.html +class ConditionaltostdminmaxCheck : public ClangTidyCheck { +public: + ConditionaltostdminmaxCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace clang::tidy::readability + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONDITIONALTOSTDMINMAXCHECK_H diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index 0b0aad7c0dcb36c..63f8f03be6bb916 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -13,6 +13,7 @@ #include "AvoidReturnWithVoidValueCheck.h" #include "AvoidUnconditionalPreprocessorIfCheck.h" #include "BracesAroundStatementsCheck.h" +#include "ConditionaltostdminmaxCheck.h" #include "ConstReturnTypeCheck.h" #include "ContainerContainsCheck.h" #include "ContainerDataPointerCheck.h" @@ -62,6 +63,8 @@ namespace readability { class ReadabilityModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck<ConditionaltostdminmaxCheck>( + "readability-ConditionalToStdMinMax"); CheckFactories.registerCheck<AvoidConstParamsInDecls>( "readability-avoid-const-params-in-decls"); CheckFactories.registerCheck<AvoidReturnWithVoidValueCheck>( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index b4d87e0ed2a67ae..c3fb990ad6a7382 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -224,6 +224,12 @@ New checks Recommends the smallest possible underlying type for an ``enum`` or ``enum`` class based on the range of its enumerators. +- New :doc:`readability-ConditionalToStdMinMax + <clang-tidy/checks/readability/ConditionalToStdMinMax>` check. + + Replaces certain conditional statements with equivalent std::min or std::max expressions, + improving readability and promoting the use of standard library functions. + - New :doc:`readability-reference-to-constructed-temporary <clang-tidy/checks/readability/reference-to-constructed-temporary>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 2f86121ad872998..c2eac55425c69e5 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -336,6 +336,7 @@ Clang-Tidy Checks :doc:`portability-restrict-system-includes <portability/restrict-system-includes>`, "Yes" :doc:`portability-simd-intrinsics <portability/simd-intrinsics>`, :doc:`portability-std-allocator-const <portability/std-allocator-const>`, + :doc:`readability-ConditionalToStdMinMax <readability/ConditionalToStdMinMax>`, "Yes" :doc:`readability-avoid-const-params-in-decls <readability/avoid-const-params-in-decls>`, "Yes" :doc:`readability-avoid-return-with-void-value <readability/avoid-return-with-void-value>`, :doc:`readability-avoid-unconditional-preprocessor-if <readability/avoid-unconditional-preprocessor-if>`, diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst new file mode 100644 index 000000000000000..e95858999b27725 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst @@ -0,0 +1,29 @@ +.. title:: clang-tidy - readability-ConditionalToStdMinMax + +readability-ConditionalToStdMinMax +================================== + +Replaces certain conditional statements with equivalent std::min or std::max expressions, +improving readability and promoting the use of standard library functions. + +Examples: + +Before: + +.. code-block:: c++ + + void foo(){ + int a,b; + if(a < b) + a = b; + } + + +After: + +.. code-block:: c++ + + int main(){ + a = std::max(a, b); + + } \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/ConditionalToStdMinMax.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/ConditionalToStdMinMax.cpp new file mode 100644 index 000000000000000..d29e9aa9fec7080 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/ConditionalToStdMinMax.cpp @@ -0,0 +1,27 @@ +// RUN: %check_clang_tidy %s readability-ConditionalToStdMinMax %t + +void foo() { + int value1,value2; + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::max instead of < [readability-ConditionalToStdMinMax] + if (value1 < value2) + value1 = value2; // CHECK-FIXES: value1 = std::max(value1, value2); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::min instead of < [readability-ConditionalToStdMinMax] + if (value1 < value2) + value2 = value1; // CHECK-FIXES: value2 = std::min(value1, value2); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::min instead of > [readability-ConditionalToStdMinMax] + if (value2 > value1) + value2 = value1; // CHECK-FIXES: value2 = std::min(value2, value1); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::max instead of > [readability-ConditionalToStdMinMax] + if (value2 > value1) + value1 = value2; // CHECK-FIXES: value1 = std::max(value2, value1); + + // No suggestion needed here + if (value1 == value2) + value1 = value2; + + +} \ No newline at end of file >From 89be4baf2591918b11b633d2430050dc41068fde Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Fri, 12 Jan 2024 00:03:16 +0530 Subject: [PATCH 02/17] Formatted the code Signed-off-by: 11happy <soni5ha...@gmail.com> --- .../ConditionaltostdminmaxCheck.cpp | 96 ++++++++++--------- .../readability/ConditionaltostdminmaxCheck.h | 4 +- 2 files changed, 52 insertions(+), 48 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp index fba8c68f737450f..86420765d6f6c61 100644 --- a/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp @@ -16,69 +16,71 @@ namespace clang::tidy::readability { void ConditionaltostdminmaxCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( - ifStmt( - has( - binaryOperator( - anyOf(hasOperatorName("<"),hasOperatorName(">")), - hasLHS(ignoringImpCasts(declRefExpr().bind("lhsVar1"))), - hasRHS(ignoringImpCasts(declRefExpr().bind("rhsVar1"))) - ) - ) - , - hasThen( - stmt( - binaryOperator( - hasOperatorName("="), - hasLHS(ignoringImpCasts(declRefExpr().bind("lhsVar2"))), - hasRHS(ignoringImpCasts(declRefExpr().bind("rhsVar2"))) - ) - ) - ) - ).bind("ifStmt"),this); - - - + ifStmt(has(binaryOperator( + anyOf(hasOperatorName("<"), hasOperatorName(">")), + hasLHS(ignoringImpCasts(declRefExpr().bind("lhsVar1"))), + hasRHS(ignoringImpCasts(declRefExpr().bind("rhsVar1"))))), + hasThen(stmt(binaryOperator( + hasOperatorName("="), + hasLHS(ignoringImpCasts(declRefExpr().bind("lhsVar2"))), + hasRHS(ignoringImpCasts(declRefExpr().bind("rhsVar2"))))))) + .bind("ifStmt"), + this); } -void ConditionaltostdminmaxCheck::check(const MatchFinder::MatchResult &Result) { +void ConditionaltostdminmaxCheck::check( + const MatchFinder::MatchResult &Result) { const DeclRefExpr *lhsVar1 = Result.Nodes.getNodeAs<DeclRefExpr>("lhsVar1"); const DeclRefExpr *rhsVar1 = Result.Nodes.getNodeAs<DeclRefExpr>("rhsVar1"); const DeclRefExpr *lhsVar2 = Result.Nodes.getNodeAs<DeclRefExpr>("lhsVar2"); - const DeclRefExpr *rhsVar2 = Result.Nodes.getNodeAs<DeclRefExpr>("rhsVar2"); + const DeclRefExpr *rhsVar2 = Result.Nodes.getNodeAs<DeclRefExpr>("rhsVar2"); const IfStmt *ifStmt = Result.Nodes.getNodeAs<IfStmt>("ifStmt"); if (!lhsVar1 || !rhsVar1 || !lhsVar2 || !rhsVar2 || !ifStmt) - return; + return; const BinaryOperator *binaryOp = dyn_cast<BinaryOperator>(ifStmt->getCond()); if (!binaryOp) - return; + return; SourceLocation ifLocation = ifStmt->getIfLoc(); SourceLocation thenLocation = ifStmt->getEndLoc(); - if(binaryOp->getOpcode() == BO_LT){ - if(lhsVar1->getDecl() == lhsVar2->getDecl() && rhsVar1->getDecl() == rhsVar2->getDecl()){ - diag(ifStmt->getIfLoc(), "use std::max instead of <")<<FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), - lhsVar2->getNameInfo().getAsString() + " = std::max(" + lhsVar1->getNameInfo().getAsString() + ", " + - rhsVar1->getNameInfo().getAsString() + ")"); - } - else if(lhsVar1->getDecl() == rhsVar2->getDecl() && rhsVar1->getDecl() == lhsVar2->getDecl()){ - diag(ifStmt->getIfLoc(), "use std::min instead of <")<<FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), - lhsVar2->getNameInfo().getAsString() + " = std::min(" + lhsVar1->getNameInfo().getAsString() + ", " + - rhsVar1->getNameInfo().getAsString() + ")"); - } - } - else if(binaryOp->getOpcode() == BO_GT){ - if(lhsVar1->getDecl() == lhsVar2->getDecl() && rhsVar1->getDecl() == rhsVar2->getDecl()){ - diag(ifStmt->getIfLoc(), "use std::min instead of >")<<FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), - lhsVar2->getNameInfo().getAsString() + " = std::min(" + lhsVar1->getNameInfo().getAsString() + ", " + - rhsVar1->getNameInfo().getAsString() + ")"); + if (binaryOp->getOpcode() == BO_LT) { + if (lhsVar1->getDecl() == lhsVar2->getDecl() && + rhsVar1->getDecl() == rhsVar2->getDecl()) { + diag(ifStmt->getIfLoc(), "use std::max instead of <") + << FixItHint::CreateReplacement( + SourceRange(ifLocation, thenLocation), + lhsVar2->getNameInfo().getAsString() + " = std::max(" + + lhsVar1->getNameInfo().getAsString() + ", " + + rhsVar1->getNameInfo().getAsString() + ")"); + } else if (lhsVar1->getDecl() == rhsVar2->getDecl() && + rhsVar1->getDecl() == lhsVar2->getDecl()) { + diag(ifStmt->getIfLoc(), "use std::min instead of <") + << FixItHint::CreateReplacement( + SourceRange(ifLocation, thenLocation), + lhsVar2->getNameInfo().getAsString() + " = std::min(" + + lhsVar1->getNameInfo().getAsString() + ", " + + rhsVar1->getNameInfo().getAsString() + ")"); } - else if(lhsVar1->getDecl() == rhsVar2->getDecl() && rhsVar1->getDecl() == lhsVar2->getDecl()){ - diag(ifStmt->getIfLoc(), "use std::max instead of >")<<FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), - lhsVar2->getNameInfo().getAsString() + " = std::max(" + lhsVar1->getNameInfo().getAsString() + ", " + - rhsVar1->getNameInfo().getAsString() + ")"); + } else if (binaryOp->getOpcode() == BO_GT) { + if (lhsVar1->getDecl() == lhsVar2->getDecl() && + rhsVar1->getDecl() == rhsVar2->getDecl()) { + diag(ifStmt->getIfLoc(), "use std::min instead of >") + << FixItHint::CreateReplacement( + SourceRange(ifLocation, thenLocation), + lhsVar2->getNameInfo().getAsString() + " = std::min(" + + lhsVar1->getNameInfo().getAsString() + ", " + + rhsVar1->getNameInfo().getAsString() + ")"); + } else if (lhsVar1->getDecl() == rhsVar2->getDecl() && + rhsVar1->getDecl() == lhsVar2->getDecl()) { + diag(ifStmt->getIfLoc(), "use std::max instead of >") + << FixItHint::CreateReplacement( + SourceRange(ifLocation, thenLocation), + lhsVar2->getNameInfo().getAsString() + " = std::max(" + + lhsVar1->getNameInfo().getAsString() + ", " + + rhsVar1->getNameInfo().getAsString() + ")"); } } } diff --git a/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h b/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h index b7eabcc7d16d416..02ebed83fed6c82 100644 --- a/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h +++ b/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h @@ -13,7 +13,9 @@ namespace clang::tidy::readability { -/// FIXME: replaces certain conditional statements with equivalent std::min or std::max expressions, improving readability and promoting the use of standard library functions." +/// FIXME: replaces certain conditional statements with equivalent std::min or +/// std::max expressions, improving readability and promoting the use of +/// standard library functions." /// /// For the user-facing documentation see: /// http://clang.llvm.org/extra/clang-tidy/checks/readability/ConditionalToStdMinMax.html >From e1b65a9fb0ea27d62b568d8d3038c0231927cdad Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Fri, 12 Jan 2024 00:15:54 +0530 Subject: [PATCH 03/17] small fix Signed-off-by: 11happy <soni5ha...@gmail.com> --- .../clang-tidy/checks/readability/ConditionalToStdMinMax.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst index e95858999b27725..2b75e78ecb80829 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst @@ -23,7 +23,7 @@ After: .. code-block:: c++ - int main(){ + void foo(){ a = std::max(a, b); } \ No newline at end of file >From 042ddc3d89ebad999378e6bb8ef29e1f05f81745 Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Fri, 12 Jan 2024 15:43:22 +0530 Subject: [PATCH 04/17] Revert "small fix" This reverts commit e1b65a9fb0ea27d62b568d8d3038c0231927cdad. --- .../clang-tidy/checks/readability/ConditionalToStdMinMax.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst index 2b75e78ecb80829..e95858999b27725 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst @@ -23,7 +23,7 @@ After: .. code-block:: c++ - void foo(){ + int main(){ a = std::max(a, b); } \ No newline at end of file >From c17b298185d50095d964d5afd8f79f52281e85f8 Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Fri, 12 Jan 2024 15:43:38 +0530 Subject: [PATCH 05/17] Revert "Formatted the code" This reverts commit 89be4baf2591918b11b633d2430050dc41068fde. --- .../ConditionaltostdminmaxCheck.cpp | 96 +++++++++---------- .../readability/ConditionaltostdminmaxCheck.h | 4 +- 2 files changed, 48 insertions(+), 52 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp index 86420765d6f6c61..fba8c68f737450f 100644 --- a/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp @@ -16,71 +16,69 @@ namespace clang::tidy::readability { void ConditionaltostdminmaxCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( - ifStmt(has(binaryOperator( - anyOf(hasOperatorName("<"), hasOperatorName(">")), - hasLHS(ignoringImpCasts(declRefExpr().bind("lhsVar1"))), - hasRHS(ignoringImpCasts(declRefExpr().bind("rhsVar1"))))), - hasThen(stmt(binaryOperator( - hasOperatorName("="), - hasLHS(ignoringImpCasts(declRefExpr().bind("lhsVar2"))), - hasRHS(ignoringImpCasts(declRefExpr().bind("rhsVar2"))))))) - .bind("ifStmt"), - this); + ifStmt( + has( + binaryOperator( + anyOf(hasOperatorName("<"),hasOperatorName(">")), + hasLHS(ignoringImpCasts(declRefExpr().bind("lhsVar1"))), + hasRHS(ignoringImpCasts(declRefExpr().bind("rhsVar1"))) + ) + ) + , + hasThen( + stmt( + binaryOperator( + hasOperatorName("="), + hasLHS(ignoringImpCasts(declRefExpr().bind("lhsVar2"))), + hasRHS(ignoringImpCasts(declRefExpr().bind("rhsVar2"))) + ) + ) + ) + ).bind("ifStmt"),this); + + + } -void ConditionaltostdminmaxCheck::check( - const MatchFinder::MatchResult &Result) { +void ConditionaltostdminmaxCheck::check(const MatchFinder::MatchResult &Result) { const DeclRefExpr *lhsVar1 = Result.Nodes.getNodeAs<DeclRefExpr>("lhsVar1"); const DeclRefExpr *rhsVar1 = Result.Nodes.getNodeAs<DeclRefExpr>("rhsVar1"); const DeclRefExpr *lhsVar2 = Result.Nodes.getNodeAs<DeclRefExpr>("lhsVar2"); - const DeclRefExpr *rhsVar2 = Result.Nodes.getNodeAs<DeclRefExpr>("rhsVar2"); + const DeclRefExpr *rhsVar2 = Result.Nodes.getNodeAs<DeclRefExpr>("rhsVar2"); const IfStmt *ifStmt = Result.Nodes.getNodeAs<IfStmt>("ifStmt"); if (!lhsVar1 || !rhsVar1 || !lhsVar2 || !rhsVar2 || !ifStmt) - return; + return; const BinaryOperator *binaryOp = dyn_cast<BinaryOperator>(ifStmt->getCond()); if (!binaryOp) - return; + return; SourceLocation ifLocation = ifStmt->getIfLoc(); SourceLocation thenLocation = ifStmt->getEndLoc(); - if (binaryOp->getOpcode() == BO_LT) { - if (lhsVar1->getDecl() == lhsVar2->getDecl() && - rhsVar1->getDecl() == rhsVar2->getDecl()) { - diag(ifStmt->getIfLoc(), "use std::max instead of <") - << FixItHint::CreateReplacement( - SourceRange(ifLocation, thenLocation), - lhsVar2->getNameInfo().getAsString() + " = std::max(" + - lhsVar1->getNameInfo().getAsString() + ", " + - rhsVar1->getNameInfo().getAsString() + ")"); - } else if (lhsVar1->getDecl() == rhsVar2->getDecl() && - rhsVar1->getDecl() == lhsVar2->getDecl()) { - diag(ifStmt->getIfLoc(), "use std::min instead of <") - << FixItHint::CreateReplacement( - SourceRange(ifLocation, thenLocation), - lhsVar2->getNameInfo().getAsString() + " = std::min(" + - lhsVar1->getNameInfo().getAsString() + ", " + - rhsVar1->getNameInfo().getAsString() + ")"); + if(binaryOp->getOpcode() == BO_LT){ + if(lhsVar1->getDecl() == lhsVar2->getDecl() && rhsVar1->getDecl() == rhsVar2->getDecl()){ + diag(ifStmt->getIfLoc(), "use std::max instead of <")<<FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), + lhsVar2->getNameInfo().getAsString() + " = std::max(" + lhsVar1->getNameInfo().getAsString() + ", " + + rhsVar1->getNameInfo().getAsString() + ")"); + } + else if(lhsVar1->getDecl() == rhsVar2->getDecl() && rhsVar1->getDecl() == lhsVar2->getDecl()){ + diag(ifStmt->getIfLoc(), "use std::min instead of <")<<FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), + lhsVar2->getNameInfo().getAsString() + " = std::min(" + lhsVar1->getNameInfo().getAsString() + ", " + + rhsVar1->getNameInfo().getAsString() + ")"); + } + } + else if(binaryOp->getOpcode() == BO_GT){ + if(lhsVar1->getDecl() == lhsVar2->getDecl() && rhsVar1->getDecl() == rhsVar2->getDecl()){ + diag(ifStmt->getIfLoc(), "use std::min instead of >")<<FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), + lhsVar2->getNameInfo().getAsString() + " = std::min(" + lhsVar1->getNameInfo().getAsString() + ", " + + rhsVar1->getNameInfo().getAsString() + ")"); } - } else if (binaryOp->getOpcode() == BO_GT) { - if (lhsVar1->getDecl() == lhsVar2->getDecl() && - rhsVar1->getDecl() == rhsVar2->getDecl()) { - diag(ifStmt->getIfLoc(), "use std::min instead of >") - << FixItHint::CreateReplacement( - SourceRange(ifLocation, thenLocation), - lhsVar2->getNameInfo().getAsString() + " = std::min(" + - lhsVar1->getNameInfo().getAsString() + ", " + - rhsVar1->getNameInfo().getAsString() + ")"); - } else if (lhsVar1->getDecl() == rhsVar2->getDecl() && - rhsVar1->getDecl() == lhsVar2->getDecl()) { - diag(ifStmt->getIfLoc(), "use std::max instead of >") - << FixItHint::CreateReplacement( - SourceRange(ifLocation, thenLocation), - lhsVar2->getNameInfo().getAsString() + " = std::max(" + - lhsVar1->getNameInfo().getAsString() + ", " + - rhsVar1->getNameInfo().getAsString() + ")"); + else if(lhsVar1->getDecl() == rhsVar2->getDecl() && rhsVar1->getDecl() == lhsVar2->getDecl()){ + diag(ifStmt->getIfLoc(), "use std::max instead of >")<<FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), + lhsVar2->getNameInfo().getAsString() + " = std::max(" + lhsVar1->getNameInfo().getAsString() + ", " + + rhsVar1->getNameInfo().getAsString() + ")"); } } } diff --git a/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h b/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h index 02ebed83fed6c82..b7eabcc7d16d416 100644 --- a/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h +++ b/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h @@ -13,9 +13,7 @@ namespace clang::tidy::readability { -/// FIXME: replaces certain conditional statements with equivalent std::min or -/// std::max expressions, improving readability and promoting the use of -/// standard library functions." +/// FIXME: replaces certain conditional statements with equivalent std::min or std::max expressions, improving readability and promoting the use of standard library functions." /// /// For the user-facing documentation see: /// http://clang.llvm.org/extra/clang-tidy/checks/readability/ConditionalToStdMinMax.html >From 9aad658ef0e6175cdc0388c914a444a5bcd9ba1c Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Fri, 12 Jan 2024 15:43:52 +0530 Subject: [PATCH 06/17] Revert "Add readability check to suggest replacement of conditional statement with std::min/std::max" This reverts commit 1883d987b2f83adaef05fdb47ae25c7b06582a64. --- .../clang-tidy/readability/CMakeLists.txt | 1 - .../ConditionaltostdminmaxCheck.cpp | 86 ------------------- .../readability/ConditionaltostdminmaxCheck.h | 30 ------- .../readability/ReadabilityTidyModule.cpp | 3 - clang-tools-extra/docs/ReleaseNotes.rst | 6 -- .../docs/clang-tidy/checks/list.rst | 1 - .../readability/ConditionalToStdMinMax.rst | 29 ------- .../readability/ConditionalToStdMinMax.cpp | 27 ------ 8 files changed, 183 deletions(-) delete mode 100644 clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp delete mode 100644 clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h delete mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst delete mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/ConditionalToStdMinMax.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 4bc373bb69bb84a..408c822b861c5fe 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -8,7 +8,6 @@ add_clang_library(clangTidyReadabilityModule AvoidReturnWithVoidValueCheck.cpp AvoidUnconditionalPreprocessorIfCheck.cpp BracesAroundStatementsCheck.cpp - ConditionaltostdminmaxCheck.cpp ConstReturnTypeCheck.cpp ContainerContainsCheck.cpp ContainerDataPointerCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp deleted file mode 100644 index fba8c68f737450f..000000000000000 --- a/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.cpp +++ /dev/null @@ -1,86 +0,0 @@ -//===--- ConditionaltostdminmaxCheck.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 "ConditionaltostdminmaxCheck.h" -#include "clang/AST/ASTContext.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" - -using namespace clang::ast_matchers; - -namespace clang::tidy::readability { - -void ConditionaltostdminmaxCheck::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher( - ifStmt( - has( - binaryOperator( - anyOf(hasOperatorName("<"),hasOperatorName(">")), - hasLHS(ignoringImpCasts(declRefExpr().bind("lhsVar1"))), - hasRHS(ignoringImpCasts(declRefExpr().bind("rhsVar1"))) - ) - ) - , - hasThen( - stmt( - binaryOperator( - hasOperatorName("="), - hasLHS(ignoringImpCasts(declRefExpr().bind("lhsVar2"))), - hasRHS(ignoringImpCasts(declRefExpr().bind("rhsVar2"))) - ) - ) - ) - ).bind("ifStmt"),this); - - - -} - -void ConditionaltostdminmaxCheck::check(const MatchFinder::MatchResult &Result) { - const DeclRefExpr *lhsVar1 = Result.Nodes.getNodeAs<DeclRefExpr>("lhsVar1"); - const DeclRefExpr *rhsVar1 = Result.Nodes.getNodeAs<DeclRefExpr>("rhsVar1"); - const DeclRefExpr *lhsVar2 = Result.Nodes.getNodeAs<DeclRefExpr>("lhsVar2"); - const DeclRefExpr *rhsVar2 = Result.Nodes.getNodeAs<DeclRefExpr>("rhsVar2"); - const IfStmt *ifStmt = Result.Nodes.getNodeAs<IfStmt>("ifStmt"); - - if (!lhsVar1 || !rhsVar1 || !lhsVar2 || !rhsVar2 || !ifStmt) - return; - - const BinaryOperator *binaryOp = dyn_cast<BinaryOperator>(ifStmt->getCond()); - if (!binaryOp) - return; - - SourceLocation ifLocation = ifStmt->getIfLoc(); - SourceLocation thenLocation = ifStmt->getEndLoc(); - - if(binaryOp->getOpcode() == BO_LT){ - if(lhsVar1->getDecl() == lhsVar2->getDecl() && rhsVar1->getDecl() == rhsVar2->getDecl()){ - diag(ifStmt->getIfLoc(), "use std::max instead of <")<<FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), - lhsVar2->getNameInfo().getAsString() + " = std::max(" + lhsVar1->getNameInfo().getAsString() + ", " + - rhsVar1->getNameInfo().getAsString() + ")"); - } - else if(lhsVar1->getDecl() == rhsVar2->getDecl() && rhsVar1->getDecl() == lhsVar2->getDecl()){ - diag(ifStmt->getIfLoc(), "use std::min instead of <")<<FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), - lhsVar2->getNameInfo().getAsString() + " = std::min(" + lhsVar1->getNameInfo().getAsString() + ", " + - rhsVar1->getNameInfo().getAsString() + ")"); - } - } - else if(binaryOp->getOpcode() == BO_GT){ - if(lhsVar1->getDecl() == lhsVar2->getDecl() && rhsVar1->getDecl() == rhsVar2->getDecl()){ - diag(ifStmt->getIfLoc(), "use std::min instead of >")<<FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), - lhsVar2->getNameInfo().getAsString() + " = std::min(" + lhsVar1->getNameInfo().getAsString() + ", " + - rhsVar1->getNameInfo().getAsString() + ")"); - } - else if(lhsVar1->getDecl() == rhsVar2->getDecl() && rhsVar1->getDecl() == lhsVar2->getDecl()){ - diag(ifStmt->getIfLoc(), "use std::max instead of >")<<FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), - lhsVar2->getNameInfo().getAsString() + " = std::max(" + lhsVar1->getNameInfo().getAsString() + ", " + - rhsVar1->getNameInfo().getAsString() + ")"); - } - } -} - -} // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h b/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h deleted file mode 100644 index b7eabcc7d16d416..000000000000000 --- a/clang-tools-extra/clang-tidy/readability/ConditionaltostdminmaxCheck.h +++ /dev/null @@ -1,30 +0,0 @@ -//===--- ConditionaltostdminmaxCheck.h - clang-tidy -------------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONDITIONALTOSTDMINMAXCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONDITIONALTOSTDMINMAXCHECK_H - -#include "../ClangTidyCheck.h" - -namespace clang::tidy::readability { - -/// FIXME: replaces certain conditional statements with equivalent std::min or std::max expressions, improving readability and promoting the use of standard library functions." -/// -/// For the user-facing documentation see: -/// http://clang.llvm.org/extra/clang-tidy/checks/readability/ConditionalToStdMinMax.html -class ConditionaltostdminmaxCheck : public ClangTidyCheck { -public: - ConditionaltostdminmaxCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} - void registerMatchers(ast_matchers::MatchFinder *Finder) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; -}; - -} // namespace clang::tidy::readability - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONDITIONALTOSTDMINMAXCHECK_H diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index 63f8f03be6bb916..0b0aad7c0dcb36c 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -13,7 +13,6 @@ #include "AvoidReturnWithVoidValueCheck.h" #include "AvoidUnconditionalPreprocessorIfCheck.h" #include "BracesAroundStatementsCheck.h" -#include "ConditionaltostdminmaxCheck.h" #include "ConstReturnTypeCheck.h" #include "ContainerContainsCheck.h" #include "ContainerDataPointerCheck.h" @@ -63,8 +62,6 @@ namespace readability { class ReadabilityModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { - CheckFactories.registerCheck<ConditionaltostdminmaxCheck>( - "readability-ConditionalToStdMinMax"); CheckFactories.registerCheck<AvoidConstParamsInDecls>( "readability-avoid-const-params-in-decls"); CheckFactories.registerCheck<AvoidReturnWithVoidValueCheck>( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index c3fb990ad6a7382..b4d87e0ed2a67ae 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -224,12 +224,6 @@ New checks Recommends the smallest possible underlying type for an ``enum`` or ``enum`` class based on the range of its enumerators. -- New :doc:`readability-ConditionalToStdMinMax - <clang-tidy/checks/readability/ConditionalToStdMinMax>` check. - - Replaces certain conditional statements with equivalent std::min or std::max expressions, - improving readability and promoting the use of standard library functions. - - New :doc:`readability-reference-to-constructed-temporary <clang-tidy/checks/readability/reference-to-constructed-temporary>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index c2eac55425c69e5..2f86121ad872998 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -336,7 +336,6 @@ Clang-Tidy Checks :doc:`portability-restrict-system-includes <portability/restrict-system-includes>`, "Yes" :doc:`portability-simd-intrinsics <portability/simd-intrinsics>`, :doc:`portability-std-allocator-const <portability/std-allocator-const>`, - :doc:`readability-ConditionalToStdMinMax <readability/ConditionalToStdMinMax>`, "Yes" :doc:`readability-avoid-const-params-in-decls <readability/avoid-const-params-in-decls>`, "Yes" :doc:`readability-avoid-return-with-void-value <readability/avoid-return-with-void-value>`, :doc:`readability-avoid-unconditional-preprocessor-if <readability/avoid-unconditional-preprocessor-if>`, diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst deleted file mode 100644 index e95858999b27725..000000000000000 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/ConditionalToStdMinMax.rst +++ /dev/null @@ -1,29 +0,0 @@ -.. title:: clang-tidy - readability-ConditionalToStdMinMax - -readability-ConditionalToStdMinMax -================================== - -Replaces certain conditional statements with equivalent std::min or std::max expressions, -improving readability and promoting the use of standard library functions. - -Examples: - -Before: - -.. code-block:: c++ - - void foo(){ - int a,b; - if(a < b) - a = b; - } - - -After: - -.. code-block:: c++ - - int main(){ - a = std::max(a, b); - - } \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/ConditionalToStdMinMax.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/ConditionalToStdMinMax.cpp deleted file mode 100644 index d29e9aa9fec7080..000000000000000 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/ConditionalToStdMinMax.cpp +++ /dev/null @@ -1,27 +0,0 @@ -// RUN: %check_clang_tidy %s readability-ConditionalToStdMinMax %t - -void foo() { - int value1,value2; - - // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::max instead of < [readability-ConditionalToStdMinMax] - if (value1 < value2) - value1 = value2; // CHECK-FIXES: value1 = std::max(value1, value2); - - // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::min instead of < [readability-ConditionalToStdMinMax] - if (value1 < value2) - value2 = value1; // CHECK-FIXES: value2 = std::min(value1, value2); - - // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::min instead of > [readability-ConditionalToStdMinMax] - if (value2 > value1) - value2 = value1; // CHECK-FIXES: value2 = std::min(value2, value1); - - // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::max instead of > [readability-ConditionalToStdMinMax] - if (value2 > value1) - value1 = value2; // CHECK-FIXES: value1 = std::max(value2, value1); - - // No suggestion needed here - if (value1 == value2) - value1 = value2; - - -} \ No newline at end of file >From 2fc0938430a25ca6672c2d408e55f5b1a3188d60 Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Fri, 12 Jan 2024 17:24:08 +0530 Subject: [PATCH 07/17] Added Suggested Changes Signed-off-by: 11happy <soni5ha...@gmail.com> --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + .../readability/UseStdMinMaxCheck.cpp | 103 ++++++++++++++++++ .../readability/UseStdMinMaxCheck.h | 38 +++++++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../docs/clang-tidy/checks/list.rst | 1 + .../checks/readability/UseStdMinMax.rst | 33 ++++++ .../checkers/readability/use-std-min-max.cpp | 35 ++++++ 8 files changed, 219 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/UseStdMinMax.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 408c822b861c5fe..3670a4399543459 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -52,6 +52,7 @@ add_clang_library(clangTidyReadabilityModule UniqueptrDeleteReleaseCheck.cpp UppercaseLiteralSuffixCheck.cpp UseAnyOfAllOfCheck.cpp + UseStdMinMaxCheck.cpp LINK_LIBS clangTidy diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index 0b0aad7c0dcb36c..cba32ea24cec7c2 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -55,6 +55,7 @@ #include "UniqueptrDeleteReleaseCheck.h" #include "UppercaseLiteralSuffixCheck.h" #include "UseAnyOfAllOfCheck.h" +#include "UseStdMinMaxCheck.h" namespace clang::tidy { namespace readability { @@ -62,6 +63,8 @@ namespace readability { class ReadabilityModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck<UseStdMinMaxCheck>( + "readability-use-std-min-max"); CheckFactories.registerCheck<AvoidConstParamsInDecls>( "readability-avoid-const-params-in-decls"); CheckFactories.registerCheck<AvoidReturnWithVoidValueCheck>( diff --git a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp new file mode 100644 index 000000000000000..984792322ce5d30 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp @@ -0,0 +1,103 @@ +//===--- 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 "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Preprocessor.h" +#include <optional> + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +void UseStdMinMaxCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + ifStmt(has(binaryOperator( + anyOf(hasOperatorName("<"), hasOperatorName(">"),hasOperatorName("<="), hasOperatorName(">=")), + hasLHS(expr().bind("lhsVar1")), + hasRHS(expr().bind("rhsVar1")))), + hasThen(stmt(binaryOperator( + hasOperatorName("="), + hasLHS(expr().bind("lhsVar2")), + hasRHS(expr().bind("rhsVar2")))))) + .bind("ifStmt"), + this); + +} + +void UseStdMinMaxCheck::check(const MatchFinder::MatchResult &Result) { + const auto *lhsVar1 = Result.Nodes.getNodeAs<Expr>("lhsVar1"); + const auto *rhsVar1 = Result.Nodes.getNodeAs<Expr>("rhsVar1"); + const auto *lhsVar2 = Result.Nodes.getNodeAs<Expr>("lhsVar2"); + const auto *rhsVar2 = Result.Nodes.getNodeAs<Expr>("rhsVar2"); + const auto *ifStmt = Result.Nodes.getNodeAs<IfStmt>("ifStmt"); + auto &Context = *Result.Context; + + if (!lhsVar1 || !rhsVar1 || !lhsVar2 || !rhsVar2 || !ifStmt) + return; + + const BinaryOperator *binaryOp = dyn_cast<BinaryOperator>(ifStmt->getCond()); + if (!binaryOp) + return; + + SourceLocation ifLocation = ifStmt->getIfLoc(); + SourceLocation thenLocation = ifStmt->getEndLoc(); + auto lhsVar1Str = Lexer::getSourceText(CharSourceRange::getTokenRange(lhsVar1->getSourceRange()), + Context.getSourceManager(), Context.getLangOpts()); + + auto lhsVar2Str = Lexer::getSourceText(CharSourceRange::getTokenRange(lhsVar2->getSourceRange()), + Context.getSourceManager(), Context.getLangOpts()); + + auto rhsVar1Str = Lexer::getSourceText(CharSourceRange::getTokenRange(rhsVar1->getSourceRange()), + Context.getSourceManager(), Context.getLangOpts()); + + auto rhsVar2Str = Lexer::getSourceText(CharSourceRange::getTokenRange(rhsVar2->getSourceRange()), + Context.getSourceManager(), Context.getLangOpts()); + + if (binaryOp->getOpcode() == BO_LT) { + if (lhsVar1Str == lhsVar2Str && + rhsVar1Str == rhsVar2Str) { + diag(ifStmt->getIfLoc(), "use `std::max` instead of `<`") + << FixItHint::CreateReplacement( + SourceRange(ifLocation, thenLocation), + lhsVar2Str.str() + " = std::max(" + + lhsVar1Str.str() + ", " + + rhsVar1Str.str() + ")"); + } else if (lhsVar1Str == rhsVar2Str && + rhsVar1Str == lhsVar2Str) { + diag(ifStmt->getIfLoc(), "use `std::min` instead of `<`") + << FixItHint::CreateReplacement( + SourceRange(ifLocation, thenLocation), + lhsVar2Str.str() + " = std::min(" + + lhsVar1Str.str() + ", " + + rhsVar1Str.str() + ")"); + } + } else if (binaryOp->getOpcode() == BO_GT) { + if (lhsVar1Str == lhsVar2Str && + rhsVar1Str == rhsVar2Str) { + diag(ifStmt->getIfLoc(), "use `std::min` instead of `>`") + << FixItHint::CreateReplacement( + SourceRange(ifLocation, thenLocation), + lhsVar2Str.str() + " = std::min(" + + lhsVar1Str.str() + ", " + + rhsVar1Str.str() + ")"); + } else if (lhsVar1Str == rhsVar2Str && + rhsVar1Str == lhsVar2Str) { + diag(ifStmt->getIfLoc(), "use `std::max` instead of `>`") + << FixItHint::CreateReplacement( + SourceRange(ifLocation, thenLocation), + lhsVar2Str.str() + " = std::max(" + + lhsVar1Str.str() + ", " + + rhsVar1Str.str() + ")"); + } + } + +} + +} // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.h b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.h new file mode 100644 index 000000000000000..476b19ba5c9cf99 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.h @@ -0,0 +1,38 @@ +//===--- UseStdMinMaxCheck.h - clang-tidy -----------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USESTDMINMAXCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USESTDMINMAXCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::readability { + +/// replaces certain conditional statements with equivalent ``std::min`` or +/// ``std::max`` expressions, improving readability and promoting the use of +/// standard library functions. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability/UseStdMinMax.html +class UseStdMinMaxCheck : public ClangTidyCheck { +public: + UseStdMinMaxCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + std::optional<TraversalKind> getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } +}; + +} // namespace clang::tidy::readability + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USESTDMINMAXCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index b4d87e0ed2a67ae..2d3a2c2981e6ca0 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -224,6 +224,11 @@ New checks Recommends the smallest possible underlying type for an ``enum`` or ``enum`` class based on the range of its enumerators. +- New :doc:`readability-use-std-min-max + <clang-tidy/checks/readability/use-std-min-max>` check. + + FIXME: add release notes. + - New :doc:`readability-reference-to-constructed-temporary <clang-tidy/checks/readability/reference-to-constructed-temporary>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 2f86121ad872998..25b3550a77e4631 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -336,6 +336,7 @@ Clang-Tidy Checks :doc:`portability-restrict-system-includes <portability/restrict-system-includes>`, "Yes" :doc:`portability-simd-intrinsics <portability/simd-intrinsics>`, :doc:`portability-std-allocator-const <portability/std-allocator-const>`, + :doc:`readability-use-std-min-max <readability/use-std-min-max>`, "Yes" :doc:`readability-avoid-const-params-in-decls <readability/avoid-const-params-in-decls>`, "Yes" :doc:`readability-avoid-return-with-void-value <readability/avoid-return-with-void-value>`, :doc:`readability-avoid-unconditional-preprocessor-if <readability/avoid-unconditional-preprocessor-if>`, diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/UseStdMinMax.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/UseStdMinMax.rst new file mode 100644 index 000000000000000..c8b9bdc7c5ea3bd --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/UseStdMinMax.rst @@ -0,0 +1,33 @@ +.. title:: clang-tidy - readability-use-std-min-max + +readability-use-std-min-max +======================== + +Replaces certain conditional statements with equivalent ``std::min`` or ``std::max`` expressions, +improving readability and promoting the use of standard library functions. +Note: While this transformation improves code clarity, it may not be +suitable for performance-critical code. Using ``std::min`` or ``std::max`` can +introduce additional stores, potentially impacting performance compared to +the original if statement that only assigns when the value needs to change. + +Examples: + +Before: + +.. code-block:: c++ + + void foo(){ + int a,b; + if(a < b) + a = b; + } + + +After: + +.. code-block:: c++ + + void foo(){ + a = std::max(a, b); + + } diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp new file mode 100644 index 000000000000000..37ee1fd65fb9150 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp @@ -0,0 +1,35 @@ +// RUN: %check_clang_tidy %s readability-use-std-min-max %t + +void foo() { + int value1,value2,value3; + short value4; + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max] + if (value1 < value2) + value1 = value2; // CHECK-FIXES: value1 = std::max(value1, value2); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max] + if (value1 < value2) + value2 = value1; // CHECK-FIXES: value2 = std::min(value1, value2); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max] + if (value2 > value1) + value2 = value1; // CHECK-FIXES: value2 = std::min(value2, value1); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `>` [readability-use-std-min-max + if (value2 > value1) + value1 = value2; // CHECK-FIXES: value1 = std::max(value2, value1); + + // No suggestion needed here + if (value1 == value2) + value1 = value2; + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max] + if(value1<value4) + value1=value4; // CHECK-FIXES: value1 = std::max(value1, value4); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max] + if(value1+value2<value3) + value3 = value1+value2; // CHECK-FIXES: value3 = std::min(value1+value2, value3); + +} \ No newline at end of file >From 1139ac49be8023011bd57a2462781ac8b219c161 Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Fri, 12 Jan 2024 17:24:47 +0530 Subject: [PATCH 08/17] Formatted the Code Signed-off-by: 11happy <soni5ha...@gmail.com> --- .../readability/UseStdMinMaxCheck.cpp | 88 +++++++++---------- 1 file changed, 41 insertions(+), 47 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp index 984792322ce5d30..f203ec1e89b35e4 100644 --- a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp @@ -17,18 +17,17 @@ using namespace clang::ast_matchers; namespace clang::tidy::readability { void UseStdMinMaxCheck::registerMatchers(MatchFinder *Finder) { - Finder->addMatcher( - ifStmt(has(binaryOperator( - anyOf(hasOperatorName("<"), hasOperatorName(">"),hasOperatorName("<="), hasOperatorName(">=")), - hasLHS(expr().bind("lhsVar1")), - hasRHS(expr().bind("rhsVar1")))), - hasThen(stmt(binaryOperator( - hasOperatorName("="), - hasLHS(expr().bind("lhsVar2")), - hasRHS(expr().bind("rhsVar2")))))) + Finder->addMatcher( + ifStmt( + has(binaryOperator( + anyOf(hasOperatorName("<"), hasOperatorName(">"), + hasOperatorName("<="), hasOperatorName(">=")), + hasLHS(expr().bind("lhsVar1")), hasRHS(expr().bind("rhsVar1")))), + hasThen(stmt(binaryOperator(hasOperatorName("="), + hasLHS(expr().bind("lhsVar2")), + hasRHS(expr().bind("rhsVar2")))))) .bind("ifStmt"), this); - } void UseStdMinMaxCheck::check(const MatchFinder::MatchResult &Result) { @@ -48,56 +47,51 @@ void UseStdMinMaxCheck::check(const MatchFinder::MatchResult &Result) { SourceLocation ifLocation = ifStmt->getIfLoc(); SourceLocation thenLocation = ifStmt->getEndLoc(); - auto lhsVar1Str = Lexer::getSourceText(CharSourceRange::getTokenRange(lhsVar1->getSourceRange()), - Context.getSourceManager(), Context.getLangOpts()); + auto lhsVar1Str = Lexer::getSourceText( + CharSourceRange::getTokenRange(lhsVar1->getSourceRange()), + Context.getSourceManager(), Context.getLangOpts()); - auto lhsVar2Str = Lexer::getSourceText(CharSourceRange::getTokenRange(lhsVar2->getSourceRange()), - Context.getSourceManager(), Context.getLangOpts()); + auto lhsVar2Str = Lexer::getSourceText( + CharSourceRange::getTokenRange(lhsVar2->getSourceRange()), + Context.getSourceManager(), Context.getLangOpts()); - auto rhsVar1Str = Lexer::getSourceText(CharSourceRange::getTokenRange(rhsVar1->getSourceRange()), - Context.getSourceManager(), Context.getLangOpts()); + auto rhsVar1Str = Lexer::getSourceText( + CharSourceRange::getTokenRange(rhsVar1->getSourceRange()), + Context.getSourceManager(), Context.getLangOpts()); - auto rhsVar2Str = Lexer::getSourceText(CharSourceRange::getTokenRange(rhsVar2->getSourceRange()), - Context.getSourceManager(), Context.getLangOpts()); + auto rhsVar2Str = Lexer::getSourceText( + CharSourceRange::getTokenRange(rhsVar2->getSourceRange()), + Context.getSourceManager(), Context.getLangOpts()); if (binaryOp->getOpcode() == BO_LT) { - if (lhsVar1Str == lhsVar2Str && - rhsVar1Str == rhsVar2Str) { + if (lhsVar1Str == lhsVar2Str && rhsVar1Str == rhsVar2Str) { diag(ifStmt->getIfLoc(), "use `std::max` instead of `<`") - << FixItHint::CreateReplacement( - SourceRange(ifLocation, thenLocation), - lhsVar2Str.str() + " = std::max(" + - lhsVar1Str.str() + ", " + - rhsVar1Str.str() + ")"); - } else if (lhsVar1Str == rhsVar2Str && - rhsVar1Str == lhsVar2Str) { + << FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), + lhsVar2Str.str() + " = std::max(" + + lhsVar1Str.str() + ", " + + rhsVar1Str.str() + ")"); + } else if (lhsVar1Str == rhsVar2Str && rhsVar1Str == lhsVar2Str) { diag(ifStmt->getIfLoc(), "use `std::min` instead of `<`") - << FixItHint::CreateReplacement( - SourceRange(ifLocation, thenLocation), - lhsVar2Str.str() + " = std::min(" + - lhsVar1Str.str() + ", " + - rhsVar1Str.str() + ")"); + << FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), + lhsVar2Str.str() + " = std::min(" + + lhsVar1Str.str() + ", " + + rhsVar1Str.str() + ")"); } } else if (binaryOp->getOpcode() == BO_GT) { - if (lhsVar1Str == lhsVar2Str && - rhsVar1Str == rhsVar2Str) { + if (lhsVar1Str == lhsVar2Str && rhsVar1Str == rhsVar2Str) { diag(ifStmt->getIfLoc(), "use `std::min` instead of `>`") - << FixItHint::CreateReplacement( - SourceRange(ifLocation, thenLocation), - lhsVar2Str.str() + " = std::min(" + - lhsVar1Str.str() + ", " + - rhsVar1Str.str() + ")"); - } else if (lhsVar1Str == rhsVar2Str && - rhsVar1Str == lhsVar2Str) { + << FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), + lhsVar2Str.str() + " = std::min(" + + lhsVar1Str.str() + ", " + + rhsVar1Str.str() + ")"); + } else if (lhsVar1Str == rhsVar2Str && rhsVar1Str == lhsVar2Str) { diag(ifStmt->getIfLoc(), "use `std::max` instead of `>`") - << FixItHint::CreateReplacement( - SourceRange(ifLocation, thenLocation), - lhsVar2Str.str() + " = std::max(" + - lhsVar1Str.str() + ", " + - rhsVar1Str.str() + ")"); + << FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), + lhsVar2Str.str() + " = std::max(" + + lhsVar1Str.str() + ", " + + rhsVar1Str.str() + ")"); } } - } } // namespace clang::tidy::readability >From f73e9f019a03f5f864e952ddbad288d9a11b801e Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Fri, 12 Jan 2024 17:27:10 +0530 Subject: [PATCH 09/17] Added Release Notes Signed-off-by: 11happy <soni5ha...@gmail.com> --- clang-tools-extra/docs/ReleaseNotes.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 2d3a2c2981e6ca0..314e0eb69f79343 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -227,7 +227,9 @@ New checks - New :doc:`readability-use-std-min-max <clang-tidy/checks/readability/use-std-min-max>` check. - FIXME: add release notes. + Replaces certain conditional statements with equivalent ``std::min`` or + ``std::max`` expressions, improving readability and promoting the use of + standard library functions. - New :doc:`readability-reference-to-constructed-temporary <clang-tidy/checks/readability/reference-to-constructed-temporary>` check. >From faf3312b6d6713cb9af43ae09ca66f3ac05878bf Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Fri, 12 Jan 2024 17:31:38 +0530 Subject: [PATCH 10/17] corrected file name Signed-off-by: 11happy <soni5ha...@gmail.com> --- .../checks/readability/{UseStdMinMax.rst => use-std-min-max.rst} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename clang-tools-extra/docs/clang-tidy/checks/readability/{UseStdMinMax.rst => use-std-min-max.rst} (100%) diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/UseStdMinMax.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst similarity index 100% rename from clang-tools-extra/docs/clang-tidy/checks/readability/UseStdMinMax.rst rename to clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst >From 5ed9896cffdbf057b4e8088a37c98a50e0e4d946 Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Fri, 12 Jan 2024 17:35:44 +0530 Subject: [PATCH 11/17] corrected title length in docs Signed-off-by: 11happy <soni5ha...@gmail.com> --- .../docs/clang-tidy/checks/readability/use-std-min-max.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst index c8b9bdc7c5ea3bd..573beeeb2a842b9 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst @@ -1,7 +1,7 @@ .. title:: clang-tidy - readability-use-std-min-max readability-use-std-min-max -======================== +=========================== Replaces certain conditional statements with equivalent ``std::min`` or ``std::max`` expressions, improving readability and promoting the use of standard library functions. >From 19c4b63da4a183f661a6e06fabc78aaf4e2868e4 Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Fri, 12 Jan 2024 21:54:46 +0530 Subject: [PATCH 12/17] Suggested Changes Signed-off-by: 11happy <soni5ha...@gmail.com> --- .../readability/UseStdMinMaxCheck.cpp | 2 +- .../checks/readability/use-std-min-max.rst | 22 +++++++++---------- .../checkers/readability/use-std-min-max.cpp | 1 - 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp index f203ec1e89b35e4..1a4392523cf187d 100644 --- a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp @@ -41,7 +41,7 @@ void UseStdMinMaxCheck::check(const MatchFinder::MatchResult &Result) { if (!lhsVar1 || !rhsVar1 || !lhsVar2 || !rhsVar2 || !ifStmt) return; - const BinaryOperator *binaryOp = dyn_cast<BinaryOperator>(ifStmt->getCond()); + const auto *binaryOp = dyn_cast<BinaryOperator>(ifStmt->getCond()); if (!binaryOp) return; diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst index 573beeeb2a842b9..2364acacbaffd7d 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst @@ -5,10 +5,8 @@ readability-use-std-min-max Replaces certain conditional statements with equivalent ``std::min`` or ``std::max`` expressions, improving readability and promoting the use of standard library functions. -Note: While this transformation improves code clarity, it may not be -suitable for performance-critical code. Using ``std::min`` or ``std::max`` can -introduce additional stores, potentially impacting performance compared to -the original if statement that only assigns when the value needs to change. +Note: this transformation may impact performance in performance-critical code due to potential +additional stores compared to the original if statement. Examples: @@ -16,18 +14,18 @@ Before: .. code-block:: c++ - void foo(){ - int a,b; - if(a < b) - a = b; - } + void foo() { + int a, b; + if (a < b) + a = b; + } After: .. code-block:: c++ - void foo(){ - a = std::max(a, b); - + void foo() { + int a, b; + a = std::max(a, b); } diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp index 37ee1fd65fb9150..b65e3a9b27cc216 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp @@ -31,5 +31,4 @@ void foo() { // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max] if(value1+value2<value3) value3 = value1+value2; // CHECK-FIXES: value3 = std::min(value1+value2, value3); - } \ No newline at end of file >From 8167787453886874b8e458f040c878ad7e711016 Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Fri, 12 Jan 2024 22:35:17 +0530 Subject: [PATCH 13/17] body indentation Signed-off-by: 11happy <soni5ha...@gmail.com> --- .../clang-tidy/checks/readability/use-std-min-max.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst index 2364acacbaffd7d..de22f069d219232 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst @@ -15,9 +15,9 @@ Before: .. code-block:: c++ void foo() { - int a, b; - if (a < b) - a = b; + int a, b; + if (a < b) + a = b; } @@ -26,6 +26,6 @@ After: .. code-block:: c++ void foo() { - int a, b; - a = std::max(a, b); + int a, b; + a = std::max(a, b); } >From 3a28399a08b4625abe7d08dee82fbee850de534e Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Sat, 13 Jan 2024 21:17:23 +0530 Subject: [PATCH 14/17] added intialisation,constexpr tests Signed-off-by: 11happy <soni5ha...@gmail.com> --- .../checks/readability/use-std-min-max.rst | 4 ++-- .../checkers/readability/use-std-min-max.cpp | 23 +++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst index de22f069d219232..0d621d4d4477f20 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst @@ -15,7 +15,7 @@ Before: .. code-block:: c++ void foo() { - int a, b; + int a = 2, b = 3; if (a < b) a = b; } @@ -26,6 +26,6 @@ After: .. code-block:: c++ void foo() { - int a, b; + int a = 2, b = 3; a = std::max(a, b); } diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp index b65e3a9b27cc216..1dc48ffc63cfb5e 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp @@ -1,5 +1,16 @@ // RUN: %check_clang_tidy %s readability-use-std-min-max %t +constexpr int myConstexprMin(int a, int b) { + return a < b ? a : b; +} + +constexpr int myConstexprMax(int a, int b) { + return a > b ? a : b; +} + +int bar(int x, int y) { + return x < y ? x : y; +} void foo() { int value1,value2,value3; short value4; @@ -31,4 +42,16 @@ void foo() { // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max] if(value1+value2<value3) value3 = value1+value2; // CHECK-FIXES: value3 = std::min(value1+value2, value3); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max] + if (value1 < myConstexprMin(value2, value3)) + value1 = myConstexprMin(value2, value3); // CHECK-FIXES: value1 = std::max(value1, myConstexprMin(value2, value3)); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max] + if (value1 > myConstexprMax(value2, value3)) + value1 = myConstexprMax(value2, value3); // CHECK-FIXES: value1 = std::min(value1, myConstexprMax(value2, value3)); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max] + if (value1 < bar(value2, value3)) + value1 = bar(value2, value3); // CHECK-FIXES: value1 = std::max(value1, bar(value2, value3)); } \ No newline at end of file >From 2ecd08a149d60b9dfdc54d777bf10b34758054a8 Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Sat, 13 Jan 2024 21:32:22 +0530 Subject: [PATCH 15/17] small fixes Signed-off-by: 11happy <soni5ha...@gmail.com> --- .../clang-tidy/checks/readability/use-std-min-max.rst | 9 +++++---- .../clang-tidy/checkers/readability/use-std-min-max.cpp | 1 + 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst index 0d621d4d4477f20..7e64a05ccd8b89d 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/use-std-min-max.rst @@ -3,10 +3,11 @@ readability-use-std-min-max =========================== -Replaces certain conditional statements with equivalent ``std::min`` or ``std::max`` expressions, -improving readability and promoting the use of standard library functions. -Note: this transformation may impact performance in performance-critical code due to potential -additional stores compared to the original if statement. +Replaces certain conditionals with ``std::min`` or ``std::max`` for readability, +promoting use of standard library functions. Note: This may impact +performance in critical code due to potential additional stores compared +to the original if statement. + Examples: diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp index 1dc48ffc63cfb5e..9525d5e1f24890c 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp @@ -11,6 +11,7 @@ constexpr int myConstexprMax(int a, int b) { int bar(int x, int y) { return x < y ? x : y; } + void foo() { int value1,value2,value3; short value4; >From 00d4080a6a1f4af2b79b748ff60527aae10c9f97 Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Mon, 15 Jan 2024 21:12:17 +0530 Subject: [PATCH 16/17] Addded Suggested Changes Signed-off-by: 11happy <soni5ha...@gmail.com> --- .../readability/UseStdMinMaxCheck.cpp | 50 +++++++-------- .../readability/UseStdMinMaxCheck.h | 2 +- .../checkers/readability/use-std-min-max.cpp | 63 +++++++++++++++++++ 3 files changed, 88 insertions(+), 27 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp index 1a4392523cf187d..833d27d4957b38f 100644 --- a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp @@ -8,6 +8,7 @@ #include "UseStdMinMaxCheck.h" #include "clang/AST/ASTContext.h" +#include "../utils/ASTUtils.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Lex/Preprocessor.h" #include <optional> @@ -47,6 +48,7 @@ void UseStdMinMaxCheck::check(const MatchFinder::MatchResult &Result) { SourceLocation ifLocation = ifStmt->getIfLoc(); SourceLocation thenLocation = ifStmt->getEndLoc(); + auto lhsVar1Str = Lexer::getSourceText( CharSourceRange::getTokenRange(lhsVar1->getSourceRange()), Context.getSourceManager(), Context.getLangOpts()); @@ -58,38 +60,34 @@ void UseStdMinMaxCheck::check(const MatchFinder::MatchResult &Result) { auto rhsVar1Str = Lexer::getSourceText( CharSourceRange::getTokenRange(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(); - auto rhsVar2Str = Lexer::getSourceText( - CharSourceRange::getTokenRange(rhsVar2->getSourceRange()), - Context.getSourceManager(), Context.getLangOpts()); - - if (binaryOp->getOpcode() == BO_LT) { - if (lhsVar1Str == lhsVar2Str && rhsVar1Str == rhsVar2Str) { - diag(ifStmt->getIfLoc(), "use `std::max` instead of `<`") + 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), - lhsVar2Str.str() + " = std::max(" + - lhsVar1Str.str() + ", " + - rhsVar1Str.str() + ")"); - } else if (lhsVar1Str == rhsVar2Str && rhsVar1Str == lhsVar2Str) { - diag(ifStmt->getIfLoc(), "use `std::min` instead of `<`") + std::move(replacementMax)); + } 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), - lhsVar2Str.str() + " = std::min(" + - lhsVar1Str.str() + ", " + - rhsVar1Str.str() + ")"); + std::move(replacementMin)); } - } else if (binaryOp->getOpcode() == BO_GT) { - if (lhsVar1Str == lhsVar2Str && rhsVar1Str == rhsVar2Str) { - diag(ifStmt->getIfLoc(), "use `std::min` instead of `>`") + } else if (binaryOp->getOpcode() == BO_GT || binaryOp->getOpcode() == BO_GE) { + if (tidy::utils::areStatementsIdentical(lhsVar1, lhsVar2,Context) && + tidy::utils::areStatementsIdentical(rhsVar1, rhsVar2,Context)) { + diag(ifStmt->getIfLoc(), "use `std::min` instead of `%0`")<< operatorStr << FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), - lhsVar2Str.str() + " = std::min(" + - lhsVar1Str.str() + ", " + - rhsVar1Str.str() + ")"); - } else if (lhsVar1Str == rhsVar2Str && rhsVar1Str == lhsVar2Str) { - diag(ifStmt->getIfLoc(), "use `std::max` instead of `>`") + std::move(replacementMin)); + } else if (tidy::utils::areStatementsIdentical(lhsVar1, rhsVar2,Context) && + tidy::utils::areStatementsIdentical(rhsVar1, lhsVar2,Context)) { + diag(ifStmt->getIfLoc(), "use `std::max` instead of `%0`")<< operatorStr << FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), - lhsVar2Str.str() + " = std::max(" + - lhsVar1Str.str() + ", " + - rhsVar1Str.str() + ")"); + std::move(replacementMax)); } } } diff --git a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.h b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.h index 476b19ba5c9cf99..d6f4fdf40ea3062 100644 --- a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.h +++ b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.h @@ -13,7 +13,7 @@ namespace clang::tidy::readability { -/// replaces certain conditional statements with equivalent ``std::min`` or +/// Replaces certain conditional statements with equivalent ``std::min`` or /// ``std::max`` expressions, improving readability and promoting the use of /// standard library functions. /// diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp index 9525d5e1f24890c..99be29fb163bb6c 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/use-std-min-max.cpp @@ -12,9 +12,16 @@ int bar(int x, int y) { return x < y ? x : y; } +class MyClass { +public: + int member1; + int member2; +}; + void foo() { int value1,value2,value3; short value4; + MyClass obj; // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max] if (value1 < value2) @@ -55,4 +62,60 @@ void foo() { // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max] if (value1 < bar(value2, value3)) value1 = bar(value2, value3); // CHECK-FIXES: value1 = std::max(value1, bar(value2, value3)); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `<=` [readability-use-std-min-max] + if (value1 <= value2) + value2 = value1; // CHECK-FIXES: value2 = std::min(value1, value2); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<=` [readability-use-std-min-max] + if (value1 <= value2) + value1 = value2; // CHECK-FIXES: value1 = std::max(value1, value2); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `>=` [readability-use-std-min-max] + if (value2 >= value1) + value1 = value2; // CHECK-FIXES: value1 = std::max(value2, value1); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `>=` [readability-use-std-min-max] + if (value2 >= value1) + value2 = value1; // CHECK-FIXES: value2 = std::min(value2, value1); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max] + if (obj.member1 < obj.member2) + obj.member1 = obj.member2; // CHECK-FIXES: obj.member1 = std::max(obj.member1, obj.member2); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max] + if (obj.member1 < obj.member2) + obj.member2 = obj.member1; // CHECK-FIXES: obj.member2 = std::min(obj.member1, obj.member2); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `>` [readability-use-std-min-max] + if (obj.member2 > obj.member1) + obj.member2 = obj.member1; // CHECK-FIXES: obj.member2 = std::min(obj.member2, obj.member1); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `>` [readability-use-std-min-max] + if (obj.member2 > obj.member1) + obj.member1 = obj.member2; // CHECK-FIXES: obj.member1 = std::max(obj.member2, obj.member1); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<` [readability-use-std-min-max] + if (obj.member1 < value4) + obj.member1 = value4; // CHECK-FIXES: obj.member1 = std::max(obj.member1, value4); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `<` [readability-use-std-min-max] + if (obj.member1 + value2 < value3) + value3 = obj.member1 + value2; // CHECK-FIXES: value3 = std::min(obj.member1 + value2, value3); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `<=` [readability-use-std-min-max] + if (value1 <= obj.member2) + obj.member2 = value1; // CHECK-FIXES: obj.member2 = std::min(value1, obj.member2); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<=` [readability-use-std-min-max] + if (value1 <= obj.member2) + value1 = obj.member2; // CHECK-FIXES: value1 = std::max(value1, obj.member2); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `>=` [readability-use-std-min-max] + if (obj.member2 >= value1) + value1 = obj.member2; // CHECK-FIXES: value1 = std::max(obj.member2, value1); + + // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `>=` [readability-use-std-min-max] + if (obj.member2 >= value1) + obj.member2 = value1; // CHECK-FIXES: obj.member2 = std::min(obj.member2, value1); } \ No newline at end of file >From 6db08204966b52862ecdcf5ff829fe06e266b882 Mon Sep 17 00:00:00 2001 From: 11happy <soni5ha...@gmail.com> Date: Mon, 15 Jan 2024 21:12:41 +0530 Subject: [PATCH 17/17] formatted the code Signed-off-by: 11happy <soni5ha...@gmail.com> --- .../readability/UseStdMinMaxCheck.cpp | 46 +++++++++++-------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp index 833d27d4957b38f..2f7a8d944a50458 100644 --- a/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp @@ -7,8 +7,8 @@ //===----------------------------------------------------------------------===// #include "UseStdMinMaxCheck.h" -#include "clang/AST/ASTContext.h" #include "../utils/ASTUtils.h" +#include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Lex/Preprocessor.h" #include <optional> @@ -60,34 +60,40 @@ void UseStdMinMaxCheck::check(const MatchFinder::MatchResult &Result) { auto rhsVar1Str = Lexer::getSourceText( CharSourceRange::getTokenRange(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 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 + 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)); - } else if (tidy::utils::areStatementsIdentical(lhsVar1, rhsVar2,Context) && - tidy::utils::areStatementsIdentical(rhsVar1, lhsVar2,Context)) { - diag(ifStmt->getIfLoc(), "use `std::min` instead of `%0`")<< operatorStr + std::move(replacementMax)); + } 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)); + std::move(replacementMin)); } } else if (binaryOp->getOpcode() == BO_GT || binaryOp->getOpcode() == BO_GE) { - if (tidy::utils::areStatementsIdentical(lhsVar1, lhsVar2,Context) && - tidy::utils::areStatementsIdentical(rhsVar1, rhsVar2,Context)) { - diag(ifStmt->getIfLoc(), "use `std::min` instead of `%0`")<< operatorStr + if (tidy::utils::areStatementsIdentical(lhsVar1, lhsVar2, Context) && + tidy::utils::areStatementsIdentical(rhsVar1, rhsVar2, Context)) { + diag(ifStmt->getIfLoc(), "use `std::min` instead of `%0`") + << operatorStr << FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), - std::move(replacementMin)); - } else if (tidy::utils::areStatementsIdentical(lhsVar1, rhsVar2,Context) && - tidy::utils::areStatementsIdentical(rhsVar1, lhsVar2,Context)) { - diag(ifStmt->getIfLoc(), "use `std::max` instead of `%0`")<< operatorStr + std::move(replacementMin)); + } else if (tidy::utils::areStatementsIdentical(lhsVar1, rhsVar2, Context) && + tidy::utils::areStatementsIdentical(rhsVar1, lhsVar2, Context)) { + diag(ifStmt->getIfLoc(), "use `std::max` instead of `%0`") + << operatorStr << FixItHint::CreateReplacement(SourceRange(ifLocation, thenLocation), - std::move(replacementMax)); + std::move(replacementMax)); } } } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
