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

2024-05-15 Thread Julian Schmidt via cfe-commits 

https://github.com/5chmidti closed 
https://github.com/llvm/llvm-project/pull/91992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-05-15 Thread Julian Schmidt via cfe-commits 

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/91992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-05-13 Thread via cfe-commits 

https://github.com/sopyb approved this pull request.

Glad someone was around to make a fix 🙂

https://github.com/llvm/llvm-project/pull/91992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-05-13 Thread Piotr Zegar via cfe-commits 

https://github.com/PiotrZSL approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/91992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-05-13 Thread Julian Schmidt via cfe-commits 

5chmidti wrote:

I have a few small changes to `findArgs` that I have not committed, which would 
change the `if`'s to check the exact number of args that are expected, instead 
of `< 3` and `else` (+change from iterators to `getArg(n)`). Should I add them 
as well?

https://github.com/llvm/llvm-project/pull/91992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-05-13 Thread Julian Schmidt via cfe-commits 

5chmidti wrote:

CC @sopyb I was already looking into it ^^

https://github.com/llvm/llvm-project/pull/91992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-05-13 Thread via cfe-commits 

llvmbot wrote:




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

Author: Julian Schmidt (5chmidti)


Changes

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
 

``




https://github.com/llvm/llvm-project/pull/91992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-05-13 Thread Julian Schmidt via cfe-commits 

https://github.com/5chmidti edited 
https://github.com/llvm/llvm-project/pull/91992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2024-05-13 Thread Julian Schmidt via cfe-commits 

https://github.com/5chmidti created 
https://github.com/llvm/llvm-project/pull/91992

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.


>From 4819130e4b5aa02edc452eaccea9b451dfff9de1 Mon Sep 17 00:00:00 2001
From: Julian Schmidt 
Date: Mon, 13 May 2024 18:35:51 +0200
Subject: [PATCH] [clang-tidy] fix crash due to assumed callee in
 min-max-use-initializer-list

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.
---
 .../MinMaxUseInitializerListCheck.cpp | 10 -
 .../min-max-use-initializer-list.cpp  | 21 +++
 2 files changed, 26 insertions(+), 5 deletions(-)

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
 

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits