[clang-tools-extra] [clang-tidy] fix crash due to assumed callee in min-max-use-initializer-list (PR #91992)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tools-extra

Author: Julian Schmidt (5chmidti)

<details>
<summary>Changes</summary>

Previously, the call to `findArgs` for a `CallExpr` inside of a `min` or
`max` call would call `findArgs` before checking if the argument is a
call to `min` or `max`, which is what `findArgs` is expecting.
The fix moves the name checking before the call to `findArgs`, such that
only a `min` or `max` function call is used as an argument.

Fixes #<!-- -->91982 

---
Full diff: https://github.com/llvm/llvm-project/pull/91992.diff


2 Files Affected:

- (modified) clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp (+5-5) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/min-max-use-initializer-list.cpp (+21) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
index 45f7700463d57..418699ffbc4d1 100644
--- a/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MinMaxUseInitializerListCheck.cpp
@@ -129,17 +129,17 @@ generateReplacements(const MatchFinder::MatchResult &Match,
       continue;
     }
 
+    // if the nested call is not the same as the top call
+    if (InnerCall->getDirectCallee()->getQualifiedNameAsString() !=
+        TopCall->getDirectCallee()->getQualifiedNameAsString())
+      continue;
+
     const FindArgsResult InnerResult = findArgs(InnerCall);
 
     // if the nested call doesn't have arguments skip it
     if (!InnerResult.First || !InnerResult.Last)
       continue;
 
-    // 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,
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 51ab9bda975f1..1f2dad2b933ca 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
@@ -300,6 +300,27 @@ B maxTT2 = std::max(B(), std::max(B(), B()));
 B maxTT3 = std::max(B(), std::max(B(), B()), [](const B &lhs, const B &rhs) { return lhs.a[0] < rhs.a[0]; });
 // CHECK-FIXES: B maxTT3 = std::max(B(), std::max(B(), B()), [](const B &lhs, const B &rhs) { return lhs.a[0] < rhs.a[0]; });
 
+struct GH91982 {
+  int fun0Args();
+  int fun1Arg(int a);
+  int fun2Args(int a, int b);
+  int fun3Args(int a, int b, int c);
+  int fun4Args(int a, int b, int c, int d);
+
+  int foo() {
+    return std::max(
+        fun0Args(),
+        std::max(fun1Arg(0),
+                 std::max(fun2Args(0, 1),
+                          std::max(fun3Args(0, 1, 2), fun4Args(0, 1, 2, 3)))));
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: do not use nested 'std::max' calls, use an initializer list instead [modernize-min-max-use-initializer-list]
+// CHECK-FIXES: return std::max(
+// CHECK-FIXES-NEXT: {fun0Args(),
+// CHECK-FIXES-NEXT: fun1Arg(0),
+// CHECK-FIXES-NEXT: fun2Args(0, 1),
+// CHECK-FIXES-NEXT: fun3Args(0, 1, 2), fun4Args(0, 1, 2, 3)});
+  }
+};
 
 } // namespace
 

``````````

</details>


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