[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)

via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 20 05:26:27 PDT 2024


https://github.com/sopyb updated https://github.com/llvm/llvm-project/pull/85572

>From 4e466541a5ccf22d6fef08c71077e1f95ed10616 Mon Sep 17 00:00:00 2001
From: sopy <contact at sopy.one>
Date: Sun, 17 Mar 2024 17:30:27 +0200
Subject: [PATCH 1/7] [clang-tidy] add check to suggest replacement of nested
 std::min or std::max with initializer lists

---
 .../clang-tidy/modernize/CMakeLists.txt       |   1 +
 .../MinMaxUseInitializerListCheck.cpp         | 138 ++++++++++++++++++
 .../modernize/MinMaxUseInitializerListCheck.h |  58 ++++++++
 .../modernize/ModernizeTidyModule.cpp         |   3 +
 clang-tools-extra/docs/ReleaseNotes.rst       |   6 +
 .../docs/clang-tidy/checks/list.rst           |   1 +
 6 files changed, 207 insertions(+)
 create mode 100644 clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
 create mode 100644 clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h

diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
index 6852db6c2ee311..8005d6e91c060c 100644
--- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
@@ -16,6 +16,7 @@ add_clang_library(clangTidyModernizeModule
   MakeSharedCheck.cpp
   MakeSmartPtrCheck.cpp
   MakeUniqueCheck.cpp
+  MinMaxUseInitializerListCheck.cpp
   ModernizeTidyModule.cpp
   PassByValueCheck.cpp
   RawStringLiteralCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
new file mode 100644
index 00000000000000..b7dc3ff436f6e3
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
@@ -0,0 +1,138 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy -------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      Inserter(Options.getLocalOrGlobal("IncludeStyle",
+                                        utils::IncludeSorter::IS_LLVM),
+               areDiagsSelfContained()) {}
+
+void MinMaxUseInitializerListCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+}
+
+void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      callExpr(
+          callee(functionDecl(hasName("::std::max"))),
+          hasAnyArgument(callExpr(callee(functionDecl(hasName("::std::max"))))),
+          unless(
+              hasParent(callExpr(callee(functionDecl(hasName("::std::max")))))))
+          .bind("maxCall"),
+      this);
+
+  Finder->addMatcher(
+      callExpr(
+          callee(functionDecl(hasName("::std::min"))),
+          hasAnyArgument(callExpr(callee(functionDecl(hasName("::std::min"))))),
+          unless(
+              hasParent(callExpr(callee(functionDecl(hasName("::std::min")))))))
+          .bind("minCall"),
+      this);
+}
+
+void MinMaxUseInitializerListCheck::registerPPCallbacks(
+    const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+  Inserter.registerPreprocessor(PP);
+}
+
+void MinMaxUseInitializerListCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *MaxCall = Result.Nodes.getNodeAs<CallExpr>("maxCall");
+  const auto *MinCall = Result.Nodes.getNodeAs<CallExpr>("minCall");
+
+  const CallExpr *TopCall = MaxCall ? MaxCall : MinCall;
+  if (!TopCall) {
+    return;
+  }
+  const QualType ResultType =
+      TopCall->getDirectCallee()->getReturnType().getNonReferenceType();
+
+  const Expr *FirstArg = nullptr;
+  const Expr *LastArg = nullptr;
+  std::vector<const Expr *> Args;
+  findArgs(TopCall, &FirstArg, &LastArg, Args);
+
+  if (!FirstArg || !LastArg || Args.size() <= 2) {
+    return;
+  }
+
+  std::string ReplacementText = "{";
+  for (const Expr *Arg : Args) {
+    QualType ArgType = Arg->getType();
+    bool CastNeeded =
+        ArgType.getCanonicalType() != ResultType.getCanonicalType();
+
+    if (CastNeeded)
+      ReplacementText += "static_cast<" + ResultType.getAsString() + ">(";
+
+    ReplacementText += Lexer::getSourceText(
+        CharSourceRange::getTokenRange(Arg->getSourceRange()),
+        *Result.SourceManager, Result.Context->getLangOpts());
+
+    if (CastNeeded)
+      ReplacementText += ")";
+    ReplacementText += ", ";
+  }
+  ReplacementText = ReplacementText.substr(0, ReplacementText.size() - 2) + "}";
+
+  diag(TopCall->getBeginLoc(),
+       "do not use nested std::%0 calls, use %1 instead")
+      << TopCall->getDirectCallee()->getName() << ReplacementText
+      << FixItHint::CreateReplacement(
+             CharSourceRange::getTokenRange(
+                 FirstArg->getBeginLoc(),
+                 Lexer::getLocForEndOfToken(TopCall->getEndLoc(), 0,
+                                            Result.Context->getSourceManager(),
+                                            Result.Context->getLangOpts())
+                     .getLocWithOffset(-2)),
+             ReplacementText)
+      << Inserter.createMainFileIncludeInsertion("<algorithm>");
+}
+
+void MinMaxUseInitializerListCheck::findArgs(const CallExpr *Call,
+                                             const Expr **First,
+                                             const Expr **Last,
+                                             std::vector<const Expr *> &Args) {
+  if (!Call) {
+    return;
+  }
+
+  const FunctionDecl *Callee = Call->getDirectCallee();
+  if (!Callee) {
+    return;
+  }
+
+  for (const Expr *Arg : Call->arguments()) {
+    if (!*First)
+      *First = Arg;
+
+    const CallExpr *InnerCall = dyn_cast<CallExpr>(Arg);
+    if (InnerCall && InnerCall->getDirectCallee() &&
+        InnerCall->getDirectCallee()->getNameAsString() ==
+            Call->getDirectCallee()->getNameAsString()) {
+      findArgs(InnerCall, First, Last, Args);
+    } else
+      Args.push_back(Arg);
+
+    *Last = Arg;
+  }
+}
+
+} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h
new file mode 100644
index 00000000000000..dc111d4ce7800e
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h
@@ -0,0 +1,58 @@
+//===--- MinMaxUseInitializerListCheck.h - clang-tidy -----------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MINMAXUSEINITIALIZERLISTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MINMAXUSEINITIALIZERLISTCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+
+namespace clang::tidy::modernize {
+
+/// Transforms the repeated calls to `std::min` and `std::max` into a single
+/// call using initializer lists.
+///
+/// It identifies cases where `std::min` or `std::max` is used to find the
+/// minimum or maximum value among more than two items through repeated calls.
+/// The check replaces these calls with a single call to `std::min` or
+/// `std::max` that uses an initializer list. This makes the code slightly more
+/// efficient.
+/// \n\n
+/// For example:
+///
+/// \code
+///   int a = std::max(std::max(i, j), k);
+/// \endcode
+///
+/// This code is transformed to:
+///
+/// \code
+///   int a = std::max({i, j, k});
+/// \endcode
+class MinMaxUseInitializerListCheck : public ClangTidyCheck {
+public:
+  MinMaxUseInitializerListCheck(StringRef Name, ClangTidyContext *Context);
+
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus11;
+  }
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+                           Preprocessor *ModuleExpanderPP) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  utils::IncludeInserter Inserter;
+  void findArgs(const CallExpr *call, const Expr **first, const Expr **last,
+                std::vector<const Expr *> &args);
+};
+
+} // namespace clang::tidy::modernize
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_MINMAXUSEINITIALIZERLISTCHECK_H
diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
index e96cf274f58cfe..776558433c5baa 100644
--- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -18,6 +18,7 @@
 #include "MacroToEnumCheck.h"
 #include "MakeSharedCheck.h"
 #include "MakeUniqueCheck.h"
+#include "MinMaxUseInitializerListCheck.h"
 #include "PassByValueCheck.h"
 #include "RawStringLiteralCheck.h"
 #include "RedundantVoidArgCheck.h"
@@ -68,6 +69,8 @@ class ModernizeModule : public ClangTidyModule {
     CheckFactories.registerCheck<MacroToEnumCheck>("modernize-macro-to-enum");
     CheckFactories.registerCheck<MakeSharedCheck>("modernize-make-shared");
     CheckFactories.registerCheck<MakeUniqueCheck>("modernize-make-unique");
+    CheckFactories.registerCheck<MinMaxUseInitializerListCheck>(
+        "modernize-min-max-use-initializer-list");
     CheckFactories.registerCheck<PassByValueCheck>("modernize-pass-by-value");
     CheckFactories.registerCheck<UseDesignatedInitializersCheck>(
         "modernize-use-designated-initializers");
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index a604e9276668ae..e743438f589987 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -117,6 +117,12 @@ New checks
   to reading out-of-bounds data due to inadequate or incorrect string null
   termination.
 
+- New :doc:`modernize-min-max-use-initializer-list
+  <clang-tidy/checks/modernize/min-max-use-initializer-list>` check.
+
+  Replaces chained ``std::min`` and ``std::max`` calls that can be written as
+  initializer lists.
+
 - New :doc:`modernize-use-designated-initializers
   <clang-tidy/checks/modernize/use-designated-initializers>` check.
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 79e81dd174e4f3..4809d704a94646 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -275,6 +275,7 @@ Clang-Tidy Checks
    :doc:`modernize-macro-to-enum <modernize/macro-to-enum>`, "Yes"
    :doc:`modernize-make-shared <modernize/make-shared>`, "Yes"
    :doc:`modernize-make-unique <modernize/make-unique>`, "Yes"
+   :doc:`modernize-min-max-use-initializer-list <modernize/min-max-use-initializer-list>`, "Yes"
    :doc:`modernize-pass-by-value <modernize/pass-by-value>`, "Yes"
    :doc:`modernize-raw-string-literal <modernize/raw-string-literal>`, "Yes"
    :doc:`modernize-redundant-void-arg <modernize/redundant-void-arg>`, "Yes"

>From 219096fa70b9a630b784d964c0d4bda642853b1f Mon Sep 17 00:00:00 2001
From: sopy <contact at sopy.one>
Date: Wed, 20 Mar 2024 12:11:34 +0200
Subject: [PATCH 2/7] Summary: Refactor min-max-use-initializer-list.cpp and
 add tests

Details:
- Refactored min-max-use-initializer-list.cpp to handle cases where std::{min,max} is given a compare function argument
- Shortened documentation in MinMaxUseInitializerListCheck.h
- Added tests for min-modernize-min-max-use-initializer-list
- Added documentation for min-modernize-min-max-use-initializer-list
- Updated ReleaseNotes.rst based on mantainer suggestions
---
 .../MinMaxUseInitializerListCheck.cpp         | 176 ++++++++++++------
 .../modernize/MinMaxUseInitializerListCheck.h |  35 ++--
 clang-tools-extra/docs/ReleaseNotes.rst       |   4 +-
 .../min-max-use-initializer-list.rst          |  26 +++
 .../min-max-use-initializer-list.cpp          | 105 +++++++++++
 5 files changed, 270 insertions(+), 76 deletions(-)
 create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/min-max-use-initializer-list.rst
 create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/min-max-use-initializer-list.cpp

diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
index b7dc3ff436f6e3..ddeac5b1b3ac77 100644
--- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
@@ -31,19 +31,25 @@ void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       callExpr(
           callee(functionDecl(hasName("::std::max"))),
-          hasAnyArgument(callExpr(callee(functionDecl(hasName("::std::max"))))),
+          anyOf(hasArgument(
+                    0, callExpr(callee(functionDecl(hasName("::std::max"))))),
+                hasArgument(
+                    1, callExpr(callee(functionDecl(hasName("::std::max")))))),
           unless(
               hasParent(callExpr(callee(functionDecl(hasName("::std::max")))))))
-          .bind("maxCall"),
+          .bind("topCall"),
       this);
 
   Finder->addMatcher(
       callExpr(
           callee(functionDecl(hasName("::std::min"))),
-          hasAnyArgument(callExpr(callee(functionDecl(hasName("::std::min"))))),
+          anyOf(hasArgument(
+                    0, callExpr(callee(functionDecl(hasName("::std::min"))))),
+                hasArgument(
+                    1, callExpr(callee(functionDecl(hasName("::std::min")))))),
           unless(
               hasParent(callExpr(callee(functionDecl(hasName("::std::min")))))))
-          .bind("minCall"),
+          .bind("topCall"),
       this);
 }
 
@@ -53,29 +59,112 @@ void MinMaxUseInitializerListCheck::registerPPCallbacks(
 }
 
 void MinMaxUseInitializerListCheck::check(
-    const MatchFinder::MatchResult &Result) {
-  const auto *MaxCall = Result.Nodes.getNodeAs<CallExpr>("maxCall");
-  const auto *MinCall = Result.Nodes.getNodeAs<CallExpr>("minCall");
+    const MatchFinder::MatchResult &Match) {
+  const CallExpr *TopCall = Match.Nodes.getNodeAs<CallExpr>("topCall");
+  MinMaxUseInitializerListCheck::FindArgsResult Result =
+      findArgs(Match, TopCall);
 
-  const CallExpr *TopCall = MaxCall ? MaxCall : MinCall;
-  if (!TopCall) {
+  if (!Result.First || !Result.Last || Result.Args.size() <= 2) {
     return;
   }
-  const QualType ResultType =
-      TopCall->getDirectCallee()->getReturnType().getNonReferenceType();
 
-  const Expr *FirstArg = nullptr;
-  const Expr *LastArg = nullptr;
-  std::vector<const Expr *> Args;
-  findArgs(TopCall, &FirstArg, &LastArg, Args);
+  std::string ReplacementText = generateReplacement(Match, TopCall, Result);
 
-  if (!FirstArg || !LastArg || Args.size() <= 2) {
-    return;
+  diag(TopCall->getBeginLoc(),
+       "do not use nested std::%0 calls, use %1 instead")
+      << TopCall->getDirectCallee()->getName() << ReplacementText
+      << FixItHint::CreateReplacement(
+             CharSourceRange::getTokenRange(TopCall->getBeginLoc(),
+                                            TopCall->getEndLoc()),
+             ReplacementText)
+      << Inserter.createMainFileIncludeInsertion("<algorithm>");
+}
+
+MinMaxUseInitializerListCheck::FindArgsResult
+MinMaxUseInitializerListCheck::findArgs(const MatchFinder::MatchResult &Match,
+                                        const CallExpr *Call) {
+  FindArgsResult Result;
+  Result.First = nullptr;
+  Result.Last = nullptr;
+  Result.Compare = nullptr;
+
+  if (Call->getNumArgs() > 2) {
+    auto argIterator = Call->arguments().begin();
+    std::advance(argIterator, 2);
+    Result.Compare = *argIterator;
   }
 
-  std::string ReplacementText = "{";
-  for (const Expr *Arg : Args) {
+  for (const Expr *Arg : Call->arguments()) {
+    if (!Result.First)
+      Result.First = Arg;
+
+    const CallExpr *InnerCall = dyn_cast<CallExpr>(Arg);
+    if (InnerCall && InnerCall->getDirectCallee() &&
+        InnerCall->getDirectCallee()->getNameAsString() ==
+            Call->getDirectCallee()->getNameAsString()) {
+      FindArgsResult InnerResult = findArgs(Match, InnerCall);
+
+      bool processInnerResult = false;
+
+      if (!Result.Compare && !InnerResult.Compare)
+        processInnerResult = true;
+      else if (Result.Compare && InnerResult.Compare &&
+               Lexer::getSourceText(CharSourceRange::getTokenRange(
+                                        Result.Compare->getSourceRange()),
+                                    *Match.SourceManager,
+                                    Match.Context->getLangOpts()) ==
+                   Lexer::getSourceText(
+                       CharSourceRange::getTokenRange(
+                           InnerResult.Compare->getSourceRange()),
+                       *Match.SourceManager, Match.Context->getLangOpts()))
+        processInnerResult = true;
+
+      if (processInnerResult) {
+        Result.Args.insert(Result.Args.end(), InnerResult.Args.begin(),
+                           InnerResult.Args.end());
+        continue;
+      }
+    }
+
+    if (Arg == Result.Compare)
+      continue;
+
+    Result.Args.push_back(Arg);
+    Result.Last = Arg;
+  }
+
+  return Result;
+}
+
+std::string MinMaxUseInitializerListCheck::generateReplacement(
+    const MatchFinder::MatchResult &Match, const CallExpr *TopCall,
+    const FindArgsResult Result) {
+  std::string ReplacementText =
+      Lexer::getSourceText(
+          CharSourceRange::getTokenRange(
+              TopCall->getBeginLoc(),
+              Result.First->getBeginLoc().getLocWithOffset(-1)),
+          *Match.SourceManager, Match.Context->getLangOpts())
+          .str() +
+      "{";
+  const QualType ResultType =
+      TopCall->getDirectCallee()->getReturnType().getNonReferenceType();
+
+  for (const Expr *Arg : Result.Args) {
     QualType ArgType = Arg->getType();
+
+    // check if expression is std::min or std::max
+    if (const auto *InnerCall = dyn_cast<CallExpr>(Arg)) {
+      if (InnerCall->getDirectCallee() &&
+          InnerCall->getDirectCallee()->getNameAsString() !=
+              TopCall->getDirectCallee()->getNameAsString()) {
+        FindArgsResult innerResult = findArgs(Match, InnerCall);
+        ReplacementText += generateReplacement(Match, InnerCall, innerResult) +=
+            "})";
+        continue;
+      }
+    }
+
     bool CastNeeded =
         ArgType.getCanonicalType() != ResultType.getCanonicalType();
 
@@ -84,55 +173,22 @@ void MinMaxUseInitializerListCheck::check(
 
     ReplacementText += Lexer::getSourceText(
         CharSourceRange::getTokenRange(Arg->getSourceRange()),
-        *Result.SourceManager, Result.Context->getLangOpts());
+        *Match.SourceManager, Match.Context->getLangOpts());
 
     if (CastNeeded)
       ReplacementText += ")";
     ReplacementText += ", ";
   }
   ReplacementText = ReplacementText.substr(0, ReplacementText.size() - 2) + "}";
-
-  diag(TopCall->getBeginLoc(),
-       "do not use nested std::%0 calls, use %1 instead")
-      << TopCall->getDirectCallee()->getName() << ReplacementText
-      << FixItHint::CreateReplacement(
-             CharSourceRange::getTokenRange(
-                 FirstArg->getBeginLoc(),
-                 Lexer::getLocForEndOfToken(TopCall->getEndLoc(), 0,
-                                            Result.Context->getSourceManager(),
-                                            Result.Context->getLangOpts())
-                     .getLocWithOffset(-2)),
-             ReplacementText)
-      << Inserter.createMainFileIncludeInsertion("<algorithm>");
-}
-
-void MinMaxUseInitializerListCheck::findArgs(const CallExpr *Call,
-                                             const Expr **First,
-                                             const Expr **Last,
-                                             std::vector<const Expr *> &Args) {
-  if (!Call) {
-    return;
-  }
-
-  const FunctionDecl *Callee = Call->getDirectCallee();
-  if (!Callee) {
-    return;
+  if (Result.Compare) {
+    ReplacementText += ", ";
+    ReplacementText += Lexer::getSourceText(
+        CharSourceRange::getTokenRange(Result.Compare->getSourceRange()),
+        *Match.SourceManager, Match.Context->getLangOpts());
   }
+  ReplacementText += ")";
 
-  for (const Expr *Arg : Call->arguments()) {
-    if (!*First)
-      *First = Arg;
-
-    const CallExpr *InnerCall = dyn_cast<CallExpr>(Arg);
-    if (InnerCall && InnerCall->getDirectCallee() &&
-        InnerCall->getDirectCallee()->getNameAsString() ==
-            Call->getDirectCallee()->getNameAsString()) {
-      findArgs(InnerCall, First, Last, Args);
-    } else
-      Args.push_back(Arg);
-
-    *Last = Arg;
-  }
+  return ReplacementText;
 }
 
 } // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h
index dc111d4ce7800e..43d95004511430 100644
--- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.h
@@ -14,15 +14,9 @@
 
 namespace clang::tidy::modernize {
 
-/// Transforms the repeated calls to `std::min` and `std::max` into a single
-/// call using initializer lists.
+/// Replaces chained ``std::min`` and ``std::max`` calls with a initializer list
+/// where applicable.
 ///
-/// It identifies cases where `std::min` or `std::max` is used to find the
-/// minimum or maximum value among more than two items through repeated calls.
-/// The check replaces these calls with a single call to `std::min` or
-/// `std::max` that uses an initializer list. This makes the code slightly more
-/// efficient.
-/// \n\n
 /// For example:
 ///
 /// \code
@@ -38,19 +32,32 @@ class MinMaxUseInitializerListCheck : public ClangTidyCheck {
 public:
   MinMaxUseInitializerListCheck(StringRef Name, ClangTidyContext *Context);
 
-  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
-    return LangOpts.CPlusPlus11;
-  }
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
                            Preprocessor *ModuleExpanderPP) override;
-  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Match) override;
+
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus11;
+  }
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
 
 private:
+  struct FindArgsResult {
+    const Expr *First;
+    const Expr *Last;
+    const Expr *Compare;
+    std::vector<const Expr *> Args;
+  };
   utils::IncludeInserter Inserter;
-  void findArgs(const CallExpr *call, const Expr **first, const Expr **last,
-                std::vector<const Expr *> &args);
+  FindArgsResult findArgs(const ast_matchers::MatchFinder::MatchResult &Match,
+                          const CallExpr *Call);
+  std::string
+  generateReplacement(const ast_matchers::MatchFinder::MatchResult &Match,
+                      const CallExpr *TopCall, const FindArgsResult Result);
 };
 
 } // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index e743438f589987..13d14fa9ccf0aa 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -120,8 +120,8 @@ New checks
 - New :doc:`modernize-min-max-use-initializer-list
   <clang-tidy/checks/modernize/min-max-use-initializer-list>` check.
 
-  Replaces chained ``std::min`` and ``std::max`` calls that can be written as
-  initializer lists.
+  Replaces chained ``std::min`` and ``std::max`` calls with a initializer list
+  where applicable.
 
 - New :doc:`modernize-use-designated-initializers
   <clang-tidy/checks/modernize/use-designated-initializers>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/min-max-use-initializer-list.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/min-max-use-initializer-list.rst
new file mode 100644
index 00000000000000..0748e180897948
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/min-max-use-initializer-list.rst
@@ -0,0 +1,26 @@
+.. title:: clang-tidy - modernize-min-max-use-initializer-list
+
+modernize-min-max-use-initializer-list
+======================================
+
+Replaces chained ``std::min`` and ``std::max`` calls with a initializer list where applicable.
+
+For instance, consider the following code:
+
+.. code-block:: cpp
+
+   int a = std::max(std::max(i, j), k);
+
+`modernize-min-max-use-initializer-list` check will transform the above code to:
+
+.. code-block:: cpp
+
+   int a = std::max({i, j, k});
+
+Options
+=======
+
+.. option:: IncludeStyle
+
+   A string specifying which include-style is used, `llvm` or `google`. Default
+   is `llvm`.
\ No newline at end of file
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/min-max-use-initializer-list.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/min-max-use-initializer-list.cpp
new file mode 100644
index 00000000000000..3e15774f7b3f14
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/min-max-use-initializer-list.cpp
@@ -0,0 +1,105 @@
+// RUN: %check_clang_tidy %s modernize-min-max-use-initializer-list %t
+
+// CHECK-FIXES: #include <algorithm>
+namespace std {
+template< class T >
+const T& max( const T& a, const T& b ) {
+  return (a < b) ? b : a;
+};
+
+template< class T, class Compare >
+const T& max( const T& a, const T& b, Compare comp ) {
+  return (comp(a, b)) ? b : a;
+};
+
+template< class T >
+const T& min( const T& a, const T& b ) {
+  return (b < a) ? b : a;
+};
+
+template< class T, class Compare >
+const T& min( const T& a, const T& b, Compare comp ) {
+  return (comp(b, a)) ? b : a;
+};
+}
+
+using namespace std;
+
+namespace {
+bool fless_than(int a, int b) {
+return a < b;
+}
+
+bool fgreater_than(int a, int b) {
+return a > b;
+}
+auto less_than = [](int a, int b) { return a < b; };
+auto greater_than = [](int a, int b) { return a > b; };
+
+int max1 = std::max(1, std::max(2, 3));
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::max calls, use std::max({1, 2, 3}) instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: int max1 = std::max({1, 2, 3});
+
+int min1 = std::min(1, std::min(2, 3));
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::min calls, use std::min({1, 2, 3}) instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: int min1 = std::min({1, 2, 3});
+
+int max2 = std::max(1, std::max(2, std::max(3, 4)));
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::max calls, use std::max({1, 2, 3, 4}) instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: int max2 = std::max({1, 2, 3, 4});
+
+int min2 = std::min(1, std::min(2, std::min(3, 4)));
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::min calls, use std::min({1, 2, 3, 4}) instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: int min2 = std::min({1, 2, 3, 4});
+
+int max3 = std::max(std::max(4, 5), std::min(2, std::min(3, 1)));
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::max calls, use std::max({4, 5, std::min({2, 3, 1})}) instead [modernize-min-max-use-initializer-list]
+// CHECK-MESSAGES: :[[@LINE-2]]:37: warning: do not use nested std::min calls, use std::min({2, 3, 1}) instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: int max3 = std::max({4, 5, std::min({2, 3, 1})});
+
+int min3 = std::min(std::min(4, 5), std::max(2, std::max(3, 1)));
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::min calls, use std::min({4, 5, std::max({2, 3, 1})}) instead [modernize-min-max-use-initializer-list]
+// CHECK-MESSAGES: :[[@LINE-2]]:37: warning: do not use nested std::max calls, use std::max({2, 3, 1}) instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: int min3 = std::min({4, 5, std::max({2, 3, 1})});
+
+int max4 = std::max(1,std::max(2,3, greater_than), less_than);
+// CHECK-FIXES: int max4 = std::max(1,std::max(2,3, greater_than), less_than);
+
+int min4 = std::min(1,std::min(2,3, greater_than), less_than);
+// CHECK-FIXES: int min4 = std::min(1,std::min(2,3, greater_than), less_than);
+
+int max5 = std::max(1,std::max(2,3), less_than);
+// CHECK-FIXES: int max5 = std::max(1,std::max(2,3), less_than);
+
+int min5 = std::min(1,std::min(2,3), less_than);
+// CHECK-FIXES: int min5 = std::min(1,std::min(2,3), less_than);
+
+int max6 = std::max(1,std::max(2,3, greater_than), greater_than);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::max calls, use std::max({1, 2, 3}, greater_than) instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: int max6 = std::max({1, 2, 3}, greater_than);
+
+int min6 = std::min(1,std::min(2,3, greater_than), greater_than);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::min calls, use std::min({1, 2, 3}, greater_than) instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: int min6 = std::min({1, 2, 3}, greater_than);
+
+int max7 = std::max(1,std::max(2,3, fless_than), fgreater_than);
+// CHECK-FIXES: int max7 = std::max(1,std::max(2,3, fless_than), fgreater_than);
+
+int min7 = std::min(1,std::min(2,3, fless_than), fgreater_than);
+// CHECK-FIXES: int min7 = std::min(1,std::min(2,3, fless_than), fgreater_than);
+
+int max8 = std::max(1,std::max(2,3, fless_than), less_than);
+// CHECK-FIXES: int max8 = std::max(1,std::max(2,3, fless_than), less_than)
+
+int min8 = std::min(1,std::min(2,3, fless_than), less_than);
+// CHECK-FIXES: int min8 = std::min(1,std::min(2,3, fless_than), less_than);
+
+int max9 = std::max(1,std::max(2,3, fless_than), fless_than);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::max calls, use std::max({1, 2, 3}, fless_than) instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: int max9 = std::max({1, 2, 3}, fless_than);
+
+int min9 = std::min(1,std::min(2,3, fless_than), fless_than);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: do not use nested std::min calls, use std::min({1, 2, 3}, fless_than) instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: int min9 = std::min({1, 2, 3}, fless_than);
+
+}
\ No newline at end of file

>From bd9273039fec0514e568801ce499484b1ce6519b Mon Sep 17 00:00:00 2001
From: sopy <contact at sopy.one>
Date: Wed, 20 Mar 2024 13:12:55 +0200
Subject: [PATCH 3/7] Make sure InnerCall is std::min or std::max when
 generating replacement

---
 .../MinMaxUseInitializerListCheck.cpp         | 20 +++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
index ddeac5b1b3ac77..2fc6b517ad79ce 100644
--- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
@@ -153,15 +153,19 @@ std::string MinMaxUseInitializerListCheck::generateReplacement(
   for (const Expr *Arg : Result.Args) {
     QualType ArgType = Arg->getType();
 
-    // check if expression is std::min or std::max
     if (const auto *InnerCall = dyn_cast<CallExpr>(Arg)) {
-      if (InnerCall->getDirectCallee() &&
-          InnerCall->getDirectCallee()->getNameAsString() !=
-              TopCall->getDirectCallee()->getNameAsString()) {
-        FindArgsResult innerResult = findArgs(Match, InnerCall);
-        ReplacementText += generateReplacement(Match, InnerCall, innerResult) +=
-            "})";
-        continue;
+      if (InnerCall->getDirectCallee()) {
+        std::string InnerCallNameStr =
+            InnerCall->getDirectCallee()->getNameAsString();
+
+        if (InnerCallNameStr != TopCall->getDirectCallee()->getNameAsString() &&
+            (InnerCallNameStr == "std::min" ||
+             InnerCallNameStr == "std::max")) {
+          FindArgsResult innerResult = findArgs(Match, InnerCall);
+          ReplacementText +=
+              generateReplacement(Match, InnerCall, innerResult);
+          continue;
+        }
       }
     }
 

>From 18517aca52aa32ad2e2e7a838a9558c4a6fcbf0f Mon Sep 17 00:00:00 2001
From: sopy <contact at sopy.one>
Date: Wed, 20 Mar 2024 13:27:54 +0200
Subject: [PATCH 4/7] Clang format fix

---
 .../clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp     | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
index 2fc6b517ad79ce..6e86c39843612a 100644
--- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
@@ -162,8 +162,7 @@ std::string MinMaxUseInitializerListCheck::generateReplacement(
             (InnerCallNameStr == "std::min" ||
              InnerCallNameStr == "std::max")) {
           FindArgsResult innerResult = findArgs(Match, InnerCall);
-          ReplacementText +=
-              generateReplacement(Match, InnerCall, innerResult);
+          ReplacementText += generateReplacement(Match, InnerCall, innerResult);
           continue;
         }
       }

>From 80d37c4fbf916c879903545b7cd72cfbd99d1371 Mon Sep 17 00:00:00 2001
From: sopy <contact at sopy.one>
Date: Wed, 20 Mar 2024 13:44:56 +0200
Subject: [PATCH 5/7] Clang format fix

---
 .../clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp  | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
index 6e86c39843612a..4b2b8b4e62c2ac 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()) {

>From 2eda64973fcc97f7f1fb1c54306b0e47d73e9975 Mon Sep 17 00:00:00 2001
From: sopy <contact at sopy.one>
Date: Wed, 20 Mar 2024 14:19:54 +0200
Subject: [PATCH 6/7] Fix ReplacementText generation

---
 .../clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp  | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
index 4b2b8b4e62c2ac..a4ec85fccf1597 100644
--- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
@@ -156,13 +156,13 @@ std::string MinMaxUseInitializerListCheck::generateReplacement(
     if (const auto *InnerCall = dyn_cast<CallExpr>(Arg)) {
       if (InnerCall->getDirectCallee()) {
         std::string InnerCallNameStr =
-            InnerCall->getDirectCallee()->getNameAsString();
+            InnerCall->getDirectCallee()->getQualifiedNameAsString();
 
-        if (InnerCallNameStr != TopCall->getDirectCallee()->getNameAsString() &&
+        if (InnerCallNameStr != TopCall->getDirectCallee()->getQualifiedNameAsString() &&
             (InnerCallNameStr == "std::min" ||
              InnerCallNameStr == "std::max")) {
           FindArgsResult innerResult = findArgs(Match, InnerCall);
-          ReplacementText += generateReplacement(Match, InnerCall, innerResult);
+          ReplacementText += generateReplacement(Match, InnerCall, innerResult) + ", ";
           continue;
         }
       }

>From d057241df701a5e90b32977afe437ae4b1f282aa Mon Sep 17 00:00:00 2001
From: sopy <contact at sopy.one>
Date: Wed, 20 Mar 2024 14:26:10 +0200
Subject: [PATCH 7/7] Clang format fix

---
 .../clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp  | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
index a4ec85fccf1597..93abbe023d8748 100644
--- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
@@ -158,11 +158,13 @@ std::string MinMaxUseInitializerListCheck::generateReplacement(
         std::string InnerCallNameStr =
             InnerCall->getDirectCallee()->getQualifiedNameAsString();
 
-        if (InnerCallNameStr != TopCall->getDirectCallee()->getQualifiedNameAsString() &&
+        if (InnerCallNameStr !=
+                TopCall->getDirectCallee()->getQualifiedNameAsString() &&
             (InnerCallNameStr == "std::min" ||
              InnerCallNameStr == "std::max")) {
           FindArgsResult innerResult = findArgs(Match, InnerCall);
-          ReplacementText += generateReplacement(Match, InnerCall, innerResult) + ", ";
+          ReplacementText +=
+              generateReplacement(Match, InnerCall, innerResult) + ", ";
           continue;
         }
       }



More information about the cfe-commits mailing list