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

[Piotr Zegar via cfe-commits]

================
@@ -0,0 +1,274 @@
+//===--- 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 "../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;
+
+namespace {
+
+struct FindArgsResult {
+  const Expr *First;
+  const Expr *Last;
+  const Expr *Compare;
+  SmallVector<const clang::Expr *, 2> Args;
+};
+
+} // anonymous namespace
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+static FindArgsResult findArgs(const CallExpr *Call) {
+  FindArgsResult Result;
+  Result.First = nullptr;
+  Result.Last = nullptr;
+  Result.Compare = nullptr;
+
+  //   check if the function has initializer list argument
+  if (Call->getNumArgs() < 3) {
+    auto ArgIterator = Call->arguments().begin();
+
+    const auto *InitListExpr =
+        dyn_cast<CXXStdInitializerListExpr>(*ArgIterator);
+    const auto *InitList =
+        InitListExpr != nullptr
+            ? dyn_cast<clang::InitListExpr>(
+                  InitListExpr->getSubExpr()->IgnoreImplicit())
+            : nullptr;
+
+    if (InitList) {
+      Result.Args.append(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;
+    }
+  } else {
+    // if it has 3 arguments then the last will be the comparison
+    Result.Compare = *(std::next(Call->arguments().begin(), 2));
+  }
+
+  if (Result.Compare)
+    Result.Args = SmallVector<const Expr *>(llvm::drop_end(Call->arguments()));
+  else
+    Result.Args = SmallVector<const Expr *>(Call->arguments());
+
+  Result.First = Result.Args.front();
+  Result.Last = Result.Args.back();
+
+  return Result;
+}
+
+static SmallVector<FixItHint>
+generateReplacements(const MatchFinder::MatchResult &Match,
+                     const CallExpr *TopCall, const FindArgsResult &Result,
+                     const bool IgnoreNonTrivialTypes,
+                     const unsigned long IgnoreTrivialTypesOfSizeAbove) {
+  SmallVector<FixItHint> FixItHints;
+
+  const QualType ResultType = TopCall->getDirectCallee()
+                                  ->getReturnType()
+                                  .getNonReferenceType()
+                                  .getUnqualifiedType()
+                                  .getCanonicalType();
+
+  // check if the type is trivial
+  const bool isResultTypeTrivial = Match.Context->getBaseElementType(ResultType)
+                                       .isTrivialType(*Match.Context);
+  const SourceManager &SourceMngr = *Match.SourceManager;
+  const LangOptions &LanguageOpts = Match.Context->getLangOpts();
+
+  if ((!isResultTypeTrivial && IgnoreNonTrivialTypes))
+    return FixItHints;
+
+  if (isResultTypeTrivial &&
+      // size in bits divided by 8 to get bytes
+      Match.Context->getTypeSize(ResultType) / 8 >
+          IgnoreTrivialTypesOfSizeAbove)
+    return FixItHints;
+
+  for (const Expr *Arg : Result.Args) {
+    const auto *InnerCall = dyn_cast<CallExpr>(Arg->IgnoreParenImpCasts());
+
+    // If the argument is not a nested call
+    if (!InnerCall) {
+      // check if typecast is required
+      const QualType ArgType = Arg->IgnoreParenImpCasts()
+                                   ->getType()
+                                   .getUnqualifiedType()
+                                   .getCanonicalType();
+
+      if (ArgType == ResultType)
+        continue;
+
+      const StringRef ArgText = Lexer::getSourceText(
+          CharSourceRange::getTokenRange(Arg->getSourceRange()), SourceMngr,
+          LanguageOpts);
+
+      Twine Replacement = llvm::Twine("static_cast<")
+                              .concat(ResultType.getAsString(LanguageOpts))
+                              .concat(">(")
+                              .concat(ArgText)
+                              .concat(")");
----------------
PiotrZSL wrote:

It doesn't mean that other checks are correct. Twine saves only pointer to std::string, it doesn't make a copy. Therefore in this case after call to last concat result of getAsString will be destroyed, if you would add call to .str() at the end of this chain it would be ok, but currently this is just a bug, additionally most probably because return type is small, it fit into stack memory.

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