https://github.com/sopyb updated https://github.com/llvm/llvm-project/pull/85572
>From 7f0e9d1b00ce1aba5ae2bee9563f1b681ff5d9b8 Mon Sep 17 00:00:00 2001 From: sopy <cont...@sopy.one> Date: Sun, 17 Mar 2024 17:30:27 +0200 Subject: [PATCH 1/3] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists --- .../clang-tidy/modernize/CMakeLists.txt | 1 + .../MinMaxUseInitializerListCheck.cpp | 138 ++++++++++++++++++ .../modernize/MinMaxUseInitializerListCheck.h | 58 ++++++++ .../modernize/ModernizeTidyModule.cpp | 3 + clang-tools-extra/docs/ReleaseNotes.rst | 6 + .../docs/clang-tidy/checks/list.rst | 1 + 6 files changed, 207 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index 6852db6c2ee311..8005d6e91c060c 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -16,6 +16,7 @@ add_clang_library(clangTidyModernizeModule MakeSharedCheck.cpp MakeSmartPtrCheck.cpp MakeUniqueCheck.cpp + MinMaxUseInitializerListCheck.cpp ModernizeTidyModule.cpp PassByValueCheck.cpp RawStringLiteralCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp new file mode 100644 index 00000000000000..b7dc3ff436f6e3 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp @@ -0,0 +1,138 @@ +//===--- MinMaxUseInitializerListCheck.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 "MinMaxUseInitializerListCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::modernize { + +MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + Inserter(Options.getLocalOrGlobal("IncludeStyle", + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()) {} + +void MinMaxUseInitializerListCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IncludeStyle", Inserter.getStyle()); +} + +void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + callExpr( + callee(functionDecl(hasName("::std::max"))), + hasAnyArgument(callExpr(callee(functionDecl(hasName("::std::max"))))), + unless( + hasParent(callExpr(callee(functionDecl(hasName("::std::max"))))))) + .bind("maxCall"), + this); + + Finder->addMatcher( + callExpr( + callee(functionDecl(hasName("::std::min"))), + hasAnyArgument(callExpr(callee(functionDecl(hasName("::std::min"))))), + unless( + hasParent(callExpr(callee(functionDecl(hasName("::std::min"))))))) + .bind("minCall"), + this); +} + +void MinMaxUseInitializerListCheck::registerPPCallbacks( + const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + Inserter.registerPreprocessor(PP); +} + +void MinMaxUseInitializerListCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *MaxCall = Result.Nodes.getNodeAs<CallExpr>("maxCall"); + const auto *MinCall = Result.Nodes.getNodeAs<CallExpr>("minCall"); + + const CallExpr *TopCall = MaxCall ? MaxCall : MinCall; + if (!TopCall) { + return; + } + const QualType ResultType = + TopCall->getDirectCallee()->getReturnType().getNonReferenceType(); + + const Expr *FirstArg = nullptr; + const Expr *LastArg = nullptr; + std::vector<const Expr *> Args; + findArgs(TopCall, &FirstArg, &LastArg, Args); + + if (!FirstArg || !LastArg || Args.size() <= 2) { + return; + } + + std::string ReplacementText = "{"; + for (const Expr *Arg : Args) { + QualType ArgType = Arg->getType(); + bool CastNeeded = + ArgType.getCanonicalType() != ResultType.getCanonicalType(); + + if (CastNeeded) + ReplacementText += "static_cast<" + ResultType.getAsString() + ">("; + + ReplacementText += Lexer::getSourceText( + CharSourceRange::getTokenRange(Arg->getSourceRange()), + *Result.SourceManager, Result.Context->getLangOpts()); + + if (CastNeeded) + ReplacementText += ")"; + ReplacementText += ", "; + } + ReplacementText = ReplacementText.substr(0, ReplacementText.size() - 2) + "}"; + + diag(TopCall->getBeginLoc(), + "do not use nested std::%0 calls, use %1 instead") + << TopCall->getDirectCallee()->getName() << ReplacementText + << FixItHint::CreateReplacement( + CharSourceRange::getTokenRange( + FirstArg->getBeginLoc(), + Lexer::getLocForEndOfToken(TopCall->getEndLoc(), 0, + Result.Context->getSourceManager(), + Result.Context->getLangOpts()) + .getLocWithOffset(-2)), + ReplacementText) + << Inserter.createMainFileIncludeInsertion("<algorithm>"); +} + +void MinMaxUseInitializerListCheck::findArgs(const CallExpr *Call, + const Expr **First, + const Expr **Last, + std::vector<const Expr *> &Args) { + if (!Call) { + return; + } + + const FunctionDecl *Callee = Call->getDirectCallee(); + if (!Callee) { + return; + } + + for (const Expr *Arg : Call->arguments()) { + if (!*First) + *First = Arg; + + const CallExpr *InnerCall = dyn_cast<CallExpr>(Arg); + if (InnerCall && InnerCall->getDirectCallee() && + InnerCall->getDirectCallee()->getNameAsString() == + Call->getDirectCallee()->getNameAsString()) { + findArgs(InnerCall, First, Last, Args); + } else + Args.push_back(Arg); + + *Last = Arg; + } +} + +} // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h new file mode 100644 index 00000000000000..dc111d4ce7800e --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h @@ -0,0 +1,58 @@ +//===--- MinMaxUseInitializerListCheck.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_MODERNIZE_MINMAXUSEINITIALIZERLISTCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MINMAXUSEINITIALIZERLISTCHECK_H + +#include "../ClangTidyCheck.h" +#include "../utils/IncludeInserter.h" + +namespace clang::tidy::modernize { + +/// Transforms the repeated calls to `std::min` and `std::max` into a single +/// call using initializer lists. +/// +/// It identifies cases where `std::min` or `std::max` is used to find the +/// minimum or maximum value among more than two items through repeated calls. +/// The check replaces these calls with a single call to `std::min` or +/// `std::max` that uses an initializer list. This makes the code slightly more +/// efficient. +/// \n\n +/// For example: +/// +/// \code +/// int a = std::max(std::max(i, j), k); +/// \endcode +/// +/// This code is transformed to: +/// +/// \code +/// int a = std::max({i, j, k}); +/// \endcode +class MinMaxUseInitializerListCheck : public ClangTidyCheck { +public: + MinMaxUseInitializerListCheck(StringRef Name, ClangTidyContext *Context); + + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus11; + } + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + utils::IncludeInserter Inserter; + void findArgs(const CallExpr *call, const Expr **first, const Expr **last, + std::vector<const Expr *> &args); +}; + +} // namespace clang::tidy::modernize + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MINMAXUSEINITIALIZERLISTCHECK_H diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index e96cf274f58cfe..776558433c5baa 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -18,6 +18,7 @@ #include "MacroToEnumCheck.h" #include "MakeSharedCheck.h" #include "MakeUniqueCheck.h" +#include "MinMaxUseInitializerListCheck.h" #include "PassByValueCheck.h" #include "RawStringLiteralCheck.h" #include "RedundantVoidArgCheck.h" @@ -68,6 +69,8 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck<MacroToEnumCheck>("modernize-macro-to-enum"); CheckFactories.registerCheck<MakeSharedCheck>("modernize-make-shared"); CheckFactories.registerCheck<MakeUniqueCheck>("modernize-make-unique"); + CheckFactories.registerCheck<MinMaxUseInitializerListCheck>( + "modernize-min-max-use-initializer-list"); CheckFactories.registerCheck<PassByValueCheck>("modernize-pass-by-value"); CheckFactories.registerCheck<UseDesignatedInitializersCheck>( "modernize-use-designated-initializers"); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index a604e9276668ae..e743438f589987 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -117,6 +117,12 @@ New checks to reading out-of-bounds data due to inadequate or incorrect string null termination. +- New :doc:`modernize-min-max-use-initializer-list + <clang-tidy/checks/modernize/min-max-use-initializer-list>` check. + + Replaces chained ``std::min`` and ``std::max`` calls that can be written as + initializer lists. + - New :doc:`modernize-use-designated-initializers <clang-tidy/checks/modernize/use-designated-initializers>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 79e81dd174e4f3..4809d704a94646 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -275,6 +275,7 @@ Clang-Tidy Checks :doc:`modernize-macro-to-enum <modernize/macro-to-enum>`, "Yes" :doc:`modernize-make-shared <modernize/make-shared>`, "Yes" :doc:`modernize-make-unique <modernize/make-unique>`, "Yes" + :doc:`modernize-min-max-use-initializer-list <modernize/min-max-use-initializer-list>`, "Yes" :doc:`modernize-pass-by-value <modernize/pass-by-value>`, "Yes" :doc:`modernize-raw-string-literal <modernize/raw-string-literal>`, "Yes" :doc:`modernize-redundant-void-arg <modernize/redundant-void-arg>`, "Yes" >From 7e8b2d9f676114927a71845cf6c87c114d71e408 Mon Sep 17 00:00:00 2001 From: sopy <cont...@sopy.one> Date: Wed, 20 Mar 2024 12:11:34 +0200 Subject: [PATCH 2/3] Summary: Refactor min-max-use-initializer-list.cpp and add tests Details: - Refactored min-max-use-initializer-list.cpp to handle cases where std::{min,max} is given a compare function argument - Shortened documentation in MinMaxUseInitializerListCheck.h - Added tests for min-modernize-min-max-use-initializer-list - Added documentation for min-modernize-min-max-use-initializer-list - Updated ReleaseNotes.rst based on mantainer suggestions --- .../MinMaxUseInitializerListCheck.cpp | 176 ++++++++++++------ .../modernize/MinMaxUseInitializerListCheck.h | 35 ++-- clang-tools-extra/docs/ReleaseNotes.rst | 4 +- .../min-max-use-initializer-list.rst | 26 +++ .../min-max-use-initializer-list.cpp | 105 +++++++++++ 5 files changed, 270 insertions(+), 76 deletions(-) create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/min-max-use-initializer-list.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/min-max-use-initializer-list.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp index b7dc3ff436f6e3..ddeac5b1b3ac77 100644 --- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp @@ -31,19 +31,25 @@ void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( callExpr( callee(functionDecl(hasName("::std::max"))), - hasAnyArgument(callExpr(callee(functionDecl(hasName("::std::max"))))), + anyOf(hasArgument( + 0, callExpr(callee(functionDecl(hasName("::std::max"))))), + hasArgument( + 1, callExpr(callee(functionDecl(hasName("::std::max")))))), unless( hasParent(callExpr(callee(functionDecl(hasName("::std::max"))))))) - .bind("maxCall"), + .bind("topCall"), this); Finder->addMatcher( callExpr( callee(functionDecl(hasName("::std::min"))), - hasAnyArgument(callExpr(callee(functionDecl(hasName("::std::min"))))), + anyOf(hasArgument( + 0, callExpr(callee(functionDecl(hasName("::std::min"))))), + hasArgument( + 1, callExpr(callee(functionDecl(hasName("::std::min")))))), unless( hasParent(callExpr(callee(functionDecl(hasName("::std::min"))))))) - .bind("minCall"), + .bind("topCall"), this); } @@ -53,29 +59,112 @@ void MinMaxUseInitializerListCheck::registerPPCallbacks( } void MinMaxUseInitializerListCheck::check( - const MatchFinder::MatchResult &Result) { - const auto *MaxCall = Result.Nodes.getNodeAs<CallExpr>("maxCall"); - const auto *MinCall = Result.Nodes.getNodeAs<CallExpr>("minCall"); + const MatchFinder::MatchResult &Match) { + const CallExpr *TopCall = Match.Nodes.getNodeAs<CallExpr>("topCall"); + MinMaxUseInitializerListCheck::FindArgsResult Result = + findArgs(Match, TopCall); - const CallExpr *TopCall = MaxCall ? MaxCall : MinCall; - if (!TopCall) { + if (!Result.First || !Result.Last || Result.Args.size() <= 2) { return; } - const QualType ResultType = - TopCall->getDirectCallee()->getReturnType().getNonReferenceType(); - const Expr *FirstArg = nullptr; - const Expr *LastArg = nullptr; - std::vector<const Expr *> Args; - findArgs(TopCall, &FirstArg, &LastArg, Args); + std::string ReplacementText = generateReplacement(Match, TopCall, Result); - if (!FirstArg || !LastArg || Args.size() <= 2) { - return; + diag(TopCall->getBeginLoc(), + "do not use nested std::%0 calls, use %1 instead") + << TopCall->getDirectCallee()->getName() << ReplacementText + << FixItHint::CreateReplacement( + CharSourceRange::getTokenRange(TopCall->getBeginLoc(), + TopCall->getEndLoc()), + ReplacementText) + << Inserter.createMainFileIncludeInsertion("<algorithm>"); +} + +MinMaxUseInitializerListCheck::FindArgsResult +MinMaxUseInitializerListCheck::findArgs(const MatchFinder::MatchResult &Match, + const CallExpr *Call) { + FindArgsResult Result; + Result.First = nullptr; + Result.Last = nullptr; + Result.Compare = nullptr; + + if (Call->getNumArgs() > 2) { + auto argIterator = Call->arguments().begin(); + std::advance(argIterator, 2); + Result.Compare = *argIterator; } - std::string ReplacementText = "{"; - for (const Expr *Arg : Args) { + for (const Expr *Arg : Call->arguments()) { + if (!Result.First) + Result.First = Arg; + + const CallExpr *InnerCall = dyn_cast<CallExpr>(Arg); + if (InnerCall && InnerCall->getDirectCallee() && + InnerCall->getDirectCallee()->getNameAsString() == + Call->getDirectCallee()->getNameAsString()) { + FindArgsResult InnerResult = findArgs(Match, InnerCall); + + bool processInnerResult = false; + + if (!Result.Compare && !InnerResult.Compare) + processInnerResult = true; + else if (Result.Compare && InnerResult.Compare && + Lexer::getSourceText(CharSourceRange::getTokenRange( + Result.Compare->getSourceRange()), + *Match.SourceManager, + Match.Context->getLangOpts()) == + Lexer::getSourceText( + CharSourceRange::getTokenRange( + InnerResult.Compare->getSourceRange()), + *Match.SourceManager, Match.Context->getLangOpts())) + processInnerResult = true; + + if (processInnerResult) { + Result.Args.insert(Result.Args.end(), InnerResult.Args.begin(), + InnerResult.Args.end()); + continue; + } + } + + if (Arg == Result.Compare) + continue; + + Result.Args.push_back(Arg); + Result.Last = Arg; + } + + return Result; +} + +std::string MinMaxUseInitializerListCheck::generateReplacement( + const MatchFinder::MatchResult &Match, const CallExpr *TopCall, + const FindArgsResult Result) { + std::string ReplacementText = + Lexer::getSourceText( + CharSourceRange::getTokenRange( + TopCall->getBeginLoc(), + Result.First->getBeginLoc().getLocWithOffset(-1)), + *Match.SourceManager, Match.Context->getLangOpts()) + .str() + + "{"; + const QualType ResultType = + TopCall->getDirectCallee()->getReturnType().getNonReferenceType(); + + for (const Expr *Arg : Result.Args) { QualType ArgType = Arg->getType(); + + // check if expression is std::min or std::max + if (const auto *InnerCall = dyn_cast<CallExpr>(Arg)) { + if (InnerCall->getDirectCallee() && + InnerCall->getDirectCallee()->getNameAsString() != + TopCall->getDirectCallee()->getNameAsString()) { + FindArgsResult innerResult = findArgs(Match, InnerCall); + ReplacementText += generateReplacement(Match, InnerCall, innerResult) += + "})"; + continue; + } + } + bool CastNeeded = ArgType.getCanonicalType() != ResultType.getCanonicalType(); @@ -84,55 +173,22 @@ void MinMaxUseInitializerListCheck::check( ReplacementText += Lexer::getSourceText( CharSourceRange::getTokenRange(Arg->getSourceRange()), - *Result.SourceManager, Result.Context->getLangOpts()); + *Match.SourceManager, Match.Context->getLangOpts()); if (CastNeeded) ReplacementText += ")"; ReplacementText += ", "; } ReplacementText = ReplacementText.substr(0, ReplacementText.size() - 2) + "}"; - - diag(TopCall->getBeginLoc(), - "do not use nested std::%0 calls, use %1 instead") - << TopCall->getDirectCallee()->getName() << ReplacementText - << FixItHint::CreateReplacement( - CharSourceRange::getTokenRange( - FirstArg->getBeginLoc(), - Lexer::getLocForEndOfToken(TopCall->getEndLoc(), 0, - Result.Context->getSourceManager(), - Result.Context->getLangOpts()) - .getLocWithOffset(-2)), - ReplacementText) - << Inserter.createMainFileIncludeInsertion("<algorithm>"); -} - -void MinMaxUseInitializerListCheck::findArgs(const CallExpr *Call, - const Expr **First, - const Expr **Last, - std::vector<const Expr *> &Args) { - if (!Call) { - return; - } - - const FunctionDecl *Callee = Call->getDirectCallee(); - if (!Callee) { - return; + if (Result.Compare) { + ReplacementText += ", "; + ReplacementText += Lexer::getSourceText( + CharSourceRange::getTokenRange(Result.Compare->getSourceRange()), + *Match.SourceManager, Match.Context->getLangOpts()); } + ReplacementText += ")"; - for (const Expr *Arg : Call->arguments()) { - if (!*First) - *First = Arg; - - const CallExpr *InnerCall = dyn_cast<CallExpr>(Arg); - if (InnerCall && InnerCall->getDirectCallee() && - InnerCall->getDirectCallee()->getNameAsString() == - Call->getDirectCallee()->getNameAsString()) { - findArgs(InnerCall, First, Last, Args); - } else - Args.push_back(Arg); - - *Last = Arg; - } + return ReplacementText; } } // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h index dc111d4ce7800e..43d95004511430 100644 --- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h @@ -14,15 +14,9 @@ namespace clang::tidy::modernize { -/// Transforms the repeated calls to `std::min` and `std::max` into a single -/// call using initializer lists. +/// Replaces chained ``std::min`` and ``std::max`` calls with a initializer list +/// where applicable. /// -/// It identifies cases where `std::min` or `std::max` is used to find the -/// minimum or maximum value among more than two items through repeated calls. -/// The check replaces these calls with a single call to `std::min` or -/// `std::max` that uses an initializer list. This makes the code slightly more -/// efficient. -/// \n\n /// For example: /// /// \code @@ -38,19 +32,32 @@ class MinMaxUseInitializerListCheck : public ClangTidyCheck { public: MinMaxUseInitializerListCheck(StringRef Name, ClangTidyContext *Context); - bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { - return LangOpts.CPlusPlus11; - } void storeOptions(ClangTidyOptions::OptionMap &Opts) override; void registerMatchers(ast_matchers::MatchFinder *Finder) override; void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void check(const ast_matchers::MatchFinder::MatchResult &Match) override; + + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus11; + } + std::optional<TraversalKind> getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } private: + struct FindArgsResult { + const Expr *First; + const Expr *Last; + const Expr *Compare; + std::vector<const Expr *> Args; + }; utils::IncludeInserter Inserter; - void findArgs(const CallExpr *call, const Expr **first, const Expr **last, - std::vector<const Expr *> &args); + FindArgsResult findArgs(const ast_matchers::MatchFinder::MatchResult &Match, + const CallExpr *Call); + std::string + generateReplacement(const ast_matchers::MatchFinder::MatchResult &Match, + const CallExpr *TopCall, const FindArgsResult Result); }; } // namespace clang::tidy::modernize diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index e743438f589987..13d14fa9ccf0aa 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -120,8 +120,8 @@ New checks - New :doc:`modernize-min-max-use-initializer-list <clang-tidy/checks/modernize/min-max-use-initializer-list>` check. - Replaces chained ``std::min`` and ``std::max`` calls that can be written as - initializer lists. + Replaces chained ``std::min`` and ``std::max`` calls with a initializer list + where applicable. - New :doc:`modernize-use-designated-initializers <clang-tidy/checks/modernize/use-designated-initializers>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/min-max-use-initializer-list.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/min-max-use-initializer-list.rst new file mode 100644 index 00000000000000..0748e180897948 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/min-max-use-initializer-list.rst @@ -0,0 +1,26 @@ +.. title:: clang-tidy - modernize-min-max-use-initializer-list + +modernize-min-max-use-initializer-list +====================================== + +Replaces chained ``std::min`` and ``std::max`` calls with a initializer list where applicable. + +For instance, consider the following code: + +.. code-block:: cpp + + int a = std::max(std::max(i, j), k); + +`modernize-min-max-use-initializer-list` check will transform the above code to: + +.. code-block:: cpp + + int a = std::max({i, j, k}); + +Options +======= + +.. option:: IncludeStyle + + A string specifying which include-style is used, `llvm` or `google`. Default + is `llvm`. \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/min-max-use-initializer-list.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/min-max-use-initializer-list.cpp new file mode 100644 index 00000000000000..3e15774f7b3f14 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/min-max-use-initializer-list.cpp @@ -0,0 +1,105 @@ +// RUN: %check_clang_tidy %s modernize-min-max-use-initializer-list %t + +// CHECK-FIXES: #include <algorithm> +namespace std { +template< class T > +const T& max( const T& a, const T& b ) { + return (a < b) ? b : a; +}; + +template< class T, class Compare > +const T& max( const T& a, const T& b, Compare comp ) { + return (comp(a, b)) ? b : a; +}; + +template< class T > +const T& min( const T& a, const T& b ) { + return (b < a) ? b : a; +}; + +template< class T, class Compare > +const T& min( const T& a, const T& b, Compare comp ) { + return (comp(b, a)) ? b : a; +}; +} + +using namespace std; + +namespace { +bool fless_than(int a, int b) { +return a < b; +} + +bool fgreater_than(int a, int b) { +return a > b; +} +auto less_than = [](int a, int b) { return a < b; }; +auto greater_than = [](int a, int b) { return a > b; }; + +int max1 = std::max(1, std::max(2, 3)); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::max calls, use std::max({1, 2, 3}) instead [modernize-min-max-use-initializer-list] +// CHECK-FIXES: int max1 = std::max({1, 2, 3}); + +int min1 = std::min(1, std::min(2, 3)); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::min calls, use std::min({1, 2, 3}) instead [modernize-min-max-use-initializer-list] +// CHECK-FIXES: int min1 = std::min({1, 2, 3}); + +int max2 = std::max(1, std::max(2, std::max(3, 4))); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::max calls, use std::max({1, 2, 3, 4}) instead [modernize-min-max-use-initializer-list] +// CHECK-FIXES: int max2 = std::max({1, 2, 3, 4}); + +int min2 = std::min(1, std::min(2, std::min(3, 4))); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::min calls, use std::min({1, 2, 3, 4}) instead [modernize-min-max-use-initializer-list] +// CHECK-FIXES: int min2 = std::min({1, 2, 3, 4}); + +int max3 = std::max(std::max(4, 5), std::min(2, std::min(3, 1))); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::max calls, use std::max({4, 5, std::min({2, 3, 1})}) instead [modernize-min-max-use-initializer-list] +// CHECK-MESSAGES: :[[@LINE-2]]:37: warning: do not use nested std::min calls, use std::min({2, 3, 1}) instead [modernize-min-max-use-initializer-list] +// CHECK-FIXES: int max3 = std::max({4, 5, std::min({2, 3, 1})}); + +int min3 = std::min(std::min(4, 5), std::max(2, std::max(3, 1))); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::min calls, use std::min({4, 5, std::max({2, 3, 1})}) instead [modernize-min-max-use-initializer-list] +// CHECK-MESSAGES: :[[@LINE-2]]:37: warning: do not use nested std::max calls, use std::max({2, 3, 1}) instead [modernize-min-max-use-initializer-list] +// CHECK-FIXES: int min3 = std::min({4, 5, std::max({2, 3, 1})}); + +int max4 = std::max(1,std::max(2,3, greater_than), less_than); +// CHECK-FIXES: int max4 = std::max(1,std::max(2,3, greater_than), less_than); + +int min4 = std::min(1,std::min(2,3, greater_than), less_than); +// CHECK-FIXES: int min4 = std::min(1,std::min(2,3, greater_than), less_than); + +int max5 = std::max(1,std::max(2,3), less_than); +// CHECK-FIXES: int max5 = std::max(1,std::max(2,3), less_than); + +int min5 = std::min(1,std::min(2,3), less_than); +// CHECK-FIXES: int min5 = std::min(1,std::min(2,3), less_than); + +int max6 = std::max(1,std::max(2,3, greater_than), greater_than); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::max calls, use std::max({1, 2, 3}, greater_than) instead [modernize-min-max-use-initializer-list] +// CHECK-FIXES: int max6 = std::max({1, 2, 3}, greater_than); + +int min6 = std::min(1,std::min(2,3, greater_than), greater_than); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::min calls, use std::min({1, 2, 3}, greater_than) instead [modernize-min-max-use-initializer-list] +// CHECK-FIXES: int min6 = std::min({1, 2, 3}, greater_than); + +int max7 = std::max(1,std::max(2,3, fless_than), fgreater_than); +// CHECK-FIXES: int max7 = std::max(1,std::max(2,3, fless_than), fgreater_than); + +int min7 = std::min(1,std::min(2,3, fless_than), fgreater_than); +// CHECK-FIXES: int min7 = std::min(1,std::min(2,3, fless_than), fgreater_than); + +int max8 = std::max(1,std::max(2,3, fless_than), less_than); +// CHECK-FIXES: int max8 = std::max(1,std::max(2,3, fless_than), less_than) + +int min8 = std::min(1,std::min(2,3, fless_than), less_than); +// CHECK-FIXES: int min8 = std::min(1,std::min(2,3, fless_than), less_than); + +int max9 = std::max(1,std::max(2,3, fless_than), fless_than); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::max calls, use std::max({1, 2, 3}, fless_than) instead [modernize-min-max-use-initializer-list] +// CHECK-FIXES: int max9 = std::max({1, 2, 3}, fless_than); + +int min9 = std::min(1,std::min(2,3, fless_than), fless_than); +// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::min calls, use std::min({1, 2, 3}, fless_than) instead [modernize-min-max-use-initializer-list] +// CHECK-FIXES: int min9 = std::min({1, 2, 3}, fless_than); + +} \ No newline at end of file >From bf7543ed9e52eba668b1d56e375e71288d800d99 Mon Sep 17 00:00:00 2001 From: sopy <cont...@sopy.one> Date: Wed, 20 Mar 2024 13:12:55 +0200 Subject: [PATCH 3/3] Make sure InnerCall is std::min or std::max when generating replacement --- .../MinMaxUseInitializerListCheck.cpp | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp index ddeac5b1b3ac77..2fc6b517ad79ce 100644 --- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp @@ -153,15 +153,19 @@ std::string MinMaxUseInitializerListCheck::generateReplacement( for (const Expr *Arg : Result.Args) { QualType ArgType = Arg->getType(); - // check if expression is std::min or std::max if (const auto *InnerCall = dyn_cast<CallExpr>(Arg)) { - if (InnerCall->getDirectCallee() && - InnerCall->getDirectCallee()->getNameAsString() != - TopCall->getDirectCallee()->getNameAsString()) { - FindArgsResult innerResult = findArgs(Match, InnerCall); - ReplacementText += generateReplacement(Match, InnerCall, innerResult) += - "})"; - continue; + if (InnerCall->getDirectCallee()) { + std::string InnerCallNameStr = + InnerCall->getDirectCallee()->getNameAsString(); + + if (InnerCallNameStr != TopCall->getDirectCallee()->getNameAsString() && + (InnerCallNameStr == "std::min" || + InnerCallNameStr == "std::max")) { + FindArgsResult innerResult = findArgs(Match, InnerCall); + ReplacementText += + generateReplacement(Match, InnerCall, innerResult); + continue; + } } } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
