[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
Mon Apr 1 08:57:43 PDT 2024
https://github.com/sopyb updated https://github.com/llvm/llvm-project/pull/85572
>From 17d6ad65216237f9a325d9b608c1372cbbf83ff0 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/9] [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 1bed756e8d45c48be834bd0d12610bb05ede478e 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/9] 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 f2e8474f88706363929855c543d0c8a9a4d77226 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/9] Squash: Apply Clang format fixes and ensure InnerCall
uses std::min or std::max in replacement generation
---
.../MinMaxUseInitializerListCheck.cpp | 27 +++++++++++--------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
index ddeac5b1b3ac77..93abbe023d8748 100644
--- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
@@ -89,9 +89,9 @@ MinMaxUseInitializerListCheck::findArgs(const MatchFinder::MatchResult &Match,
Result.Compare = nullptr;
if (Call->getNumArgs() > 2) {
- auto argIterator = Call->arguments().begin();
- std::advance(argIterator, 2);
- Result.Compare = *argIterator;
+ auto ArgIterator = Call->arguments().begin();
+ std::advance(ArgIterator, 2);
+ Result.Compare = *ArgIterator;
}
for (const Expr *Arg : Call->arguments()) {
@@ -153,15 +153,20 @@ 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()->getQualifiedNameAsString();
+
+ if (InnerCallNameStr !=
+ TopCall->getDirectCallee()->getQualifiedNameAsString() &&
+ (InnerCallNameStr == "std::min" ||
+ InnerCallNameStr == "std::max")) {
+ FindArgsResult innerResult = findArgs(Match, InnerCall);
+ ReplacementText +=
+ generateReplacement(Match, InnerCall, innerResult) + ", ";
+ continue;
+ }
}
}
>From bbdf160e7d9b700514d545cf28d382cb2db14c7f Mon Sep 17 00:00:00 2001
From: sopy <contact at sopy.one>
Date: Wed, 27 Mar 2024 11:56:34 +0200
Subject: [PATCH 4/9] Addressing code smell issues and consider nested calls
with initializer lists as arguments
---
.../MinMaxUseInitializerListCheck.cpp | 176 +++++++++++-------
.../modernize/MinMaxUseInitializerListCheck.h | 13 +-
clang-tools-extra/docs/ReleaseNotes.rst | 2 +-
.../min-max-use-initializer-list.rst | 2 +-
.../min-max-use-initializer-list.cpp | 133 +++++++++++--
5 files changed, 228 insertions(+), 98 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
index 93abbe023d8748..baf3da851e7f40 100644
--- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Lex/Lexer.h"
@@ -15,6 +16,19 @@ using namespace clang::ast_matchers;
namespace clang::tidy::modernize {
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ std::vector<const Expr *> Args;
+};
+
+static const FindArgsResult findArgs(const MatchFinder::MatchResult &Match,
+ const CallExpr *Call);
+static const std::string
+generateReplacement(const MatchFinder::MatchResult &Match,
+ const CallExpr *TopCall, const FindArgsResult &Result);
+
MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck(
StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
@@ -28,29 +42,18 @@ void MinMaxUseInitializerListCheck::storeOptions(
}
void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) {
- Finder->addMatcher(
- 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("topCall"),
- this);
-
- Finder->addMatcher(
- 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("topCall"),
- this);
+ auto createMatcher = [](const std::string &functionName) {
+ auto funcDecl = functionDecl(hasName(functionName));
+ auto expr = callExpr(callee(funcDecl));
+
+ return callExpr(callee(funcDecl),
+ anyOf(hasArgument(0, expr), hasArgument(1, expr)),
+ unless(hasParent(expr)))
+ .bind("topCall");
+ };
+
+ Finder->addMatcher(createMatcher("::std::max"), this);
+ Finder->addMatcher(createMatcher("::std::min"), this);
}
void MinMaxUseInitializerListCheck::registerPPCallbacks(
@@ -60,75 +63,98 @@ void MinMaxUseInitializerListCheck::registerPPCallbacks(
void MinMaxUseInitializerListCheck::check(
const MatchFinder::MatchResult &Match) {
- const CallExpr *TopCall = Match.Nodes.getNodeAs<CallExpr>("topCall");
- MinMaxUseInitializerListCheck::FindArgsResult Result =
- findArgs(Match, TopCall);
- if (!Result.First || !Result.Last || Result.Args.size() <= 2) {
+ const auto *TopCall = Match.Nodes.getNodeAs<CallExpr>("topCall");
+ FindArgsResult Result = findArgs(Match, TopCall);
+
+ if (Result.Args.size() <= 2) {
return;
}
- std::string ReplacementText = generateReplacement(Match, TopCall, Result);
+ const std::string ReplacementText =
+ generateReplacement(Match, TopCall, Result);
diag(TopCall->getBeginLoc(),
- "do not use nested std::%0 calls, use %1 instead")
+ "do not use nested 'std::%0' calls, use '%1' instead")
<< TopCall->getDirectCallee()->getName() << ReplacementText
<< FixItHint::CreateReplacement(
- CharSourceRange::getTokenRange(TopCall->getBeginLoc(),
- TopCall->getEndLoc()),
+ CharSourceRange::getTokenRange(TopCall->getSourceRange()),
ReplacementText)
- << Inserter.createMainFileIncludeInsertion("<algorithm>");
+ << Inserter.createIncludeInsertion(
+ Match.SourceManager->getFileID(TopCall->getBeginLoc()),
+ "<algorithm>");
}
-MinMaxUseInitializerListCheck::FindArgsResult
-MinMaxUseInitializerListCheck::findArgs(const MatchFinder::MatchResult &Match,
- const CallExpr *Call) {
+static const FindArgsResult findArgs(const MatchFinder::MatchResult &Match,
+ const CallExpr *Call) {
FindArgsResult Result;
Result.First = nullptr;
Result.Last = nullptr;
Result.Compare = nullptr;
- if (Call->getNumArgs() > 2) {
+ if (Call->getNumArgs() == 3) {
auto ArgIterator = Call->arguments().begin();
std::advance(ArgIterator, 2);
Result.Compare = *ArgIterator;
+ } else {
+ auto ArgIterator = Call->arguments().begin();
+
+ if (const auto *InitListExpr =
+ dyn_cast<CXXStdInitializerListExpr>(*ArgIterator)) {
+ if (const auto *TempExpr =
+ dyn_cast<MaterializeTemporaryExpr>(InitListExpr->getSubExpr())) {
+ if (const auto *InitList =
+ dyn_cast<clang::InitListExpr>(TempExpr->getSubExpr())) {
+ for (const Expr *Init : InitList->inits()) {
+ Result.Args.push_back(Init);
+ }
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+ Result.Compare = *ArgIterator;
+ }
+ return Result;
+ }
+ }
+ }
}
for (const Expr *Arg : Call->arguments()) {
if (!Result.First)
Result.First = Arg;
- const CallExpr *InnerCall = dyn_cast<CallExpr>(Arg);
+ if (Arg == Result.Compare)
+ continue;
+
+ const auto *InnerCall = dyn_cast<CallExpr>(Arg->IgnoreParenImpCasts());
+
+ if (InnerCall) {
+ printf("InnerCall: %s\n",
+ InnerCall->getDirectCallee()->getQualifiedNameAsString().c_str());
+ printf("Call: %s\n",
+ Call->getDirectCallee()->getQualifiedNameAsString().c_str());
+ }
+
if (InnerCall && InnerCall->getDirectCallee() &&
- InnerCall->getDirectCallee()->getNameAsString() ==
- Call->getDirectCallee()->getNameAsString()) {
+ InnerCall->getDirectCallee()->getQualifiedNameAsString() ==
+ Call->getDirectCallee()->getQualifiedNameAsString()) {
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) {
+ const bool ProcessInnerResult =
+ (!Result.Compare && !InnerResult.Compare) ||
+ utils::areStatementsIdentical(Result.Compare, InnerResult.Compare,
+ *Match.Context);
+
+ if (ProcessInnerResult) {
Result.Args.insert(Result.Args.end(), InnerResult.Args.begin(),
InnerResult.Args.end());
+ Result.Last = InnerResult.Last;
continue;
}
}
- if (Arg == Result.Compare)
- continue;
-
Result.Args.push_back(Arg);
Result.Last = Arg;
}
@@ -136,9 +162,16 @@ MinMaxUseInitializerListCheck::findArgs(const MatchFinder::MatchResult &Match,
return Result;
}
-std::string MinMaxUseInitializerListCheck::generateReplacement(
- const MatchFinder::MatchResult &Match, const CallExpr *TopCall,
- const FindArgsResult Result) {
+static const std::string
+generateReplacement(const MatchFinder::MatchResult &Match,
+ const CallExpr *TopCall, const FindArgsResult &Result) {
+
+ const QualType ResultType = TopCall->getDirectCallee()
+ ->getReturnType()
+ .getNonReferenceType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+
std::string ReplacementText =
Lexer::getSourceText(
CharSourceRange::getTokenRange(
@@ -147,15 +180,16 @@ std::string MinMaxUseInitializerListCheck::generateReplacement(
*Match.SourceManager, Match.Context->getLangOpts())
.str() +
"{";
- const QualType ResultType =
- TopCall->getDirectCallee()->getReturnType().getNonReferenceType();
for (const Expr *Arg : Result.Args) {
- QualType ArgType = Arg->getType();
+ const QualType ArgType = Arg->IgnoreParenImpCasts()
+ ->getType()
+ .getUnqualifiedType()
+ .getCanonicalType();
if (const auto *InnerCall = dyn_cast<CallExpr>(Arg)) {
if (InnerCall->getDirectCallee()) {
- std::string InnerCallNameStr =
+ const std::string InnerCallNameStr =
InnerCall->getDirectCallee()->getQualifiedNameAsString();
if (InnerCallNameStr !=
@@ -163,15 +197,17 @@ std::string MinMaxUseInitializerListCheck::generateReplacement(
(InnerCallNameStr == "std::min" ||
InnerCallNameStr == "std::max")) {
FindArgsResult innerResult = findArgs(Match, InnerCall);
- ReplacementText +=
- generateReplacement(Match, InnerCall, innerResult) + ", ";
- continue;
+ if (innerResult.Args.size() > 2) {
+ ReplacementText +=
+ generateReplacement(Match, InnerCall, innerResult) + ", ";
+
+ continue;
+ }
}
}
}
- bool CastNeeded =
- ArgType.getCanonicalType() != ResultType.getCanonicalType();
+ const bool CastNeeded = ArgType != ResultType;
if (CastNeeded)
ReplacementText += "static_cast<" + ResultType.getAsString() + ">(";
diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h
index 43d95004511430..65907d79457834 100644
--- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h
@@ -14,7 +14,7 @@
namespace clang::tidy::modernize {
-/// Replaces chained ``std::min`` and ``std::max`` calls with a initializer list
+/// Replaces nested ``std::min`` and ``std::max`` calls with an initializer list
/// where applicable.
///
/// For example:
@@ -46,18 +46,7 @@ class MinMaxUseInitializerListCheck : public ClangTidyCheck {
}
private:
- struct FindArgsResult {
- const Expr *First;
- const Expr *Last;
- const Expr *Compare;
- std::vector<const Expr *> Args;
- };
utils::IncludeInserter Inserter;
- 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 13d14fa9ccf0aa..851e1f8a389a4d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -120,7 +120,7 @@ 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 with a initializer list
+ Replaces nested ``std::min`` and ``std::max`` calls with an initializer list
where applicable.
- New :doc:`modernize-use-designated-initializers
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
index 0748e180897948..a166ec4b98cbc7 100644
--- 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
@@ -3,7 +3,7 @@
modernize-min-max-use-initializer-list
======================================
-Replaces chained ``std::min`` and ``std::max`` calls with a initializer list where applicable.
+Replaces nested ``std::min`` and ``std::max`` calls with an initializer list where applicable.
For instance, consider the following code:
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
index 3e15774f7b3f14..931854286374ed 100644
--- 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
@@ -1,27 +1,96 @@
// RUN: %check_clang_tidy %s modernize-min-max-use-initializer-list %t
// CHECK-FIXES: #include <algorithm>
+namespace utils {
+template <typename T>
+T max(T a, T b) {
+ return (a < b) ? b : a;
+}
+} // namespace utils
+
namespace std {
+template< class T >
+struct initializer_list {
+ initializer_list()=default;
+ initializer_list(T*,int){}
+ const T* begin() const {return nullptr;}
+ const T* end() const {return nullptr;}
+};
+
+template<class ForwardIt>
+ForwardIt min_element(ForwardIt first, ForwardIt last)
+{
+ if (first == last)
+ return last;
+
+ ForwardIt smallest = first;
+
+ while (++first != last)
+ if (*first < *smallest)
+ smallest = first;
+
+ return smallest;
+}
+
+template<class ForwardIt>
+ForwardIt max_element(ForwardIt first, ForwardIt last)
+{
+ if (first == last)
+ return last;
+
+ ForwardIt largest = first;
+
+ while (++first != last)
+ if (*largest < *first)
+ largest = first;
+
+ return largest;
+}
+
template< class T >
const T& max( const T& a, const T& b ) {
return (a < b) ? b : a;
};
+template< class T >
+T max(std::initializer_list<T> ilist)
+{
+ return *std::max_element(ilist.begin(), ilist.end());
+}
+
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, class Compare >
+T max(std::initializer_list<T> ilist, Compare comp) {
+ return *std::max_element(ilist.begin(), ilist.end(), comp);
+};
+
template< class T >
const T& min( const T& a, const T& b ) {
return (b < a) ? b : a;
};
+template< class T >
+T min(std::initializer_list<T> ilist)
+{
+ return *std::min_element(ilist.begin(), ilist.end());
+}
+
+
template< class T, class Compare >
const T& min( const T& a, const T& b, Compare comp ) {
return (comp(b, a)) ? b : a;
};
-}
+
+template< class T, class Compare >
+T min(std::initializer_list<T> ilist, Compare comp) {
+ return *std::min_element(ilist.begin(), ilist.end(), comp);
+};
+
+} // namespace std
using namespace std;
@@ -37,29 +106,45 @@ 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-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-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-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 max2b = std::max(std::max(std::max(1, 2), std::max(3, 4)), std::max(std::max(5, 6), std::max(7, 8)));
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use nested 'std::max' calls, use 'std::max({1, 2, 3, 4, 5, 6, 7, 8})' instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: int max2b = std::max({1, 2, 3, 4, 5, 6, 7, 8});
+
+int max2c = std::max(std::max(1, std::max(2, 3)), 4);
+// CHECK-MESSAGES: :[[@LINE-1]]:13: 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 max2c = std::max({1, 2, 3, 4});
+
+int max2d = std::max(std::max({1, 2, 3}), 4);
+// CHECK-MESSAGES: :[[@LINE-1]]:13: 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 max2d = std::max({1, 2, 3, 4});
+
+int max2e = std::max(1, max(2, max(3, 4)));
+// CHECK-MESSAGES: :[[@LINE-1]]:13: 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 max2e = 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-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-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-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);
@@ -75,11 +160,11 @@ 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-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-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);
@@ -95,11 +180,31 @@ 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-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-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
+int min10 = std::min(std::min(4, 5), std::max(2, utils::max(3, 1)));
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use nested 'std::min' calls, use 'std::min({4, 5, std::max(2, utils::max(3, 1))})' instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: int min10 = std::min({4, 5, std::max(2, utils::max(3, 1))});
+
+int typecastTest = std::max(std::max<int>(0U, 0.0f), 0);
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: do not use nested 'std::max' calls, use 'std::max({static_cast<int>(0U), static_cast<int>(0.0f), 0})' instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: int typecastTest = std::max({static_cast<int>(0U), static_cast<int>(0.0f), 0});
+
+int typecastTest1 = std::max(std::max<long>(0U, 0.0f), 0L);
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: do not use nested 'std::max' calls, use 'std::max({static_cast<long>(0U), static_cast<long>(0.0f), 0L})' instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: int typecastTest1 = std::max({static_cast<long>(0U), static_cast<long>(0.0f), 0L});
+
+int typecastTest2 = std::max(std::max<int>(10U, 20.0f), 30);
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: do not use nested 'std::max' calls, use 'std::max({static_cast<int>(10U), static_cast<int>(20.0f), 30})' instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: int typecastTest2 = std::max({static_cast<int>(10U), static_cast<int>(20.0f), 30});
+
+int typecastTest3 = std::max(std::max<int>(0U, std::max<int>(0.0f, 1.0f)), 0);
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: do not use nested 'std::max' calls, use 'std::max({static_cast<int>(0U), static_cast<int>(0.0f), static_cast<int>(1.0f), 0})' instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: int typecastTest3 = std::max({static_cast<int>(0U), static_cast<int>(0.0f), static_cast<int>(1.0f), 0});
+
+}
>From 40273795cf0736dad10ad7aa4eda82267f0ce304 Mon Sep 17 00:00:00 2001
From: sopy <contact at sopy.one>
Date: Fri, 29 Mar 2024 03:39:40 +0200
Subject: [PATCH 5/9] Refactor generateReplacement to return FixItHints to
avoid check conflicts
---
.../MinMaxUseInitializerListCheck.cpp | 318 +++++++++---------
.../min-max-use-initializer-list.rst | 2 +-
.../min-max-use-initializer-list.cpp | 129 ++++---
3 files changed, 257 insertions(+), 192 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
index baf3da851e7f40..7e0ce414bf0ef2 100644
--- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
@@ -8,85 +8,29 @@
#include "MinMaxUseInitializerListCheck.h"
#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Lex/Lexer.h"
-using namespace clang::ast_matchers;
+using namespace clang;
-namespace clang::tidy::modernize {
+namespace {
struct FindArgsResult {
const Expr *First;
const Expr *Last;
const Expr *Compare;
- std::vector<const Expr *> Args;
+ std::vector<const clang::Expr *> Args;
};
-static const FindArgsResult findArgs(const MatchFinder::MatchResult &Match,
- const CallExpr *Call);
-static const std::string
-generateReplacement(const MatchFinder::MatchResult &Match,
- const CallExpr *TopCall, const FindArgsResult &Result);
-
-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) {
- auto createMatcher = [](const std::string &functionName) {
- auto funcDecl = functionDecl(hasName(functionName));
- auto expr = callExpr(callee(funcDecl));
-
- return callExpr(callee(funcDecl),
- anyOf(hasArgument(0, expr), hasArgument(1, expr)),
- unless(hasParent(expr)))
- .bind("topCall");
- };
-
- Finder->addMatcher(createMatcher("::std::max"), this);
- Finder->addMatcher(createMatcher("::std::min"), this);
-}
-
-void MinMaxUseInitializerListCheck::registerPPCallbacks(
- const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
- Inserter.registerPreprocessor(PP);
-}
-
-void MinMaxUseInitializerListCheck::check(
- const MatchFinder::MatchResult &Match) {
-
- const auto *TopCall = Match.Nodes.getNodeAs<CallExpr>("topCall");
- FindArgsResult Result = findArgs(Match, TopCall);
+} // anonymous namespace
- if (Result.Args.size() <= 2) {
- return;
- }
-
- const std::string ReplacementText =
- generateReplacement(Match, TopCall, Result);
+using namespace clang::ast_matchers;
- diag(TopCall->getBeginLoc(),
- "do not use nested 'std::%0' calls, use '%1' instead")
- << TopCall->getDirectCallee()->getName() << ReplacementText
- << FixItHint::CreateReplacement(
- CharSourceRange::getTokenRange(TopCall->getSourceRange()),
- ReplacementText)
- << Inserter.createIncludeInsertion(
- Match.SourceManager->getFileID(TopCall->getBeginLoc()),
- "<algorithm>");
-}
+namespace clang::tidy::modernize {
-static const FindArgsResult findArgs(const MatchFinder::MatchResult &Match,
- const CallExpr *Call) {
+static FindArgsResult findArgs(const CallExpr *Call) {
FindArgsResult Result;
Result.First = nullptr;
Result.Last = nullptr;
@@ -101,22 +45,19 @@ static const FindArgsResult findArgs(const MatchFinder::MatchResult &Match,
if (const auto *InitListExpr =
dyn_cast<CXXStdInitializerListExpr>(*ArgIterator)) {
- if (const auto *TempExpr =
- dyn_cast<MaterializeTemporaryExpr>(InitListExpr->getSubExpr())) {
- if (const auto *InitList =
- dyn_cast<clang::InitListExpr>(TempExpr->getSubExpr())) {
- for (const Expr *Init : InitList->inits()) {
- Result.Args.push_back(Init);
- }
- Result.First = *ArgIterator;
- Result.Last = *ArgIterator;
+ if (const auto *InitList = dyn_cast<clang::InitListExpr>(
+ InitListExpr->getSubExpr()->IgnoreImplicit())) {
+ Result.Args.insert(Result.Args.begin(), InitList->inits().begin(),
+ InitList->inits().end());
- std::advance(ArgIterator, 1);
- if (ArgIterator != Call->arguments().end()) {
- Result.Compare = *ArgIterator;
- }
- return Result;
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+ Result.Compare = *ArgIterator;
}
+ return Result;
}
}
}
@@ -128,33 +69,6 @@ static const FindArgsResult findArgs(const MatchFinder::MatchResult &Match,
if (Arg == Result.Compare)
continue;
- const auto *InnerCall = dyn_cast<CallExpr>(Arg->IgnoreParenImpCasts());
-
- if (InnerCall) {
- printf("InnerCall: %s\n",
- InnerCall->getDirectCallee()->getQualifiedNameAsString().c_str());
- printf("Call: %s\n",
- Call->getDirectCallee()->getQualifiedNameAsString().c_str());
- }
-
- if (InnerCall && InnerCall->getDirectCallee() &&
- InnerCall->getDirectCallee()->getQualifiedNameAsString() ==
- Call->getDirectCallee()->getQualifiedNameAsString()) {
- FindArgsResult InnerResult = findArgs(Match, InnerCall);
-
- const bool ProcessInnerResult =
- (!Result.Compare && !InnerResult.Compare) ||
- utils::areStatementsIdentical(Result.Compare, InnerResult.Compare,
- *Match.Context);
-
- if (ProcessInnerResult) {
- Result.Args.insert(Result.Args.end(), InnerResult.Args.begin(),
- InnerResult.Args.end());
- Result.Last = InnerResult.Last;
- continue;
- }
- }
-
Result.Args.push_back(Arg);
Result.Last = Arg;
}
@@ -162,74 +76,178 @@ static const FindArgsResult findArgs(const MatchFinder::MatchResult &Match,
return Result;
}
-static const std::string
+static std::vector<FixItHint>
generateReplacement(const MatchFinder::MatchResult &Match,
const CallExpr *TopCall, const FindArgsResult &Result) {
+ std::vector<FixItHint> FixItHints;
const QualType ResultType = TopCall->getDirectCallee()
->getReturnType()
.getNonReferenceType()
.getUnqualifiedType()
.getCanonicalType();
+ const bool IsInitializerList = Result.First == Result.Last;
- std::string ReplacementText =
- Lexer::getSourceText(
- CharSourceRange::getTokenRange(
- TopCall->getBeginLoc(),
- Result.First->getBeginLoc().getLocWithOffset(-1)),
- *Match.SourceManager, Match.Context->getLangOpts())
- .str() +
- "{";
+ if (!IsInitializerList)
+ FixItHints.push_back(
+ FixItHint::CreateInsertion(Result.First->getBeginLoc(), "{"));
for (const Expr *Arg : Result.Args) {
+ if (const auto *InnerCall =
+ dyn_cast<CallExpr>(Arg->IgnoreParenImpCasts())) {
+ const auto InnerResult = findArgs(InnerCall);
+ const auto InnerReplacement =
+ generateReplacement(Match, InnerCall, InnerResult);
+ if (InnerCall->getDirectCallee()->getQualifiedNameAsString() ==
+ TopCall->getDirectCallee()->getQualifiedNameAsString() &&
+ ((!Result.Compare && !InnerResult.Compare) ||
+ utils::areStatementsIdentical(Result.Compare, InnerResult.Compare,
+ *Match.Context))) {
+
+ FixItHints.push_back(
+ FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+ InnerCall->getCallee()->getSourceRange())));
+
+ const auto LParen = utils::lexer::findNextTokenSkippingComments(
+ InnerCall->getCallee()->getEndLoc(), *Match.SourceManager,
+ Match.Context->getLangOpts());
+ if (LParen && LParen->getKind() == tok::l_paren)
+ FixItHints.push_back(
+ FixItHint::CreateRemoval(SourceRange(LParen->getLocation())));
+
+ FixItHints.push_back(
+ FixItHint::CreateRemoval(SourceRange(InnerCall->getRParenLoc())));
+
+ if (InnerResult.First == InnerResult.Last) {
+ FixItHints.insert(FixItHints.end(), InnerReplacement.begin(),
+ InnerReplacement.end());
+
+ FixItHints.push_back(
+ FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+ InnerResult.First->getBeginLoc())));
+ FixItHints.push_back(FixItHint::CreateRemoval(
+ CharSourceRange::getTokenRange(InnerResult.First->getEndLoc())));
+ } else
+ FixItHints.insert(FixItHints.end(), InnerReplacement.begin() + 1,
+ InnerReplacement.end() - 1);
+
+ if (InnerResult.Compare) {
+ const std::optional<Token> Comma =
+ utils::lexer::findNextTokenSkippingComments(
+ InnerResult.Last->getEndLoc(), *Match.SourceManager,
+ Match.Context->getLangOpts());
+ if (Comma && Comma->getKind() == tok::comma)
+ FixItHints.push_back(
+ FixItHint::CreateRemoval(SourceRange(Comma->getLocation())));
+
+ if (utils::lexer::getPreviousToken(
+ InnerResult.Compare->getExprLoc(), *Match.SourceManager,
+ Match.Context->getLangOpts(), false)
+ .getLocation() == Comma->getLocation()) {
+ FixItHints.push_back(
+ FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+ Comma->getLocation(), InnerResult.Compare->getEndLoc())));
+ } else {
+ FixItHints.push_back(
+ FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+ InnerResult.Compare->getSourceRange())));
+ }
+ }
+ }
+ continue;
+ }
+
const QualType ArgType = Arg->IgnoreParenImpCasts()
->getType()
.getUnqualifiedType()
.getCanonicalType();
- if (const auto *InnerCall = dyn_cast<CallExpr>(Arg)) {
- if (InnerCall->getDirectCallee()) {
- const std::string InnerCallNameStr =
- InnerCall->getDirectCallee()->getQualifiedNameAsString();
-
- if (InnerCallNameStr !=
- TopCall->getDirectCallee()->getQualifiedNameAsString() &&
- (InnerCallNameStr == "std::min" ||
- InnerCallNameStr == "std::max")) {
- FindArgsResult innerResult = findArgs(Match, InnerCall);
- if (innerResult.Args.size() > 2) {
- ReplacementText +=
- generateReplacement(Match, InnerCall, innerResult) + ", ";
-
- continue;
- }
- }
- }
+ if (ArgType != ResultType) {
+ const std::string ArgText =
+ Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Arg->getSourceRange()),
+ *Match.SourceManager, Match.Context->getLangOpts())
+ .str();
+
+ FixItHints.push_back(FixItHint::CreateReplacement(
+ Arg->getSourceRange(),
+ "static_cast<" + ResultType.getAsString() + ">(" + ArgText + ")"));
}
+ }
- const bool CastNeeded = ArgType != ResultType;
+ if (!IsInitializerList) {
+ if (Result.Compare)
+ FixItHints.push_back(FixItHint::CreateInsertion(
+ Lexer::getLocForEndOfToken(Result.Last->getEndLoc(), 0,
+ *Match.SourceManager,
+ Match.Context->getLangOpts()),
+ "}"));
+ else
+ FixItHints.push_back(
+ FixItHint::CreateInsertion(TopCall->getEndLoc(), "}"));
+ }
- if (CastNeeded)
- ReplacementText += "static_cast<" + ResultType.getAsString() + ">(";
+ return FixItHints;
+}
- ReplacementText += Lexer::getSourceText(
- CharSourceRange::getTokenRange(Arg->getSourceRange()),
- *Match.SourceManager, Match.Context->getLangOpts());
+MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ Inserter(Options.getLocalOrGlobal("IncludeStyle",
+ utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
- if (CastNeeded)
- ReplacementText += ")";
- ReplacementText += ", ";
- }
- ReplacementText = ReplacementText.substr(0, ReplacementText.size() - 2) + "}";
- if (Result.Compare) {
- ReplacementText += ", ";
- ReplacementText += Lexer::getSourceText(
- CharSourceRange::getTokenRange(Result.Compare->getSourceRange()),
- *Match.SourceManager, Match.Context->getLangOpts());
+void MinMaxUseInitializerListCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+}
+
+void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) {
+ auto CreateMatcher = [](const StringRef &FunctionName) {
+ auto FuncDecl = functionDecl(hasName(FunctionName));
+ auto Expression = callExpr(callee(FuncDecl));
+
+ return callExpr(callee(FuncDecl),
+ anyOf(hasArgument(0, Expression),
+ hasArgument(1, Expression),
+ hasArgument(0, cxxStdInitializerListExpr())),
+ unless(hasParent(Expression)))
+ .bind("topCall");
+ };
+
+ Finder->addMatcher(CreateMatcher("::std::max"), this);
+ Finder->addMatcher(CreateMatcher("::std::min"), this);
+}
+
+void MinMaxUseInitializerListCheck::registerPPCallbacks(
+ const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+ Inserter.registerPreprocessor(PP);
+}
+
+void MinMaxUseInitializerListCheck::check(
+ const MatchFinder::MatchResult &Match) {
+
+ const auto *TopCall = Match.Nodes.getNodeAs<CallExpr>("topCall");
+
+ const FindArgsResult Result = findArgs(TopCall);
+ const std::vector<FixItHint> Replacement =
+ generateReplacement(Match, TopCall, Result);
+
+ if (Replacement.size() <= 2) {
+ return;
}
- ReplacementText += ")";
- return ReplacementText;
+ const DiagnosticBuilder Diagnostic =
+ diag(TopCall->getBeginLoc(),
+ "do not use nested 'std::%0' calls, use an initializer list instead")
+ << TopCall->getDirectCallee()->getName()
+ << Inserter.createIncludeInsertion(
+ Match.SourceManager->getFileID(TopCall->getBeginLoc()),
+ "<algorithm>");
+
+ for (const auto &FixIt : Replacement) {
+ Diagnostic << FixIt;
+ }
}
} // namespace clang::tidy::modernize
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
index a166ec4b98cbc7..b25dbf04b2fde9 100644
--- 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
@@ -11,7 +11,7 @@ For instance, consider the following code:
int a = std::max(std::max(i, j), k);
-`modernize-min-max-use-initializer-list` check will transform the above code to:
+The `modernize-min-max-use-initializer-list` check will transform the above code to:
.. code-block:: cpp
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
index 931854286374ed..16f2f6ce252a61 100644
--- 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
@@ -32,6 +32,21 @@ ForwardIt min_element(ForwardIt first, ForwardIt last)
return smallest;
}
+template<class ForwardIt, class Compare>
+ForwardIt min_element(ForwardIt first, ForwardIt last, Compare comp)
+{
+ if (first == last)
+ return last;
+
+ ForwardIt smallest = first;
+
+ while (++first != last)
+ if (comp(*first, *smallest))
+ smallest = first;
+
+ return smallest;
+}
+
template<class ForwardIt>
ForwardIt max_element(ForwardIt first, ForwardIt last)
{
@@ -47,6 +62,21 @@ ForwardIt max_element(ForwardIt first, ForwardIt last)
return largest;
}
+template<class ForwardIt, class Compare>
+ForwardIt max_element(ForwardIt first, ForwardIt last, Compare comp)
+{
+ if (first == last)
+ return last;
+
+ ForwardIt largest = first;
+
+ while(++first != last)
+ if (comp(*largest, *first))
+ largest = first;
+
+ return largest;
+}
+
template< class T >
const T& max( const T& a, const T& b ) {
return (a < b) ? b : a;
@@ -106,105 +136,122 @@ 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-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested 'std::max' calls, use an initializer list 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-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested 'std::min' calls, use an initializer list 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-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested 'std::max' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
// CHECK-FIXES: int max2 = std::max({1, 2, 3, 4});
int max2b = std::max(std::max(std::max(1, 2), std::max(3, 4)), std::max(std::max(5, 6), std::max(7, 8)));
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use nested 'std::max' calls, use 'std::max({1, 2, 3, 4, 5, 6, 7, 8})' instead [modernize-min-max-use-initializer-list]
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use nested 'std::max' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
// CHECK-FIXES: int max2b = std::max({1, 2, 3, 4, 5, 6, 7, 8});
int max2c = std::max(std::max(1, std::max(2, 3)), 4);
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use nested 'std::max' calls, use 'std::max({1, 2, 3, 4})' instead [modernize-min-max-use-initializer-list]
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use nested 'std::max' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
// CHECK-FIXES: int max2c = std::max({1, 2, 3, 4});
int max2d = std::max(std::max({1, 2, 3}), 4);
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use nested 'std::max' calls, use 'std::max({1, 2, 3, 4})' instead [modernize-min-max-use-initializer-list]
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use nested 'std::max' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
// CHECK-FIXES: int max2d = std::max({1, 2, 3, 4});
int max2e = std::max(1, max(2, max(3, 4)));
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use nested 'std::max' calls, use 'std::max({1, 2, 3, 4})' instead [modernize-min-max-use-initializer-list]
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use nested 'std::max' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
// CHECK-FIXES: int max2e = 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-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested 'std::min' calls, use an initializer list 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-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested 'std::max' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
+// CHECK-MESSAGES: :[[@LINE-2]]:37: warning: do not use nested 'std::min' calls, use an initializer list 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-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested 'std::min' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
+// CHECK-MESSAGES: :[[@LINE-2]]:37: warning: do not use nested 'std::max' calls, use an initializer list 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 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 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 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 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]
+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 an initializer list 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]
+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 an initializer list 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 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 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 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 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]
+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 an initializer list 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]
+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 an initializer list instead [modernize-min-max-use-initializer-list]
// CHECK-FIXES: int min9 = std::min({1, 2, 3}, fless_than);
int min10 = std::min(std::min(4, 5), std::max(2, utils::max(3, 1)));
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use nested 'std::min' calls, use 'std::min({4, 5, std::max(2, utils::max(3, 1))})' instead [modernize-min-max-use-initializer-list]
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use nested 'std::min' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
// CHECK-FIXES: int min10 = std::min({4, 5, std::max(2, utils::max(3, 1))});
+int max10 = std::max({std::max(1, 2), std::max({5, 6, 1}), 2, std::min({1, 2, 4})});
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use nested 'std::max' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: int max10 = std::max({1, 2, 5, 6, 1, 2, std::min({1, 2, 4})});
+
int typecastTest = std::max(std::max<int>(0U, 0.0f), 0);
-// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: do not use nested 'std::max' calls, use 'std::max({static_cast<int>(0U), static_cast<int>(0.0f), 0})' instead [modernize-min-max-use-initializer-list]
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: do not use nested 'std::max' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
// CHECK-FIXES: int typecastTest = std::max({static_cast<int>(0U), static_cast<int>(0.0f), 0});
int typecastTest1 = std::max(std::max<long>(0U, 0.0f), 0L);
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: do not use nested 'std::max' calls, use 'std::max({static_cast<long>(0U), static_cast<long>(0.0f), 0L})' instead [modernize-min-max-use-initializer-list]
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: do not use nested 'std::max' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
// CHECK-FIXES: int typecastTest1 = std::max({static_cast<long>(0U), static_cast<long>(0.0f), 0L});
int typecastTest2 = std::max(std::max<int>(10U, 20.0f), 30);
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: do not use nested 'std::max' calls, use 'std::max({static_cast<int>(10U), static_cast<int>(20.0f), 30})' instead [modernize-min-max-use-initializer-list]
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: do not use nested 'std::max' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
// CHECK-FIXES: int typecastTest2 = std::max({static_cast<int>(10U), static_cast<int>(20.0f), 30});
int typecastTest3 = std::max(std::max<int>(0U, std::max<int>(0.0f, 1.0f)), 0);
-// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: do not use nested 'std::max' calls, use 'std::max({static_cast<int>(0U), static_cast<int>(0.0f), static_cast<int>(1.0f), 0})' instead [modernize-min-max-use-initializer-list]
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: do not use nested 'std::max' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
// CHECK-FIXES: int typecastTest3 = std::max({static_cast<int>(0U), static_cast<int>(0.0f), static_cast<int>(1.0f), 0});
+#define max3f(a, b, c) std::max(a, std::max(b, c))
+// CHECK-FIXES: #define max3f(a, b, c) std::max(a, std::max(b, c))
+
+#define value 4545
+int macroVarMax = std::max(value, std::max(1, 2));
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: do not use nested 'std::max' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: int macroVarMax = std::max({value, 1, 2});
+
+#define value2 45U
+int macroVarMax2 = std::max(1, std::max<int>(value2, 2.0f));
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: do not use nested 'std::max' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: int macroVarMax2 = std::max({1, static_cast<int>(value2), static_cast<int>(2.0f)});
+
}
>From 1bd655030b8f19fe8a736c5e04ced3d30eac001d Mon Sep 17 00:00:00 2001
From: sopy <contact at sopy.one>
Date: Sun, 31 Mar 2024 15:21:26 +0300
Subject: [PATCH 6/9] Update documentation
---
.../checks/modernize/min-max-use-initializer-list.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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
index b25dbf04b2fde9..25fc047452849d 100644
--- 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
@@ -11,7 +11,7 @@ For instance, consider the following code:
int a = std::max(std::max(i, j), k);
-The `modernize-min-max-use-initializer-list` check will transform the above code to:
+The check will transform the above code to:
.. code-block:: cpp
>From 61ceb147b8befffcb652bfc1e20621d25b156408 Mon Sep 17 00:00:00 2001
From: sopy <contact at sopy.one>
Date: Sun, 31 Mar 2024 15:36:44 +0300
Subject: [PATCH 7/9] consistency changes
---
.../modernize/MinMaxUseInitializerListCheck.cpp | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
index 7e0ce414bf0ef2..9fd045fcee0915 100644
--- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
@@ -95,8 +95,8 @@ generateReplacement(const MatchFinder::MatchResult &Match,
for (const Expr *Arg : Result.Args) {
if (const auto *InnerCall =
dyn_cast<CallExpr>(Arg->IgnoreParenImpCasts())) {
- const auto InnerResult = findArgs(InnerCall);
- const auto InnerReplacement =
+ const FindArgsResult InnerResult = findArgs(InnerCall);
+ const std::vector<FixItHint> InnerReplacements =
generateReplacement(Match, InnerCall, InnerResult);
if (InnerCall->getDirectCallee()->getQualifiedNameAsString() ==
TopCall->getDirectCallee()->getQualifiedNameAsString() &&
@@ -119,8 +119,8 @@ generateReplacement(const MatchFinder::MatchResult &Match,
FixItHint::CreateRemoval(SourceRange(InnerCall->getRParenLoc())));
if (InnerResult.First == InnerResult.Last) {
- FixItHints.insert(FixItHints.end(), InnerReplacement.begin(),
- InnerReplacement.end());
+ FixItHints.insert(FixItHints.end(), InnerReplacements.begin(),
+ InnerReplacements.end());
FixItHints.push_back(
FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
@@ -128,11 +128,11 @@ generateReplacement(const MatchFinder::MatchResult &Match,
FixItHints.push_back(FixItHint::CreateRemoval(
CharSourceRange::getTokenRange(InnerResult.First->getEndLoc())));
} else
- FixItHints.insert(FixItHints.end(), InnerReplacement.begin() + 1,
- InnerReplacement.end() - 1);
+ FixItHints.insert(FixItHints.end(), InnerReplacements.begin() + 1,
+ InnerReplacements.end() - 1);
if (InnerResult.Compare) {
- const std::optional<Token> Comma =
+ const auto Comma =
utils::lexer::findNextTokenSkippingComments(
InnerResult.Last->getEndLoc(), *Match.SourceManager,
Match.Context->getLangOpts());
@@ -203,7 +203,7 @@ void MinMaxUseInitializerListCheck::storeOptions(
}
void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) {
- auto CreateMatcher = [](const StringRef &FunctionName) {
+ auto CreateMatcher = [](const StringRef FunctionName) {
auto FuncDecl = functionDecl(hasName(FunctionName));
auto Expression = callExpr(callee(FuncDecl));
>From d506f63a2f38472f49ba44067eee6e30743d1e70 Mon Sep 17 00:00:00 2001
From: sopy <contact at sopy.one>
Date: Sun, 31 Mar 2024 15:42:49 +0300
Subject: [PATCH 8/9] Remove braces from if statement
---
.../modernize/MinMaxUseInitializerListCheck.cpp | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
index 9fd045fcee0915..d770578761a28e 100644
--- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
@@ -132,10 +132,9 @@ generateReplacement(const MatchFinder::MatchResult &Match,
InnerReplacements.end() - 1);
if (InnerResult.Compare) {
- const auto Comma =
- utils::lexer::findNextTokenSkippingComments(
- InnerResult.Last->getEndLoc(), *Match.SourceManager,
- Match.Context->getLangOpts());
+ const auto Comma = utils::lexer::findNextTokenSkippingComments(
+ InnerResult.Last->getEndLoc(), *Match.SourceManager,
+ Match.Context->getLangOpts());
if (Comma && Comma->getKind() == tok::comma)
FixItHints.push_back(
FixItHint::CreateRemoval(SourceRange(Comma->getLocation())));
@@ -143,11 +142,11 @@ generateReplacement(const MatchFinder::MatchResult &Match,
if (utils::lexer::getPreviousToken(
InnerResult.Compare->getExprLoc(), *Match.SourceManager,
Match.Context->getLangOpts(), false)
- .getLocation() == Comma->getLocation()) {
+ .getLocation() == Comma->getLocation())
FixItHints.push_back(
FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
Comma->getLocation(), InnerResult.Compare->getEndLoc())));
- } else {
+ else {
FixItHints.push_back(
FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
InnerResult.Compare->getSourceRange())));
>From 6abf8a26d6b880d0c8ef155c43d5d6d12dc2d527 Mon Sep 17 00:00:00 2001
From: sopy <contact at sopy.one>
Date: Mon, 1 Apr 2024 18:56:50 +0300
Subject: [PATCH 9/9] Adding comments and code cleanup
---
.../MinMaxUseInitializerListCheck.cpp | 236 ++++++++++--------
.../min-max-use-initializer-list.cpp | 1 +
2 files changed, 131 insertions(+), 106 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
index d770578761a28e..7c7d93cc17518e 100644
--- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
@@ -21,7 +21,7 @@ struct FindArgsResult {
const Expr *First;
const Expr *Last;
const Expr *Compare;
- std::vector<const clang::Expr *> Args;
+ SmallVector<const clang::Expr *, 2> Args;
};
} // anonymous namespace
@@ -36,30 +36,35 @@ static FindArgsResult findArgs(const CallExpr *Call) {
Result.Last = nullptr;
Result.Compare = nullptr;
- if (Call->getNumArgs() == 3) {
- auto ArgIterator = Call->arguments().begin();
- std::advance(ArgIterator, 2);
- Result.Compare = *ArgIterator;
- } else {
+ // check if the function has initializer list argument
+ if (Call->getNumArgs() < 3) {
auto ArgIterator = Call->arguments().begin();
- if (const auto *InitListExpr =
- dyn_cast<CXXStdInitializerListExpr>(*ArgIterator)) {
- if (const auto *InitList = dyn_cast<clang::InitListExpr>(
- InitListExpr->getSubExpr()->IgnoreImplicit())) {
- Result.Args.insert(Result.Args.begin(), InitList->inits().begin(),
- InitList->inits().end());
-
- Result.First = *ArgIterator;
- Result.Last = *ArgIterator;
-
- std::advance(ArgIterator, 1);
- if (ArgIterator != Call->arguments().end()) {
- Result.Compare = *ArgIterator;
- }
- return Result;
+ const auto *InitListExpr =
+ dyn_cast<CXXStdInitializerListExpr>(*ArgIterator);
+ const auto *InitList =
+ InitListExpr != nullptr
+ ? dyn_cast<clang::InitListExpr>(
+ InitListExpr->getSubExpr()->IgnoreImplicit())
+ : nullptr;
+
+ if (InitListExpr && InitList) {
+ Result.Args.insert(Result.Args.begin(), InitList->inits().begin(),
+ InitList->inits().end());
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ // check if there is a comparison argument
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+ Result.Compare = *ArgIterator;
}
+
+ return Result;
}
+ // if it has 3 arguments then the last will be the comparison
+ } else {
+ Result.Compare = *(std::next(Call->arguments().begin(), 2));
}
for (const Expr *Arg : Call->arguments()) {
@@ -76,114 +81,133 @@ static FindArgsResult findArgs(const CallExpr *Call) {
return Result;
}
-static std::vector<FixItHint>
+static SmallVector<FixItHint>
generateReplacement(const MatchFinder::MatchResult &Match,
const CallExpr *TopCall, const FindArgsResult &Result) {
- std::vector<FixItHint> FixItHints;
+ SmallVector<FixItHint> FixItHints;
const QualType ResultType = TopCall->getDirectCallee()
->getReturnType()
.getNonReferenceType()
.getUnqualifiedType()
.getCanonicalType();
+ const auto &SourceMngr = *Match.SourceManager;
+ const auto LanguageOpts = Match.Context->getLangOpts();
const bool IsInitializerList = Result.First == Result.Last;
- if (!IsInitializerList)
+ // add { and } if the top call doesn't have an initializer list arg
+ if (!IsInitializerList) {
FixItHints.push_back(
FixItHint::CreateInsertion(Result.First->getBeginLoc(), "{"));
+ if (Result.Compare)
+ FixItHints.push_back(FixItHint::CreateInsertion(
+ Lexer::getLocForEndOfToken(Result.Last->getEndLoc(), 0, SourceMngr,
+ LanguageOpts),
+ "}"));
+ else
+ FixItHints.push_back(
+ FixItHint::CreateInsertion(TopCall->getEndLoc(), "}"));
+ }
+
for (const Expr *Arg : Result.Args) {
- if (const auto *InnerCall =
- dyn_cast<CallExpr>(Arg->IgnoreParenImpCasts())) {
- const FindArgsResult InnerResult = findArgs(InnerCall);
- const std::vector<FixItHint> InnerReplacements =
- generateReplacement(Match, InnerCall, InnerResult);
- if (InnerCall->getDirectCallee()->getQualifiedNameAsString() ==
- TopCall->getDirectCallee()->getQualifiedNameAsString() &&
- ((!Result.Compare && !InnerResult.Compare) ||
- utils::areStatementsIdentical(Result.Compare, InnerResult.Compare,
- *Match.Context))) {
+ const auto *InnerCall = dyn_cast<CallExpr>(Arg->IgnoreParenImpCasts());
- FixItHints.push_back(
- FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
- InnerCall->getCallee()->getSourceRange())));
+ // If the argument is not a nested call
+ if (!InnerCall) {
+ // check if typecast is required
+ const QualType ArgType = Arg->IgnoreParenImpCasts()
+ ->getType()
+ .getUnqualifiedType()
+ .getCanonicalType();
- const auto LParen = utils::lexer::findNextTokenSkippingComments(
- InnerCall->getCallee()->getEndLoc(), *Match.SourceManager,
- Match.Context->getLangOpts());
- if (LParen && LParen->getKind() == tok::l_paren)
- FixItHints.push_back(
- FixItHint::CreateRemoval(SourceRange(LParen->getLocation())));
+ if (ArgType == ResultType)
+ continue;
+
+ const std::string ArgText =
+ Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Arg->getSourceRange()), SourceMngr,
+ LanguageOpts)
+ .str();
+
+ Twine Replacement = llvm::Twine("static_cast<")
+ .concat(ResultType.getAsString(LanguageOpts))
+ .concat(">(")
+ .concat(ArgText)
+ .concat(")");
+
+ FixItHints.push_back(FixItHint::CreateReplacement(Arg->getSourceRange(),
+ Replacement.str()));
- FixItHints.push_back(
- FixItHint::CreateRemoval(SourceRange(InnerCall->getRParenLoc())));
-
- if (InnerResult.First == InnerResult.Last) {
- FixItHints.insert(FixItHints.end(), InnerReplacements.begin(),
- InnerReplacements.end());
-
- FixItHints.push_back(
- FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
- InnerResult.First->getBeginLoc())));
- FixItHints.push_back(FixItHint::CreateRemoval(
- CharSourceRange::getTokenRange(InnerResult.First->getEndLoc())));
- } else
- FixItHints.insert(FixItHints.end(), InnerReplacements.begin() + 1,
- InnerReplacements.end() - 1);
-
- if (InnerResult.Compare) {
- const auto Comma = utils::lexer::findNextTokenSkippingComments(
- InnerResult.Last->getEndLoc(), *Match.SourceManager,
- Match.Context->getLangOpts());
- if (Comma && Comma->getKind() == tok::comma)
- FixItHints.push_back(
- FixItHint::CreateRemoval(SourceRange(Comma->getLocation())));
-
- if (utils::lexer::getPreviousToken(
- InnerResult.Compare->getExprLoc(), *Match.SourceManager,
- Match.Context->getLangOpts(), false)
- .getLocation() == Comma->getLocation())
- FixItHints.push_back(
- FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
- Comma->getLocation(), InnerResult.Compare->getEndLoc())));
- else {
- FixItHints.push_back(
- FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
- InnerResult.Compare->getSourceRange())));
- }
- }
- }
continue;
}
- const QualType ArgType = Arg->IgnoreParenImpCasts()
- ->getType()
- .getUnqualifiedType()
- .getCanonicalType();
+ const auto InnerResult = findArgs(InnerCall);
+ const auto InnerReplacements =
+ generateReplacement(Match, InnerCall, InnerResult);
+ const bool IsInnerInitializerList = InnerResult.First == InnerResult.Last;
- if (ArgType != ResultType) {
- const std::string ArgText =
- Lexer::getSourceText(
- CharSourceRange::getTokenRange(Arg->getSourceRange()),
- *Match.SourceManager, Match.Context->getLangOpts())
- .str();
+ // if the nested call is not the same as the top call
+ if (InnerCall->getDirectCallee()->getQualifiedNameAsString() !=
+ TopCall->getDirectCallee()->getQualifiedNameAsString())
+ continue;
+
+ // if the nested call doesn't have the same compare function
+ if ((Result.Compare || InnerResult.Compare) &&
+ !utils::areStatementsIdentical(Result.Compare, InnerResult.Compare,
+ *Match.Context))
+ continue;
+
+ // remove the function call
+ FixItHints.push_back(
+ FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+ InnerCall->getCallee()->getSourceRange())));
- FixItHints.push_back(FixItHint::CreateReplacement(
- Arg->getSourceRange(),
- "static_cast<" + ResultType.getAsString() + ">(" + ArgText + ")"));
+ // remove the parentheses
+ const auto LParen = utils::lexer::findNextTokenSkippingComments(
+ InnerCall->getCallee()->getEndLoc(), SourceMngr, LanguageOpts);
+ FixItHints.push_back(
+ FixItHint::CreateRemoval(SourceRange(LParen->getLocation())));
+ FixItHints.push_back(
+ FixItHint::CreateRemoval(SourceRange(InnerCall->getRParenLoc())));
+
+ // if the inner call has an initializer list arg
+ if (IsInnerInitializerList) {
+ // remove the initializer list braces
+ FixItHints.push_back(FixItHint::CreateRemoval(
+ CharSourceRange::getTokenRange(InnerResult.First->getBeginLoc())));
+ FixItHints.push_back(FixItHint::CreateRemoval(
+ CharSourceRange::getTokenRange(InnerResult.First->getEndLoc())));
}
- }
- if (!IsInitializerList) {
- if (Result.Compare)
- FixItHints.push_back(FixItHint::CreateInsertion(
- Lexer::getLocForEndOfToken(Result.Last->getEndLoc(), 0,
- *Match.SourceManager,
- Match.Context->getLangOpts()),
- "}"));
- else
- FixItHints.push_back(
- FixItHint::CreateInsertion(TopCall->getEndLoc(), "}"));
+ FixItHints.insert(FixItHints.end(),
+ // ignore { and } insertions for the inner call if it does
+ // not have an initializer list arg
+ InnerReplacements.begin() + (!IsInnerInitializerList) * 2,
+ InnerReplacements.end());
+
+ if (InnerResult.Compare) {
+ // find the comma after the value arguments
+ const auto Comma = utils::lexer::findNextTokenSkippingComments(
+ InnerResult.Last->getEndLoc(), SourceMngr, LanguageOpts);
+
+ // if there are comments between the comma and the comparison
+ if (utils::lexer::getPreviousToken(InnerResult.Compare->getExprLoc(),
+ SourceMngr, LanguageOpts, false)
+ .getLocation() != Comma->getLocation()) {
+ // remove the comma and the comparison
+ FixItHints.push_back(
+ FixItHint::CreateRemoval(SourceRange(Comma->getLocation())));
+
+ FixItHints.push_back(
+ FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+ InnerResult.Compare->getSourceRange())));
+ } else
+ // remove everything after the last argument
+ FixItHints.push_back(
+ FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+ Comma->getLocation(), InnerResult.Compare->getEndLoc())));
+ }
}
return FixItHints;
@@ -229,7 +253,7 @@ void MinMaxUseInitializerListCheck::check(
const auto *TopCall = Match.Nodes.getNodeAs<CallExpr>("topCall");
const FindArgsResult Result = findArgs(TopCall);
- const std::vector<FixItHint> Replacement =
+ const SmallVector<FixItHint> Replacement =
generateReplacement(Match, TopCall, Result);
if (Replacement.size() <= 2) {
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
index 16f2f6ce252a61..486e76cf4bebbf 100644
--- 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
@@ -159,6 +159,7 @@ int max2d = std::max(std::max({1, 2, 3}), 4);
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use nested 'std::max' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
// CHECK-FIXES: int max2d = std::max({1, 2, 3, 4});
+
int max2e = std::max(1, max(2, max(3, 4)));
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use nested 'std::max' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
// CHECK-FIXES: int max2e = std::max({1, 2, 3, 4});
More information about the cfe-commits
mailing list