[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 20 04:37:43 PDT 2024
https://github.com/sopyb updated https://github.com/llvm/llvm-project/pull/85572
>From 4e466541a5ccf22d6fef08c71077e1f95ed10616 Mon Sep 17 00:00:00 2001
From: sopy <contact at sopy.one>
Date: Sun, 17 Mar 2024 17:30:27 +0200
Subject: [PATCH 1/4] [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 219096fa70b9a630b784d964c0d4bda642853b1f Mon Sep 17 00:00:00 2001
From: sopy <contact at sopy.one>
Date: Wed, 20 Mar 2024 12:11:34 +0200
Subject: [PATCH 2/4] 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 bd9273039fec0514e568801ce499484b1ce6519b Mon Sep 17 00:00:00 2001
From: sopy <contact at sopy.one>
Date: Wed, 20 Mar 2024 13:12:55 +0200
Subject: [PATCH 3/4] 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;
+ }
}
}
>From 18517aca52aa32ad2e2e7a838a9558c4a6fcbf0f Mon Sep 17 00:00:00 2001
From: sopy <contact at sopy.one>
Date: Wed, 20 Mar 2024 13:27:54 +0200
Subject: [PATCH 4/4] Clang format fix
---
.../clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
index 2fc6b517ad79ce..6e86c39843612a 100644
--- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
@@ -162,8 +162,7 @@ std::string MinMaxUseInitializerListCheck::generateReplacement(
(InnerCallNameStr == "std::min" ||
InnerCallNameStr == "std::max")) {
FindArgsResult innerResult = findArgs(Match, InnerCall);
- ReplacementText +=
- generateReplacement(Match, InnerCall, innerResult);
+ ReplacementText += generateReplacement(Match, InnerCall, innerResult);
continue;
}
}
More information about the cfe-commits
mailing list