[clang-tools-extra] [clang-tidy] Improved --verify-config when using literal style in config file (PR #85591)

2024-03-26 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/85591

From f015496c511b4c2898b9f4d65ebfe39a34c5119c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 17 Mar 2024 20:50:17 -0400
Subject: [PATCH 1/5] [clang-tidy] Improved --verify-config when using literal
 style in config file

Specifying checks using the literal style (|) in the clang-tidy config file is 
currently
supported but was not implemented for the --verify-config options.
This means that clang-tidy would work properly but, using the --verify-config 
option
would raise an error due to some checks not being parsed properly.

Fixes #53737
---
 clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp  |  9 ++---
 clang-tools-extra/docs/ReleaseNotes.rst  |  3 +++
 .../test/clang-tidy/infrastructure/verify-config.cpp | 12 
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp 
b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index 9f3d6b6db6cbca..b68a6215b383a6 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -454,11 +454,14 @@ static constexpr StringLiteral VerifyConfigWarningEnd = " 
[-verify-config]\n";
 
 static bool verifyChecks(const StringSet<> , StringRef CheckGlob,
  StringRef Source) {
-  llvm::StringRef Cur, Rest;
+  llvm::StringRef Cur = CheckGlob;
+  llvm::StringRef Rest = CheckGlob;
   bool AnyInvalid = false;
-  for (std::tie(Cur, Rest) = CheckGlob.split(',');
-   !(Cur.empty() && Rest.empty()); std::tie(Cur, Rest) = Rest.split(',')) {
+  while (!Cur.empty() || !Rest.empty()) {
+Cur = Rest.substr(0, Rest.find_first_of(",\n"));
+Rest = Rest.substr(Cur.size() + 1);
 Cur = Cur.trim();
+
 if (Cur.empty())
   continue;
 Cur.consume_front("-");
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index a604e9276668ae..3887384e713896 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -262,6 +262,9 @@ Miscellaneous
   option is specified. Now ``clang-apply-replacements`` applies formatting 
only with
   the option.
 
+- Fixed ``--verify-check`` option not properly parsing checks when using the 
+  literal operator in the ``.clang-tidy`` config
+
 Improvements to include-fixer
 -
 
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp 
b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
index 421f8641281acb..3659285986482a 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
@@ -18,3 +18,15 @@
 // CHECK-VERIFY: command-line option '-checks': warning: check glob 'bad*glob' 
doesn't match any known check [-verify-config]
 // CHECK-VERIFY: command-line option '-checks': warning: unknown check 
'llvm-includeorder'; did you mean 'llvm-include-order' [-verify-config]
 // CHECK-VERIFY: command-line option '-checks': warning: unknown check 
'my-made-up-check' [-verify-config]
+
+// RUN: echo -e 'Checks: |\n bugprone-argument-comment\n 
bugprone-assert-side-effect,\n bugprone-bool-pointer-implicit-conversion\n 
readability-use-anyof*' > %T/MyClangTidyConfig
+// RUN: clang-tidy -verify-config \
+// RUN: --config-file=%T/MyClangTidyConfig | FileCheck %s 
-check-prefix=CHECK-VERIFY-BLOCK-OK
+// CHECK-VERIFY-BLOCK-OK: No config errors detected.
+
+// RUN: echo -e 'Checks: |\n bugprone-arguments-*\n 
bugprone-assert-side-effects\n bugprone-bool-pointer-implicit-conversion' > 
%T/MyClangTidyConfigBad
+// RUN: not clang-tidy -verify-config \
+// RUN: --config-file=%T/MyClangTidyConfigBad 2>&1 | FileCheck %s 
-check-prefix=CHECK-VERIFY-BLOCK-BAD
+// CHECK-VERIFY-BLOCK-BAD: command-line option '-config': warning: check glob 
'bugprone-arguments-*' doesn't match any known check [-verify-config]
+// CHECK-VERIFY-BLOCK-BAD: command-line option '-config': warning: unknown 
check 'bugprone-assert-side-effects'; did you mean 
'bugprone-assert-side-effect' [-verify-config]
+

From 0e2cb54934829f42069b06f599fa9e724bdf40b3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sat, 23 Mar 2024 13:30:28 -0400
Subject: [PATCH 2/5] fixup! [clang-tidy] Improved --verify-config when using
 literal style in config file

Partially rewrote verifyChecks() to realign the checks parsing logic.

Instead of manually parsing the string of globs, verifyCHecks() now uses a 
GlobList which is responsible
to parse the string and return a list of regexes. This ensures that any changes 
made to the checks parsing logic
has to be made at only one place rather than two.
---
 clang-tools-extra/clang-tidy/GlobList.cpp | 14 ++--
 clang-tools-extra/clang-tidy/GlobList.h   |  4 ++
 .../clang-tidy/tool/ClangTidyMain.cpp 

[clang-tools-extra] [clang-tidy] Improved --verify-config when using literal style in config file (PR #85591)

2024-03-26 Thread Félix-Antoine Constantin via cfe-commits

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


[clang-tools-extra] [clang-tidy] Improved --verify-config when using literal style in config file (PR #85591)

2024-03-26 Thread Félix-Antoine Constantin via cfe-commits


@@ -454,52 +454,31 @@ static constexpr StringLiteral VerifyConfigWarningEnd = " 
[-verify-config]\n";
 
 static bool verifyChecks(const StringSet<> , StringRef CheckGlob,
  StringRef Source) {
-  llvm::StringRef Cur, Rest;
+  GlobList Globs(CheckGlob);
   bool AnyInvalid = false;
-  for (std::tie(Cur, Rest) = CheckGlob.split(',');
-   !(Cur.empty() && Rest.empty()); std::tie(Cur, Rest) = Rest.split(',')) {
-Cur = Cur.trim();
-if (Cur.empty())
+  for (const auto  : Globs.getItems()) {
+const llvm::Regex  = Item.Regex;
+const llvm::StringRef Text = Item.Text;
+if (Text.starts_with("clang-diagnostic"))
   continue;
-Cur.consume_front("-");
-if (Cur.starts_with("clang-diagnostic"))
-  continue;
-if (Cur.contains('*')) {
-  SmallString<128> RegexText("^");
-  StringRef MetaChars("()^$|*+?.[]\\{}");
-  for (char C : Cur) {
-if (C == '*')
-  RegexText.push_back('.');
-else if (MetaChars.contains(C))
-  RegexText.push_back('\\');
-RegexText.push_back(C);
-  }
-  RegexText.push_back('$');
-  llvm::Regex Glob(RegexText);
-  std::string Error;
-  if (!Glob.isValid(Error)) {
-AnyInvalid = true;
-llvm::WithColor::error(llvm::errs(), Source)
-<< "building check glob '" << Cur << "' " << Error << "'\n";
-continue;
-  }
-  if (llvm::none_of(AllChecks.keys(),
-[](StringRef S) { return Glob.match(S); })) {
-AnyInvalid = true;
+if (llvm::none_of(AllChecks.keys(), [](StringRef S) {
+  llvm::errs() << S << '\n';
+  return Reg.match(S);
+})) {
+  AnyInvalid = true;
+  if (Item.Text.contains('*'))

felix642 wrote:

You are right, I've changed slightly this function's semantics, but it 
shouldn't have any observable behaviour change for the end user.
Before my change, we would only create a regex and try to match it if the 
string contains any '*' character. But, since the GlobList implicitly generates 
a list of Regexes we always try to match the strings through regexp now. This 
won't make a difference since the generated regex will only match the exact 
string if it doesn't contain any '*', but it will be a bit less efficient since 
we are going through pattern matching rather than string comparison.

I've moved the condition with the '*' character at the of the method to 
preserve the error message that is displayed to the user in case we can't find 
any matching check. (Either with "check glob ... doesn't match" if it contains 
a '*' or "unknown check ..." if it doesn't). 

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


[clang-tools-extra] [clang-tidy] Improved --verify-config when using literal style in config file (PR #85591)

2024-03-26 Thread Félix-Antoine Constantin via cfe-commits


@@ -454,52 +454,31 @@ static constexpr StringLiteral VerifyConfigWarningEnd = " 
[-verify-config]\n";
 
 static bool verifyChecks(const StringSet<> , StringRef CheckGlob,
  StringRef Source) {
-  llvm::StringRef Cur, Rest;
+  GlobList Globs(CheckGlob);
   bool AnyInvalid = false;
-  for (std::tie(Cur, Rest) = CheckGlob.split(',');
-   !(Cur.empty() && Rest.empty()); std::tie(Cur, Rest) = Rest.split(',')) {
-Cur = Cur.trim();
-if (Cur.empty())
+  for (const auto  : Globs.getItems()) {
+const llvm::Regex  = Item.Regex;
+const llvm::StringRef Text = Item.Text;
+if (Text.starts_with("clang-diagnostic"))
   continue;
-Cur.consume_front("-");
-if (Cur.starts_with("clang-diagnostic"))
-  continue;
-if (Cur.contains('*')) {
-  SmallString<128> RegexText("^");
-  StringRef MetaChars("()^$|*+?.[]\\{}");
-  for (char C : Cur) {
-if (C == '*')
-  RegexText.push_back('.');
-else if (MetaChars.contains(C))
-  RegexText.push_back('\\');
-RegexText.push_back(C);
-  }
-  RegexText.push_back('$');
-  llvm::Regex Glob(RegexText);
-  std::string Error;
-  if (!Glob.isValid(Error)) {
-AnyInvalid = true;
-llvm::WithColor::error(llvm::errs(), Source)
-<< "building check glob '" << Cur << "' " << Error << "'\n";
-continue;
-  }
-  if (llvm::none_of(AllChecks.keys(),
-[](StringRef S) { return Glob.match(S); })) {
-AnyInvalid = true;
+if (llvm::none_of(AllChecks.keys(), [](StringRef S) {
+  llvm::errs() << S << '\n';

felix642 wrote:

No, I forgot to remove this print before committing, it should be fixed now. 
Thank you 

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


[clang-tools-extra] [clang-tidy] Improved --verify-config when using literal style in config file (PR #85591)

2024-03-26 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/85591

From f015496c511b4c2898b9f4d65ebfe39a34c5119c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 17 Mar 2024 20:50:17 -0400
Subject: [PATCH 1/4] [clang-tidy] Improved --verify-config when using literal
 style in config file

Specifying checks using the literal style (|) in the clang-tidy config file is 
currently
supported but was not implemented for the --verify-config options.
This means that clang-tidy would work properly but, using the --verify-config 
option
would raise an error due to some checks not being parsed properly.

Fixes #53737
---
 clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp  |  9 ++---
 clang-tools-extra/docs/ReleaseNotes.rst  |  3 +++
 .../test/clang-tidy/infrastructure/verify-config.cpp | 12 
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp 
b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index 9f3d6b6db6cbca..b68a6215b383a6 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -454,11 +454,14 @@ static constexpr StringLiteral VerifyConfigWarningEnd = " 
[-verify-config]\n";
 
 static bool verifyChecks(const StringSet<> , StringRef CheckGlob,
  StringRef Source) {
-  llvm::StringRef Cur, Rest;
+  llvm::StringRef Cur = CheckGlob;
+  llvm::StringRef Rest = CheckGlob;
   bool AnyInvalid = false;
-  for (std::tie(Cur, Rest) = CheckGlob.split(',');
-   !(Cur.empty() && Rest.empty()); std::tie(Cur, Rest) = Rest.split(',')) {
+  while (!Cur.empty() || !Rest.empty()) {
+Cur = Rest.substr(0, Rest.find_first_of(",\n"));
+Rest = Rest.substr(Cur.size() + 1);
 Cur = Cur.trim();
+
 if (Cur.empty())
   continue;
 Cur.consume_front("-");
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index a604e9276668ae..3887384e713896 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -262,6 +262,9 @@ Miscellaneous
   option is specified. Now ``clang-apply-replacements`` applies formatting 
only with
   the option.
 
+- Fixed ``--verify-check`` option not properly parsing checks when using the 
+  literal operator in the ``.clang-tidy`` config
+
 Improvements to include-fixer
 -
 
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp 
b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
index 421f8641281acb..3659285986482a 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
@@ -18,3 +18,15 @@
 // CHECK-VERIFY: command-line option '-checks': warning: check glob 'bad*glob' 
doesn't match any known check [-verify-config]
 // CHECK-VERIFY: command-line option '-checks': warning: unknown check 
'llvm-includeorder'; did you mean 'llvm-include-order' [-verify-config]
 // CHECK-VERIFY: command-line option '-checks': warning: unknown check 
'my-made-up-check' [-verify-config]
+
+// RUN: echo -e 'Checks: |\n bugprone-argument-comment\n 
bugprone-assert-side-effect,\n bugprone-bool-pointer-implicit-conversion\n 
readability-use-anyof*' > %T/MyClangTidyConfig
+// RUN: clang-tidy -verify-config \
+// RUN: --config-file=%T/MyClangTidyConfig | FileCheck %s 
-check-prefix=CHECK-VERIFY-BLOCK-OK
+// CHECK-VERIFY-BLOCK-OK: No config errors detected.
+
+// RUN: echo -e 'Checks: |\n bugprone-arguments-*\n 
bugprone-assert-side-effects\n bugprone-bool-pointer-implicit-conversion' > 
%T/MyClangTidyConfigBad
+// RUN: not clang-tidy -verify-config \
+// RUN: --config-file=%T/MyClangTidyConfigBad 2>&1 | FileCheck %s 
-check-prefix=CHECK-VERIFY-BLOCK-BAD
+// CHECK-VERIFY-BLOCK-BAD: command-line option '-config': warning: check glob 
'bugprone-arguments-*' doesn't match any known check [-verify-config]
+// CHECK-VERIFY-BLOCK-BAD: command-line option '-config': warning: unknown 
check 'bugprone-assert-side-effects'; did you mean 
'bugprone-assert-side-effect' [-verify-config]
+

From 0e2cb54934829f42069b06f599fa9e724bdf40b3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sat, 23 Mar 2024 13:30:28 -0400
Subject: [PATCH 2/4] fixup! [clang-tidy] Improved --verify-config when using
 literal style in config file

Partially rewrote verifyChecks() to realign the checks parsing logic.

Instead of manually parsing the string of globs, verifyCHecks() now uses a 
GlobList which is responsible
to parse the string and return a list of regexes. This ensures that any changes 
made to the checks parsing logic
has to be made at only one place rather than two.
---
 clang-tools-extra/clang-tidy/GlobList.cpp | 14 ++--
 clang-tools-extra/clang-tidy/GlobList.h   |  4 ++
 .../clang-tidy/tool/ClangTidyMain.cpp 

[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)

2024-03-23 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/82947

From 72f2b398d6f41372dc84579a83d273398884c869 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 25 Feb 2024 20:20:59 -0500
Subject: [PATCH] [clang-tidy] Improved modernize-use-using by fixing a
 false-negative

The check needs a parent decl to match but if the typedef is in a function,
the parent is a declStmt which is not a decl by itself.
Improved the matcher to match on either a decl or a declstmt and extract
the decl from the stmt in the latter case.

fixes #72179
---
 .../clang-tidy/modernize/UseUsingCheck.cpp| 24 +--
 clang-tools-extra/docs/ReleaseNotes.rst   |  3 ++
 .../checkers/modernize/use-using.cpp  | 43 ++-
 3 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
index bb05f206c717ce..24eefdb082eb32 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -8,6 +8,7 @@
 
 #include "UseUsingCheck.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclGroup.h"
 #include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
@@ -24,6 +25,7 @@ static constexpr llvm::StringLiteral ExternCDeclName = 
"extern-c-decl";
 static constexpr llvm::StringLiteral ParentDeclName = "parent-decl";
 static constexpr llvm::StringLiteral TagDeclName = "tag-decl";
 static constexpr llvm::StringLiteral TypedefName = "typedef";
+static constexpr llvm::StringLiteral DeclStmtName = "decl-stmt";
 
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
@@ -41,7 +43,8 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   unless(isInstantiated()),
   optionally(hasAncestor(
   linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))),
-  hasParent(decl().bind(ParentDeclName)))
+  anyOf(hasParent(decl().bind(ParentDeclName)),
+hasParent(declStmt().bind(DeclStmtName
   .bind(TypedefName),
   this);
 
@@ -51,17 +54,32 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   tagDecl(
   anyOf(allOf(unless(anyOf(isImplicit(),
classTemplateSpecializationDecl())),
-  hasParent(decl().bind(ParentDeclName))),
+  anyOf(hasParent(decl().bind(ParentDeclName)),
+hasParent(declStmt().bind(DeclStmtName,
 // We want the parent of the ClassTemplateDecl, not the parent
 // of the specialization.
 classTemplateSpecializationDecl(hasAncestor(classTemplateDecl(
-hasParent(decl().bind(ParentDeclName)))
+anyOf(hasParent(decl().bind(ParentDeclName)),
+  hasParent(declStmt().bind(DeclStmtName
   .bind(TagDeclName),
   this);
 }
 
 void UseUsingCheck::check(const MatchFinder::MatchResult ) {
   const auto *ParentDecl = Result.Nodes.getNodeAs(ParentDeclName);
+
+  if (!ParentDecl) {
+const auto *ParentDeclStmt = 
Result.Nodes.getNodeAs(DeclStmtName);
+if (ParentDeclStmt) {
+  if (ParentDeclStmt->isSingleDecl())
+ParentDecl = ParentDeclStmt->getSingleDecl();
+  else
+ParentDecl =
+ParentDeclStmt->getDeclGroup().getDeclGroup()
+[ParentDeclStmt->getDeclGroup().getDeclGroup().size() - 1];
+}
+  }
+
   if (!ParentDecl)
 return;
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index a604e9276668ae..14c7064930bbbc 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -235,6 +235,9 @@ Changes in existing checks
   analyzed, se the check now handles the common patterns
   `const auto e = (*vector_ptr)[i]` and `const auto e = vector_ptr->at(i);`.
 
+- Improved :doc:`modernize-use-using `
+  check by adding support for detection of typedefs declared on function level.
+
 - Improved :doc:`readability-implicit-bool-conversion
   ` check to provide
   valid fix suggestions for ``static_cast`` without a preceding space and
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
index 462bc984fd3ad5..925e5f9c1ca54e 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s modernize-use-using %t -- -- -I 
%S/Inputs/use-using/
+// RUN: %check_clang_tidy %s modernize-use-using %t -- -- 
-fno-delayed-template-parsing -I %S/Inputs/use-using/
 
 typedef int Type;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: 

[clang-tools-extra] [clang-tidy] Improved --verify-config when using literal style in config file (PR #85591)

2024-03-23 Thread Félix-Antoine Constantin via cfe-commits

felix642 wrote:

Thank you for the review @SimplyDanny, as per your suggestion, I've rewrote 
part of my fix to unify the parsing logic rather than updating the duplicated 
code. The method now uses the GlobList which already handles correctly the 
regexes generation from the list of checks.

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


[clang-tools-extra] [clang-tidy] Improved --verify-config when using literal style in config file (PR #85591)

2024-03-23 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/85591

From f015496c511b4c2898b9f4d65ebfe39a34c5119c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 17 Mar 2024 20:50:17 -0400
Subject: [PATCH 1/3] [clang-tidy] Improved --verify-config when using literal
 style in config file

Specifying checks using the literal style (|) in the clang-tidy config file is 
currently
supported but was not implemented for the --verify-config options.
This means that clang-tidy would work properly but, using the --verify-config 
option
would raise an error due to some checks not being parsed properly.

Fixes #53737
---
 clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp  |  9 ++---
 clang-tools-extra/docs/ReleaseNotes.rst  |  3 +++
 .../test/clang-tidy/infrastructure/verify-config.cpp | 12 
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp 
b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index 9f3d6b6db6cbca..b68a6215b383a6 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -454,11 +454,14 @@ static constexpr StringLiteral VerifyConfigWarningEnd = " 
[-verify-config]\n";
 
 static bool verifyChecks(const StringSet<> , StringRef CheckGlob,
  StringRef Source) {
-  llvm::StringRef Cur, Rest;
+  llvm::StringRef Cur = CheckGlob;
+  llvm::StringRef Rest = CheckGlob;
   bool AnyInvalid = false;
-  for (std::tie(Cur, Rest) = CheckGlob.split(',');
-   !(Cur.empty() && Rest.empty()); std::tie(Cur, Rest) = Rest.split(',')) {
+  while (!Cur.empty() || !Rest.empty()) {
+Cur = Rest.substr(0, Rest.find_first_of(",\n"));
+Rest = Rest.substr(Cur.size() + 1);
 Cur = Cur.trim();
+
 if (Cur.empty())
   continue;
 Cur.consume_front("-");
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index a604e9276668ae..3887384e713896 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -262,6 +262,9 @@ Miscellaneous
   option is specified. Now ``clang-apply-replacements`` applies formatting 
only with
   the option.
 
+- Fixed ``--verify-check`` option not properly parsing checks when using the 
+  literal operator in the ``.clang-tidy`` config
+
 Improvements to include-fixer
 -
 
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp 
b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
index 421f8641281acb..3659285986482a 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
@@ -18,3 +18,15 @@
 // CHECK-VERIFY: command-line option '-checks': warning: check glob 'bad*glob' 
doesn't match any known check [-verify-config]
 // CHECK-VERIFY: command-line option '-checks': warning: unknown check 
'llvm-includeorder'; did you mean 'llvm-include-order' [-verify-config]
 // CHECK-VERIFY: command-line option '-checks': warning: unknown check 
'my-made-up-check' [-verify-config]
+
+// RUN: echo -e 'Checks: |\n bugprone-argument-comment\n 
bugprone-assert-side-effect,\n bugprone-bool-pointer-implicit-conversion\n 
readability-use-anyof*' > %T/MyClangTidyConfig
+// RUN: clang-tidy -verify-config \
+// RUN: --config-file=%T/MyClangTidyConfig | FileCheck %s 
-check-prefix=CHECK-VERIFY-BLOCK-OK
+// CHECK-VERIFY-BLOCK-OK: No config errors detected.
+
+// RUN: echo -e 'Checks: |\n bugprone-arguments-*\n 
bugprone-assert-side-effects\n bugprone-bool-pointer-implicit-conversion' > 
%T/MyClangTidyConfigBad
+// RUN: not clang-tidy -verify-config \
+// RUN: --config-file=%T/MyClangTidyConfigBad 2>&1 | FileCheck %s 
-check-prefix=CHECK-VERIFY-BLOCK-BAD
+// CHECK-VERIFY-BLOCK-BAD: command-line option '-config': warning: check glob 
'bugprone-arguments-*' doesn't match any known check [-verify-config]
+// CHECK-VERIFY-BLOCK-BAD: command-line option '-config': warning: unknown 
check 'bugprone-assert-side-effects'; did you mean 
'bugprone-assert-side-effect' [-verify-config]
+

From 0e2cb54934829f42069b06f599fa9e724bdf40b3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sat, 23 Mar 2024 13:30:28 -0400
Subject: [PATCH 2/3] fixup! [clang-tidy] Improved --verify-config when using
 literal style in config file

Partially rewrote verifyChecks() to realign the checks parsing logic.

Instead of manually parsing the string of globs, verifyCHecks() now uses a 
GlobList which is responsible
to parse the string and return a list of regexes. This ensures that any changes 
made to the checks parsing logic
has to be made at only one place rather than two.
---
 clang-tools-extra/clang-tidy/GlobList.cpp | 14 ++--
 clang-tools-extra/clang-tidy/GlobList.h   |  4 ++
 .../clang-tidy/tool/ClangTidyMain.cpp 

[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)

2024-03-17 Thread Félix-Antoine Constantin via cfe-commits

felix642 wrote:

@PiotrZSL I've squashed my changed. When you'll have time, I think we can merge 
this PR. 

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


[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)

2024-03-17 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/82947

From e67122cb2c17395e863725cc25d99afe6c7bb48c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 25 Feb 2024 20:20:59 -0500
Subject: [PATCH] [clang-tidy] Improved modernize-use-using by fixing a
 false-negative

The check needs a parent decl to match but if the typedef is in a function,
the parent is a declStmt which is not a decl by itself.
Improved the matcher to match on either a decl or a declstmt and extract
the decl from the stmt in the latter case.

fixes #72179
---
 .../clang-tidy/modernize/UseUsingCheck.cpp| 24 +--
 clang-tools-extra/docs/ReleaseNotes.rst   |  3 ++
 .../checkers/modernize/use-using.cpp  | 43 ++-
 3 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
index bb05f206c717cea..24eefdb082eb32a 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -8,6 +8,7 @@
 
 #include "UseUsingCheck.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclGroup.h"
 #include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
@@ -24,6 +25,7 @@ static constexpr llvm::StringLiteral ExternCDeclName = 
"extern-c-decl";
 static constexpr llvm::StringLiteral ParentDeclName = "parent-decl";
 static constexpr llvm::StringLiteral TagDeclName = "tag-decl";
 static constexpr llvm::StringLiteral TypedefName = "typedef";
+static constexpr llvm::StringLiteral DeclStmtName = "decl-stmt";
 
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
@@ -41,7 +43,8 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   unless(isInstantiated()),
   optionally(hasAncestor(
   linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))),
-  hasParent(decl().bind(ParentDeclName)))
+  anyOf(hasParent(decl().bind(ParentDeclName)),
+hasParent(declStmt().bind(DeclStmtName
   .bind(TypedefName),
   this);
 
@@ -51,17 +54,32 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   tagDecl(
   anyOf(allOf(unless(anyOf(isImplicit(),
classTemplateSpecializationDecl())),
-  hasParent(decl().bind(ParentDeclName))),
+  anyOf(hasParent(decl().bind(ParentDeclName)),
+hasParent(declStmt().bind(DeclStmtName,
 // We want the parent of the ClassTemplateDecl, not the parent
 // of the specialization.
 classTemplateSpecializationDecl(hasAncestor(classTemplateDecl(
-hasParent(decl().bind(ParentDeclName)))
+anyOf(hasParent(decl().bind(ParentDeclName)),
+  hasParent(declStmt().bind(DeclStmtName
   .bind(TagDeclName),
   this);
 }
 
 void UseUsingCheck::check(const MatchFinder::MatchResult ) {
   const auto *ParentDecl = Result.Nodes.getNodeAs(ParentDeclName);
+
+  if (!ParentDecl) {
+const auto *ParentDeclStmt = 
Result.Nodes.getNodeAs(DeclStmtName);
+if (ParentDeclStmt) {
+  if (ParentDeclStmt->isSingleDecl())
+ParentDecl = ParentDeclStmt->getSingleDecl();
+  else
+ParentDecl =
+ParentDeclStmt->getDeclGroup().getDeclGroup()
+[ParentDeclStmt->getDeclGroup().getDeclGroup().size() - 1];
+}
+  }
+
   if (!ParentDecl)
 return;
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 44680f79de6f543..7e4c75b5c3a2948 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -224,6 +224,9 @@ Changes in existing checks
   analyzed, se the check now handles the common patterns
   `const auto e = (*vector_ptr)[i]` and `const auto e = vector_ptr->at(i);`.
 
+- Improved :doc:`modernize-use-using `
+  check by adding support for detection of typedefs declared on function level.
+
 - Improved :doc:`readability-implicit-bool-conversion
   ` check to provide
   valid fix suggestions for ``static_cast`` without a preceding space and
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
index 462bc984fd3ad56..925e5f9c1ca54e8 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s modernize-use-using %t -- -- -I 
%S/Inputs/use-using/
+// RUN: %check_clang_tidy %s modernize-use-using %t -- -- 
-fno-delayed-template-parsing -I %S/Inputs/use-using/
 
 typedef int Type;
 // CHECK-MESSAGES: 

[clang-tools-extra] [clang-tidy] Improved --verify-config when using literal style in config file (PR #85591)

2024-03-17 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 created 
https://github.com/llvm/llvm-project/pull/85591

Specifying checks using the literal style (|) in the clang-tidy config file is 
currently supported but was not implemented for the --verify-config options. 
This means that clang-tidy would work properly but, using the --verify-config 
option would raise an error due to some checks not being parsed properly.

Fixes #53737

CC @SimplyDanny 

From 2ada09c8e10ecd05e3881995e96cfc2070331f05 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 17 Mar 2024 20:50:17 -0400
Subject: [PATCH] [clang-tidy] Improved --verify-config when using literal
 style in config file

Specifying checks using the literal style (|) in the clang-tidy config file is 
currently
supported but was not implemented for the --verify-config options.
This means that clang-tidy would work properly but, using the --verify-config 
option
would raise an error due to some checks not being parsed properly.

Fixes #53737
---
 clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp  |  9 ++---
 clang-tools-extra/docs/ReleaseNotes.rst  |  3 +++
 .../test/clang-tidy/infrastructure/verify-config.cpp | 12 
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp 
b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index 9f3d6b6db6cbca..b68a6215b383a6 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -454,11 +454,14 @@ static constexpr StringLiteral VerifyConfigWarningEnd = " 
[-verify-config]\n";
 
 static bool verifyChecks(const StringSet<> , StringRef CheckGlob,
  StringRef Source) {
-  llvm::StringRef Cur, Rest;
+  llvm::StringRef Cur = CheckGlob;
+  llvm::StringRef Rest = CheckGlob;
   bool AnyInvalid = false;
-  for (std::tie(Cur, Rest) = CheckGlob.split(',');
-   !(Cur.empty() && Rest.empty()); std::tie(Cur, Rest) = Rest.split(',')) {
+  while (!Cur.empty() || !Rest.empty()) {
+Cur = Rest.substr(0, Rest.find_first_of(",\n"));
+Rest = Rest.substr(Cur.size() + 1);
 Cur = Cur.trim();
+
 if (Cur.empty())
   continue;
 Cur.consume_front("-");
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 44680f79de6f54..a699aa9aadd908 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -251,6 +251,9 @@ Miscellaneous
   option is specified. Now ``clang-apply-replacements`` applies formatting 
only with
   the option.
 
+- Fixed ``--verify-check`` option not properly parsing checks when using the 
+  literal operator in the ``.clang-tidy`` config
+
 Improvements to include-fixer
 -
 
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp 
b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
index 421f8641281acb..3659285986482a 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp
@@ -18,3 +18,15 @@
 // CHECK-VERIFY: command-line option '-checks': warning: check glob 'bad*glob' 
doesn't match any known check [-verify-config]
 // CHECK-VERIFY: command-line option '-checks': warning: unknown check 
'llvm-includeorder'; did you mean 'llvm-include-order' [-verify-config]
 // CHECK-VERIFY: command-line option '-checks': warning: unknown check 
'my-made-up-check' [-verify-config]
+
+// RUN: echo -e 'Checks: |\n bugprone-argument-comment\n 
bugprone-assert-side-effect,\n bugprone-bool-pointer-implicit-conversion\n 
readability-use-anyof*' > %T/MyClangTidyConfig
+// RUN: clang-tidy -verify-config \
+// RUN: --config-file=%T/MyClangTidyConfig | FileCheck %s 
-check-prefix=CHECK-VERIFY-BLOCK-OK
+// CHECK-VERIFY-BLOCK-OK: No config errors detected.
+
+// RUN: echo -e 'Checks: |\n bugprone-arguments-*\n 
bugprone-assert-side-effects\n bugprone-bool-pointer-implicit-conversion' > 
%T/MyClangTidyConfigBad
+// RUN: not clang-tidy -verify-config \
+// RUN: --config-file=%T/MyClangTidyConfigBad 2>&1 | FileCheck %s 
-check-prefix=CHECK-VERIFY-BLOCK-BAD
+// CHECK-VERIFY-BLOCK-BAD: command-line option '-config': warning: check glob 
'bugprone-arguments-*' doesn't match any known check [-verify-config]
+// CHECK-VERIFY-BLOCK-BAD: command-line option '-config': warning: unknown 
check 'bugprone-assert-side-effects'; did you mean 
'bugprone-assert-side-effect' [-verify-config]
+

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


[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)

2024-03-10 Thread Félix-Antoine Constantin via cfe-commits

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


[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)

2024-03-10 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/82947

From f1d957413bcc81e9ced06ad40570859769f4c1c1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 25 Feb 2024 20:20:59 -0500
Subject: [PATCH 1/6] [clang-tidy] Improved modernize-use-using by fixing a
 false-negative

The check needs a parent decl to match but if the typedef is in a function,
the parent is a declStmt which is not a decl by itself.
Improved the matcher to match on either a decl or a declstmt and extract
the decl from the stmt in the latter case.

fixes #72179
---
 .../clang-tidy/modernize/UseUsingCheck.cpp | 16 +---
 clang-tools-extra/docs/ReleaseNotes.rst|  3 +++
 .../checkers/modernize/use-using.cpp   | 18 ++
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
index bb05f206c717ce..50a07fc02e31b4 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -24,6 +24,7 @@ static constexpr llvm::StringLiteral ExternCDeclName = 
"extern-c-decl";
 static constexpr llvm::StringLiteral ParentDeclName = "parent-decl";
 static constexpr llvm::StringLiteral TagDeclName = "tag-decl";
 static constexpr llvm::StringLiteral TypedefName = "typedef";
+static constexpr llvm::StringLiteral DeclStmtName = "decl-stmt";
 
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
@@ -41,7 +42,8 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   unless(isInstantiated()),
   optionally(hasAncestor(
   linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))),
-  hasParent(decl().bind(ParentDeclName)))
+  anyOf(hasParent(decl().bind(ParentDeclName)),
+hasParent(declStmt().bind(DeclStmtName
   .bind(TypedefName),
   this);
 
@@ -51,17 +53,25 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   tagDecl(
   anyOf(allOf(unless(anyOf(isImplicit(),
classTemplateSpecializationDecl())),
-  hasParent(decl().bind(ParentDeclName))),
+  anyOf(hasParent(decl().bind(ParentDeclName)),
+hasParent(declStmt().bind(DeclStmtName,
 // We want the parent of the ClassTemplateDecl, not the parent
 // of the specialization.
 classTemplateSpecializationDecl(hasAncestor(classTemplateDecl(
-hasParent(decl().bind(ParentDeclName)))
+anyOf(hasParent(decl().bind(ParentDeclName)),
+  hasParent(declStmt().bind(DeclStmtName
   .bind(TagDeclName),
   this);
 }
 
 void UseUsingCheck::check(const MatchFinder::MatchResult ) {
   const auto *ParentDecl = Result.Nodes.getNodeAs(ParentDeclName);
+
+  if (!ParentDecl) {
+const auto *ParentDeclStmt = 
Result.Nodes.getNodeAs(DeclStmtName);
+ParentDecl = ParentDeclStmt->getSingleDecl();
+  }
+
   if (!ParentDecl)
 return;
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 44680f79de6f54..479b9b86444184 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -224,6 +224,9 @@ Changes in existing checks
   analyzed, se the check now handles the common patterns
   `const auto e = (*vector_ptr)[i]` and `const auto e = vector_ptr->at(i);`.
 
+- Improved :doc:`modernize-use-using `
+  check by fixing false-negative in functions.
+
 - Improved :doc:`readability-implicit-bool-conversion
   ` check to provide
   valid fix suggestions for ``static_cast`` without a preceding space and
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
index 462bc984fd3ad5..230c7c94cda1cf 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
@@ -342,3 +342,21 @@ typedef int InExternCPP;
 // CHECK-FIXES: using InExternCPP = int;
 
 }
+
+namespace ISSUE_72179
+{  
+  void foo()
+  {
+typedef int a;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 
'typedef' [modernize-use-using]
+// CHECK-FIXES: using a = int;
+
+  }
+
+  void foo2()
+  {
+typedef struct { int a; union { int b; }; } c;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 
'typedef' [modernize-use-using]
+// CHECK-FIXES: using c = struct { int a; union { int b; }; };
+  }
+}

From e7dcbbcf92e896354e9a52da259251a7d3a74369 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 25 Feb 2024 22:14:22 -0500

[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)

2024-03-10 Thread Félix-Antoine Constantin via cfe-commits

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


[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)

2024-03-10 Thread Félix-Antoine Constantin via cfe-commits

felix642 wrote:

Thank you for the quick review @PiotrZSL. I've implemented the proposed changes 
and rebased on top of main. We can wait a few days to see if someone else has 
some comments to add and then we should be ready to merge 

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


[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)

2024-03-10 Thread Félix-Antoine Constantin via cfe-commits

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


[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)

2024-03-10 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/82947

From f1d957413bcc81e9ced06ad40570859769f4c1c1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 25 Feb 2024 20:20:59 -0500
Subject: [PATCH 1/5] [clang-tidy] Improved modernize-use-using by fixing a
 false-negative

The check needs a parent decl to match but if the typedef is in a function,
the parent is a declStmt which is not a decl by itself.
Improved the matcher to match on either a decl or a declstmt and extract
the decl from the stmt in the latter case.

fixes #72179
---
 .../clang-tidy/modernize/UseUsingCheck.cpp | 16 +---
 clang-tools-extra/docs/ReleaseNotes.rst|  3 +++
 .../checkers/modernize/use-using.cpp   | 18 ++
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
index bb05f206c717ce..50a07fc02e31b4 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -24,6 +24,7 @@ static constexpr llvm::StringLiteral ExternCDeclName = 
"extern-c-decl";
 static constexpr llvm::StringLiteral ParentDeclName = "parent-decl";
 static constexpr llvm::StringLiteral TagDeclName = "tag-decl";
 static constexpr llvm::StringLiteral TypedefName = "typedef";
+static constexpr llvm::StringLiteral DeclStmtName = "decl-stmt";
 
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
@@ -41,7 +42,8 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   unless(isInstantiated()),
   optionally(hasAncestor(
   linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))),
-  hasParent(decl().bind(ParentDeclName)))
+  anyOf(hasParent(decl().bind(ParentDeclName)),
+hasParent(declStmt().bind(DeclStmtName
   .bind(TypedefName),
   this);
 
@@ -51,17 +53,25 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   tagDecl(
   anyOf(allOf(unless(anyOf(isImplicit(),
classTemplateSpecializationDecl())),
-  hasParent(decl().bind(ParentDeclName))),
+  anyOf(hasParent(decl().bind(ParentDeclName)),
+hasParent(declStmt().bind(DeclStmtName,
 // We want the parent of the ClassTemplateDecl, not the parent
 // of the specialization.
 classTemplateSpecializationDecl(hasAncestor(classTemplateDecl(
-hasParent(decl().bind(ParentDeclName)))
+anyOf(hasParent(decl().bind(ParentDeclName)),
+  hasParent(declStmt().bind(DeclStmtName
   .bind(TagDeclName),
   this);
 }
 
 void UseUsingCheck::check(const MatchFinder::MatchResult ) {
   const auto *ParentDecl = Result.Nodes.getNodeAs(ParentDeclName);
+
+  if (!ParentDecl) {
+const auto *ParentDeclStmt = 
Result.Nodes.getNodeAs(DeclStmtName);
+ParentDecl = ParentDeclStmt->getSingleDecl();
+  }
+
   if (!ParentDecl)
 return;
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 44680f79de6f54..479b9b86444184 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -224,6 +224,9 @@ Changes in existing checks
   analyzed, se the check now handles the common patterns
   `const auto e = (*vector_ptr)[i]` and `const auto e = vector_ptr->at(i);`.
 
+- Improved :doc:`modernize-use-using `
+  check by fixing false-negative in functions.
+
 - Improved :doc:`readability-implicit-bool-conversion
   ` check to provide
   valid fix suggestions for ``static_cast`` without a preceding space and
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
index 462bc984fd3ad5..230c7c94cda1cf 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
@@ -342,3 +342,21 @@ typedef int InExternCPP;
 // CHECK-FIXES: using InExternCPP = int;
 
 }
+
+namespace ISSUE_72179
+{  
+  void foo()
+  {
+typedef int a;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 
'typedef' [modernize-use-using]
+// CHECK-FIXES: using a = int;
+
+  }
+
+  void foo2()
+  {
+typedef struct { int a; union { int b; }; } c;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 
'typedef' [modernize-use-using]
+// CHECK-FIXES: using c = struct { int a; union { int b; }; };
+  }
+}

From e7dcbbcf92e896354e9a52da259251a7d3a74369 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 25 Feb 2024 22:14:22 -0500

[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)

2024-03-10 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/82947

From d1cbed0e2e83bd3544067fd25d7e811f1ab3f095 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 25 Feb 2024 20:20:59 -0500
Subject: [PATCH 1/4] [clang-tidy] Improved modernize-use-using by fixing a
 false-negative

The check needs a parent decl to match but if the typedef is in a function,
the parent is a declStmt which is not a decl by itself.
Improved the matcher to match on either a decl or a declstmt and extract
the decl from the stmt in the latter case.

fixes #72179
---
 .../clang-tidy/modernize/UseUsingCheck.cpp | 16 +---
 clang-tools-extra/docs/ReleaseNotes.rst|  3 +++
 .../checkers/modernize/use-using.cpp   | 18 ++
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
index bb05f206c717ce..50a07fc02e31b4 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -24,6 +24,7 @@ static constexpr llvm::StringLiteral ExternCDeclName = 
"extern-c-decl";
 static constexpr llvm::StringLiteral ParentDeclName = "parent-decl";
 static constexpr llvm::StringLiteral TagDeclName = "tag-decl";
 static constexpr llvm::StringLiteral TypedefName = "typedef";
+static constexpr llvm::StringLiteral DeclStmtName = "decl-stmt";
 
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
@@ -41,7 +42,8 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   unless(isInstantiated()),
   optionally(hasAncestor(
   linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))),
-  hasParent(decl().bind(ParentDeclName)))
+  anyOf(hasParent(decl().bind(ParentDeclName)),
+hasParent(declStmt().bind(DeclStmtName
   .bind(TypedefName),
   this);
 
@@ -51,17 +53,25 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   tagDecl(
   anyOf(allOf(unless(anyOf(isImplicit(),
classTemplateSpecializationDecl())),
-  hasParent(decl().bind(ParentDeclName))),
+  anyOf(hasParent(decl().bind(ParentDeclName)),
+hasParent(declStmt().bind(DeclStmtName,
 // We want the parent of the ClassTemplateDecl, not the parent
 // of the specialization.
 classTemplateSpecializationDecl(hasAncestor(classTemplateDecl(
-hasParent(decl().bind(ParentDeclName)))
+anyOf(hasParent(decl().bind(ParentDeclName)),
+  hasParent(declStmt().bind(DeclStmtName
   .bind(TagDeclName),
   this);
 }
 
 void UseUsingCheck::check(const MatchFinder::MatchResult ) {
   const auto *ParentDecl = Result.Nodes.getNodeAs(ParentDeclName);
+
+  if (!ParentDecl) {
+const auto *ParentDeclStmt = 
Result.Nodes.getNodeAs(DeclStmtName);
+ParentDecl = ParentDeclStmt->getSingleDecl();
+  }
+
   if (!ParentDecl)
 return;
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 6fd01ed9d471c5..2b36ae4acb9f1d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -183,6 +183,9 @@ Changes in existing checks
   ` check to also remove any trailing
   whitespace when deleting the ``virtual`` keyword.
 
+- Improved :doc:`modernize-use-using `
+  check by fixing false-negative in functions.
+
 - Improved :doc:`readability-implicit-bool-conversion
   ` check to provide
   valid fix suggestions for ``static_cast`` without a preceding space and
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
index 462bc984fd3ad5..230c7c94cda1cf 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
@@ -342,3 +342,21 @@ typedef int InExternCPP;
 // CHECK-FIXES: using InExternCPP = int;
 
 }
+
+namespace ISSUE_72179
+{  
+  void foo()
+  {
+typedef int a;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 
'typedef' [modernize-use-using]
+// CHECK-FIXES: using a = int;
+
+  }
+
+  void foo2()
+  {
+typedef struct { int a; union { int b; }; } c;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 
'typedef' [modernize-use-using]
+// CHECK-FIXES: using c = struct { int a; union { int b; }; };
+  }
+}

From 0b5c78508fbf49229d576171d7af6c6bb16b4367 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 25 Feb 2024 22:14:22 -0500
Subject: [PATCH 2/4] fixup! [clang-tidy] 

[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)

2024-03-09 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/82947

From d1cbed0e2e83bd3544067fd25d7e811f1ab3f095 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 25 Feb 2024 20:20:59 -0500
Subject: [PATCH 1/3] [clang-tidy] Improved modernize-use-using by fixing a
 false-negative

The check needs a parent decl to match but if the typedef is in a function,
the parent is a declStmt which is not a decl by itself.
Improved the matcher to match on either a decl or a declstmt and extract
the decl from the stmt in the latter case.

fixes #72179
---
 .../clang-tidy/modernize/UseUsingCheck.cpp | 16 +---
 clang-tools-extra/docs/ReleaseNotes.rst|  3 +++
 .../checkers/modernize/use-using.cpp   | 18 ++
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
index bb05f206c717ce..50a07fc02e31b4 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -24,6 +24,7 @@ static constexpr llvm::StringLiteral ExternCDeclName = 
"extern-c-decl";
 static constexpr llvm::StringLiteral ParentDeclName = "parent-decl";
 static constexpr llvm::StringLiteral TagDeclName = "tag-decl";
 static constexpr llvm::StringLiteral TypedefName = "typedef";
+static constexpr llvm::StringLiteral DeclStmtName = "decl-stmt";
 
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
@@ -41,7 +42,8 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   unless(isInstantiated()),
   optionally(hasAncestor(
   linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))),
-  hasParent(decl().bind(ParentDeclName)))
+  anyOf(hasParent(decl().bind(ParentDeclName)),
+hasParent(declStmt().bind(DeclStmtName
   .bind(TypedefName),
   this);
 
@@ -51,17 +53,25 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   tagDecl(
   anyOf(allOf(unless(anyOf(isImplicit(),
classTemplateSpecializationDecl())),
-  hasParent(decl().bind(ParentDeclName))),
+  anyOf(hasParent(decl().bind(ParentDeclName)),
+hasParent(declStmt().bind(DeclStmtName,
 // We want the parent of the ClassTemplateDecl, not the parent
 // of the specialization.
 classTemplateSpecializationDecl(hasAncestor(classTemplateDecl(
-hasParent(decl().bind(ParentDeclName)))
+anyOf(hasParent(decl().bind(ParentDeclName)),
+  hasParent(declStmt().bind(DeclStmtName
   .bind(TagDeclName),
   this);
 }
 
 void UseUsingCheck::check(const MatchFinder::MatchResult ) {
   const auto *ParentDecl = Result.Nodes.getNodeAs(ParentDeclName);
+
+  if (!ParentDecl) {
+const auto *ParentDeclStmt = 
Result.Nodes.getNodeAs(DeclStmtName);
+ParentDecl = ParentDeclStmt->getSingleDecl();
+  }
+
   if (!ParentDecl)
 return;
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 6fd01ed9d471c5..2b36ae4acb9f1d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -183,6 +183,9 @@ Changes in existing checks
   ` check to also remove any trailing
   whitespace when deleting the ``virtual`` keyword.
 
+- Improved :doc:`modernize-use-using `
+  check by fixing false-negative in functions.
+
 - Improved :doc:`readability-implicit-bool-conversion
   ` check to provide
   valid fix suggestions for ``static_cast`` without a preceding space and
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
index 462bc984fd3ad5..230c7c94cda1cf 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
@@ -342,3 +342,21 @@ typedef int InExternCPP;
 // CHECK-FIXES: using InExternCPP = int;
 
 }
+
+namespace ISSUE_72179
+{  
+  void foo()
+  {
+typedef int a;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 
'typedef' [modernize-use-using]
+// CHECK-FIXES: using a = int;
+
+  }
+
+  void foo2()
+  {
+typedef struct { int a; union { int b; }; } c;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 
'typedef' [modernize-use-using]
+// CHECK-FIXES: using c = struct { int a; union { int b; }; };
+  }
+}

From 0b5c78508fbf49229d576171d7af6c6bb16b4367 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 25 Feb 2024 22:14:22 -0500
Subject: [PATCH 2/3] fixup! [clang-tidy] 

[clang-tools-extra] [clang-tidy] bugprone-unused-return-value config now supports regexes (PR #82952)

2024-03-04 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/82952

From 00885ea60007a1da88f9d476e14252f950f358a1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 25 Feb 2024 22:05:08 -0500
Subject: [PATCH 1/2] [clang-tidy] bugprone-unused-return-value config now
 support regexes

Fixes #63107
---
 .../bugprone/UnusedReturnValueCheck.cpp   | 202 +-
 .../bugprone/UnusedReturnValueCheck.h |   6 +-
 .../hicpp/IgnoredRemoveResultCheck.cpp|   8 +-
 clang-tools-extra/docs/ReleaseNotes.rst   |   4 +
 .../checks/bugprone/unused-return-value.rst   |   5 +-
 .../bugprone/unused-return-value-custom.cpp   |   2 +-
 6 files changed, 117 insertions(+), 110 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 05012c7df6a975..b4bf85c912c3ca 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -33,98 +33,98 @@ AST_MATCHER_P(FunctionDecl, isInstantiatedFrom, 
Matcher,
 UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name,
ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  CheckedFunctions(Options.get("CheckedFunctions",
-   "::std::async;"
-   "::std::launder;"
-   "::std::remove;"
-   "::std::remove_if;"
-   "::std::unique;"
-   "::std::unique_ptr::release;"
-   "::std::basic_string::empty;"
-   "::std::vector::empty;"
-   "::std::back_inserter;"
-   "::std::distance;"
-   "::std::find;"
-   "::std::find_if;"
-   "::std::inserter;"
-   "::std::lower_bound;"
-   "::std::make_pair;"
-   "::std::map::count;"
-   "::std::map::find;"
-   "::std::map::lower_bound;"
-   "::std::multimap::equal_range;"
-   "::std::multimap::upper_bound;"
-   "::std::set::count;"
-   "::std::set::find;"
-   "::std::setfill;"
-   "::std::setprecision;"
-   "::std::setw;"
-   "::std::upper_bound;"
-   "::std::vector::at;"
-   // C standard library
-   "::bsearch;"
-   "::ferror;"
-   "::feof;"
-   "::isalnum;"
-   "::isalpha;"
-   "::isblank;"
-   "::iscntrl;"
-   "::isdigit;"
-   "::isgraph;"
-   "::islower;"
-   "::isprint;"
-   "::ispunct;"
-   "::isspace;"
-   "::isupper;"
-   "::iswalnum;"
-   "::iswprint;"
-   "::iswspace;"
-   "::isxdigit;"
-   "::memchr;"
-   "::memcmp;"
-   "::strcmp;"
-   "::strcoll;"
-   "::strncmp;"
-   "::strpbrk;"
-   "::strrchr;"
-   "::strspn;"
-   "::strstr;"
-   "::wcscmp;"
-   // POSIX
-   "::access;"
-   "::bind;"
-   "::connect;"
-   "::difftime;"
-   "::dlsym;"
-   "::fnmatch;"
-   "::getaddrinfo;"
-   "::getopt;"
-   "::htonl;"
-   "::htons;"
-   "::iconv_open;"
-

[clang-tools-extra] [clang-tidy] bugprone-unused-return-value config now supports regexes (PR #82952)

2024-03-04 Thread Félix-Antoine Constantin via cfe-commits

felix642 wrote:

Nevermind I broke the documentation.. Hold on

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


[clang-tools-extra] [clang-tidy] bugprone-unused-return-value config now supports regexes (PR #82952)

2024-03-04 Thread Félix-Antoine Constantin via cfe-commits

felix642 wrote:

> Rebase, and could be merged.

Done, thank you for the quick review.

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


[clang-tools-extra] [clang-tidy] bugprone-unused-return-value config now supports regexes (PR #82952)

2024-03-04 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/82952

From 00885ea60007a1da88f9d476e14252f950f358a1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 25 Feb 2024 22:05:08 -0500
Subject: [PATCH] [clang-tidy] bugprone-unused-return-value config now support
 regexes

Fixes #63107
---
 .../bugprone/UnusedReturnValueCheck.cpp   | 202 +-
 .../bugprone/UnusedReturnValueCheck.h |   6 +-
 .../hicpp/IgnoredRemoveResultCheck.cpp|   8 +-
 clang-tools-extra/docs/ReleaseNotes.rst   |   4 +
 .../checks/bugprone/unused-return-value.rst   |   5 +-
 .../bugprone/unused-return-value-custom.cpp   |   2 +-
 6 files changed, 117 insertions(+), 110 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 05012c7df6a975..b4bf85c912c3ca 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -33,98 +33,98 @@ AST_MATCHER_P(FunctionDecl, isInstantiatedFrom, 
Matcher,
 UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name,
ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  CheckedFunctions(Options.get("CheckedFunctions",
-   "::std::async;"
-   "::std::launder;"
-   "::std::remove;"
-   "::std::remove_if;"
-   "::std::unique;"
-   "::std::unique_ptr::release;"
-   "::std::basic_string::empty;"
-   "::std::vector::empty;"
-   "::std::back_inserter;"
-   "::std::distance;"
-   "::std::find;"
-   "::std::find_if;"
-   "::std::inserter;"
-   "::std::lower_bound;"
-   "::std::make_pair;"
-   "::std::map::count;"
-   "::std::map::find;"
-   "::std::map::lower_bound;"
-   "::std::multimap::equal_range;"
-   "::std::multimap::upper_bound;"
-   "::std::set::count;"
-   "::std::set::find;"
-   "::std::setfill;"
-   "::std::setprecision;"
-   "::std::setw;"
-   "::std::upper_bound;"
-   "::std::vector::at;"
-   // C standard library
-   "::bsearch;"
-   "::ferror;"
-   "::feof;"
-   "::isalnum;"
-   "::isalpha;"
-   "::isblank;"
-   "::iscntrl;"
-   "::isdigit;"
-   "::isgraph;"
-   "::islower;"
-   "::isprint;"
-   "::ispunct;"
-   "::isspace;"
-   "::isupper;"
-   "::iswalnum;"
-   "::iswprint;"
-   "::iswspace;"
-   "::isxdigit;"
-   "::memchr;"
-   "::memcmp;"
-   "::strcmp;"
-   "::strcoll;"
-   "::strncmp;"
-   "::strpbrk;"
-   "::strrchr;"
-   "::strspn;"
-   "::strstr;"
-   "::wcscmp;"
-   // POSIX
-   "::access;"
-   "::bind;"
-   "::connect;"
-   "::difftime;"
-   "::dlsym;"
-   "::fnmatch;"
-   "::getaddrinfo;"
-   "::getopt;"
-   "::htonl;"
-   "::htons;"
-   "::iconv_open;"
-

[clang-tools-extra] [clang-tidy] bugprone-unused-return-value config now supports regexes (PR #82952)

2024-03-04 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/82952

From 5e30d84368a05bab31a26c413fdc8092449f4111 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 25 Feb 2024 22:05:08 -0500
Subject: [PATCH 1/2] [clang-tidy] bugprone-unused-return-value config now
 support regexes

Fixes #63107
---
 .../bugprone/UnusedReturnValueCheck.cpp   | 202 +-
 .../bugprone/UnusedReturnValueCheck.h |   6 +-
 .../hicpp/IgnoredRemoveResultCheck.cpp|   8 +-
 clang-tools-extra/docs/ReleaseNotes.rst   |   4 +
 .../bugprone/unused-return-value-custom.cpp   |   2 +-
 5 files changed, 114 insertions(+), 108 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 05012c7df6a975..b4bf85c912c3ca 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -33,98 +33,98 @@ AST_MATCHER_P(FunctionDecl, isInstantiatedFrom, 
Matcher,
 UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name,
ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  CheckedFunctions(Options.get("CheckedFunctions",
-   "::std::async;"
-   "::std::launder;"
-   "::std::remove;"
-   "::std::remove_if;"
-   "::std::unique;"
-   "::std::unique_ptr::release;"
-   "::std::basic_string::empty;"
-   "::std::vector::empty;"
-   "::std::back_inserter;"
-   "::std::distance;"
-   "::std::find;"
-   "::std::find_if;"
-   "::std::inserter;"
-   "::std::lower_bound;"
-   "::std::make_pair;"
-   "::std::map::count;"
-   "::std::map::find;"
-   "::std::map::lower_bound;"
-   "::std::multimap::equal_range;"
-   "::std::multimap::upper_bound;"
-   "::std::set::count;"
-   "::std::set::find;"
-   "::std::setfill;"
-   "::std::setprecision;"
-   "::std::setw;"
-   "::std::upper_bound;"
-   "::std::vector::at;"
-   // C standard library
-   "::bsearch;"
-   "::ferror;"
-   "::feof;"
-   "::isalnum;"
-   "::isalpha;"
-   "::isblank;"
-   "::iscntrl;"
-   "::isdigit;"
-   "::isgraph;"
-   "::islower;"
-   "::isprint;"
-   "::ispunct;"
-   "::isspace;"
-   "::isupper;"
-   "::iswalnum;"
-   "::iswprint;"
-   "::iswspace;"
-   "::isxdigit;"
-   "::memchr;"
-   "::memcmp;"
-   "::strcmp;"
-   "::strcoll;"
-   "::strncmp;"
-   "::strpbrk;"
-   "::strrchr;"
-   "::strspn;"
-   "::strstr;"
-   "::wcscmp;"
-   // POSIX
-   "::access;"
-   "::bind;"
-   "::connect;"
-   "::difftime;"
-   "::dlsym;"
-   "::fnmatch;"
-   "::getaddrinfo;"
-   "::getopt;"
-   "::htonl;"
-   "::htons;"
-   "::iconv_open;"
-   "::inet_addr;"
- 

[clang-tools-extra] [clang-tidy] bugprone-unused-return-value config now supports regexes (PR #82952)

2024-03-04 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/82952

From 86bc8a57fd56a6b2593af8bcf85d50bbc0ce984e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 25 Feb 2024 22:05:08 -0500
Subject: [PATCH 1/2] [clang-tidy] bugprone-unused-return-value config now
 support regexes

Fixes #63107
---
 .../bugprone/UnusedReturnValueCheck.cpp   | 202 +-
 .../bugprone/UnusedReturnValueCheck.h |   6 +-
 .../hicpp/IgnoredRemoveResultCheck.cpp|   8 +-
 clang-tools-extra/docs/ReleaseNotes.rst   |   4 +
 .../bugprone/unused-return-value-custom.cpp   |   2 +-
 5 files changed, 114 insertions(+), 108 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 05012c7df6a975..b4bf85c912c3ca 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -33,98 +33,98 @@ AST_MATCHER_P(FunctionDecl, isInstantiatedFrom, 
Matcher,
 UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name,
ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  CheckedFunctions(Options.get("CheckedFunctions",
-   "::std::async;"
-   "::std::launder;"
-   "::std::remove;"
-   "::std::remove_if;"
-   "::std::unique;"
-   "::std::unique_ptr::release;"
-   "::std::basic_string::empty;"
-   "::std::vector::empty;"
-   "::std::back_inserter;"
-   "::std::distance;"
-   "::std::find;"
-   "::std::find_if;"
-   "::std::inserter;"
-   "::std::lower_bound;"
-   "::std::make_pair;"
-   "::std::map::count;"
-   "::std::map::find;"
-   "::std::map::lower_bound;"
-   "::std::multimap::equal_range;"
-   "::std::multimap::upper_bound;"
-   "::std::set::count;"
-   "::std::set::find;"
-   "::std::setfill;"
-   "::std::setprecision;"
-   "::std::setw;"
-   "::std::upper_bound;"
-   "::std::vector::at;"
-   // C standard library
-   "::bsearch;"
-   "::ferror;"
-   "::feof;"
-   "::isalnum;"
-   "::isalpha;"
-   "::isblank;"
-   "::iscntrl;"
-   "::isdigit;"
-   "::isgraph;"
-   "::islower;"
-   "::isprint;"
-   "::ispunct;"
-   "::isspace;"
-   "::isupper;"
-   "::iswalnum;"
-   "::iswprint;"
-   "::iswspace;"
-   "::isxdigit;"
-   "::memchr;"
-   "::memcmp;"
-   "::strcmp;"
-   "::strcoll;"
-   "::strncmp;"
-   "::strpbrk;"
-   "::strrchr;"
-   "::strspn;"
-   "::strstr;"
-   "::wcscmp;"
-   // POSIX
-   "::access;"
-   "::bind;"
-   "::connect;"
-   "::difftime;"
-   "::dlsym;"
-   "::fnmatch;"
-   "::getaddrinfo;"
-   "::getopt;"
-   "::htonl;"
-   "::htons;"
-   "::iconv_open;"
-   "::inet_addr;"
- 

[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)

2024-02-26 Thread Félix-Antoine Constantin via cfe-commits

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


[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)

2024-02-25 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/82947

From d1cbed0e2e83bd3544067fd25d7e811f1ab3f095 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 25 Feb 2024 20:20:59 -0500
Subject: [PATCH 1/2] [clang-tidy] Improved modernize-use-using by fixing a
 false-negative

The check needs a parent decl to match but if the typedef is in a function,
the parent is a declStmt which is not a decl by itself.
Improved the matcher to match on either a decl or a declstmt and extract
the decl from the stmt in the latter case.

fixes #72179
---
 .../clang-tidy/modernize/UseUsingCheck.cpp | 16 +---
 clang-tools-extra/docs/ReleaseNotes.rst|  3 +++
 .../checkers/modernize/use-using.cpp   | 18 ++
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
index bb05f206c717ce..50a07fc02e31b4 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -24,6 +24,7 @@ static constexpr llvm::StringLiteral ExternCDeclName = 
"extern-c-decl";
 static constexpr llvm::StringLiteral ParentDeclName = "parent-decl";
 static constexpr llvm::StringLiteral TagDeclName = "tag-decl";
 static constexpr llvm::StringLiteral TypedefName = "typedef";
+static constexpr llvm::StringLiteral DeclStmtName = "decl-stmt";
 
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
@@ -41,7 +42,8 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   unless(isInstantiated()),
   optionally(hasAncestor(
   linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))),
-  hasParent(decl().bind(ParentDeclName)))
+  anyOf(hasParent(decl().bind(ParentDeclName)),
+hasParent(declStmt().bind(DeclStmtName
   .bind(TypedefName),
   this);
 
@@ -51,17 +53,25 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   tagDecl(
   anyOf(allOf(unless(anyOf(isImplicit(),
classTemplateSpecializationDecl())),
-  hasParent(decl().bind(ParentDeclName))),
+  anyOf(hasParent(decl().bind(ParentDeclName)),
+hasParent(declStmt().bind(DeclStmtName,
 // We want the parent of the ClassTemplateDecl, not the parent
 // of the specialization.
 classTemplateSpecializationDecl(hasAncestor(classTemplateDecl(
-hasParent(decl().bind(ParentDeclName)))
+anyOf(hasParent(decl().bind(ParentDeclName)),
+  hasParent(declStmt().bind(DeclStmtName
   .bind(TagDeclName),
   this);
 }
 
 void UseUsingCheck::check(const MatchFinder::MatchResult ) {
   const auto *ParentDecl = Result.Nodes.getNodeAs(ParentDeclName);
+
+  if (!ParentDecl) {
+const auto *ParentDeclStmt = 
Result.Nodes.getNodeAs(DeclStmtName);
+ParentDecl = ParentDeclStmt->getSingleDecl();
+  }
+
   if (!ParentDecl)
 return;
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 6fd01ed9d471c5..2b36ae4acb9f1d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -183,6 +183,9 @@ Changes in existing checks
   ` check to also remove any trailing
   whitespace when deleting the ``virtual`` keyword.
 
+- Improved :doc:`modernize-use-using `
+  check by fixing false-negative in functions.
+
 - Improved :doc:`readability-implicit-bool-conversion
   ` check to provide
   valid fix suggestions for ``static_cast`` without a preceding space and
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
index 462bc984fd3ad5..230c7c94cda1cf 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
@@ -342,3 +342,21 @@ typedef int InExternCPP;
 // CHECK-FIXES: using InExternCPP = int;
 
 }
+
+namespace ISSUE_72179
+{  
+  void foo()
+  {
+typedef int a;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 
'typedef' [modernize-use-using]
+// CHECK-FIXES: using a = int;
+
+  }
+
+  void foo2()
+  {
+typedef struct { int a; union { int b; }; } c;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 
'typedef' [modernize-use-using]
+// CHECK-FIXES: using c = struct { int a; union { int b; }; };
+  }
+}

From 0b5c78508fbf49229d576171d7af6c6bb16b4367 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 25 Feb 2024 22:14:22 -0500
Subject: [PATCH 2/2] fixup! [clang-tidy] 

[clang-tools-extra] [clang-tidy] bugprone-unused-return-value config now supports regexes (PR #82952)

2024-02-25 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 created 
https://github.com/llvm/llvm-project/pull/82952

The parameter `CheckedReturnTypes` now supports regexes

Fixes #63107

From 86bc8a57fd56a6b2593af8bcf85d50bbc0ce984e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 25 Feb 2024 22:05:08 -0500
Subject: [PATCH] [clang-tidy] bugprone-unused-return-value config now support
 regexes

Fixes #63107
---
 .../bugprone/UnusedReturnValueCheck.cpp   | 202 +-
 .../bugprone/UnusedReturnValueCheck.h |   6 +-
 .../hicpp/IgnoredRemoveResultCheck.cpp|   8 +-
 clang-tools-extra/docs/ReleaseNotes.rst   |   4 +
 .../bugprone/unused-return-value-custom.cpp   |   2 +-
 5 files changed, 114 insertions(+), 108 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
index 05012c7df6a975..b4bf85c912c3ca 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -33,98 +33,98 @@ AST_MATCHER_P(FunctionDecl, isInstantiatedFrom, 
Matcher,
 UnusedReturnValueCheck::UnusedReturnValueCheck(llvm::StringRef Name,
ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  CheckedFunctions(Options.get("CheckedFunctions",
-   "::std::async;"
-   "::std::launder;"
-   "::std::remove;"
-   "::std::remove_if;"
-   "::std::unique;"
-   "::std::unique_ptr::release;"
-   "::std::basic_string::empty;"
-   "::std::vector::empty;"
-   "::std::back_inserter;"
-   "::std::distance;"
-   "::std::find;"
-   "::std::find_if;"
-   "::std::inserter;"
-   "::std::lower_bound;"
-   "::std::make_pair;"
-   "::std::map::count;"
-   "::std::map::find;"
-   "::std::map::lower_bound;"
-   "::std::multimap::equal_range;"
-   "::std::multimap::upper_bound;"
-   "::std::set::count;"
-   "::std::set::find;"
-   "::std::setfill;"
-   "::std::setprecision;"
-   "::std::setw;"
-   "::std::upper_bound;"
-   "::std::vector::at;"
-   // C standard library
-   "::bsearch;"
-   "::ferror;"
-   "::feof;"
-   "::isalnum;"
-   "::isalpha;"
-   "::isblank;"
-   "::iscntrl;"
-   "::isdigit;"
-   "::isgraph;"
-   "::islower;"
-   "::isprint;"
-   "::ispunct;"
-   "::isspace;"
-   "::isupper;"
-   "::iswalnum;"
-   "::iswprint;"
-   "::iswspace;"
-   "::isxdigit;"
-   "::memchr;"
-   "::memcmp;"
-   "::strcmp;"
-   "::strcoll;"
-   "::strncmp;"
-   "::strpbrk;"
-   "::strrchr;"
-   "::strspn;"
-   "::strstr;"
-   "::wcscmp;"
-   // POSIX
-   "::access;"
-   "::bind;"
-   "::connect;"
-   "::difftime;"
-   "::dlsym;"
-   "::fnmatch;"
-   "::getaddrinfo;"
-   "::getopt;"
-   "::htonl;"
-   "::htons;"
-   "::iconv_open;"
- 

[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)

2024-02-25 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/82947

From d1cbed0e2e83bd3544067fd25d7e811f1ab3f095 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 25 Feb 2024 20:20:59 -0500
Subject: [PATCH] [clang-tidy] Improved modernize-use-using by fixing a
 false-negative

The check needs a parent decl to match but if the typedef is in a function,
the parent is a declStmt which is not a decl by itself.
Improved the matcher to match on either a decl or a declstmt and extract
the decl from the stmt in the latter case.

fixes #72179
---
 .../clang-tidy/modernize/UseUsingCheck.cpp | 16 +---
 clang-tools-extra/docs/ReleaseNotes.rst|  3 +++
 .../checkers/modernize/use-using.cpp   | 18 ++
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
index bb05f206c717ce..50a07fc02e31b4 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -24,6 +24,7 @@ static constexpr llvm::StringLiteral ExternCDeclName = 
"extern-c-decl";
 static constexpr llvm::StringLiteral ParentDeclName = "parent-decl";
 static constexpr llvm::StringLiteral TagDeclName = "tag-decl";
 static constexpr llvm::StringLiteral TypedefName = "typedef";
+static constexpr llvm::StringLiteral DeclStmtName = "decl-stmt";
 
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
@@ -41,7 +42,8 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   unless(isInstantiated()),
   optionally(hasAncestor(
   linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))),
-  hasParent(decl().bind(ParentDeclName)))
+  anyOf(hasParent(decl().bind(ParentDeclName)),
+hasParent(declStmt().bind(DeclStmtName
   .bind(TypedefName),
   this);
 
@@ -51,17 +53,25 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   tagDecl(
   anyOf(allOf(unless(anyOf(isImplicit(),
classTemplateSpecializationDecl())),
-  hasParent(decl().bind(ParentDeclName))),
+  anyOf(hasParent(decl().bind(ParentDeclName)),
+hasParent(declStmt().bind(DeclStmtName,
 // We want the parent of the ClassTemplateDecl, not the parent
 // of the specialization.
 classTemplateSpecializationDecl(hasAncestor(classTemplateDecl(
-hasParent(decl().bind(ParentDeclName)))
+anyOf(hasParent(decl().bind(ParentDeclName)),
+  hasParent(declStmt().bind(DeclStmtName
   .bind(TagDeclName),
   this);
 }
 
 void UseUsingCheck::check(const MatchFinder::MatchResult ) {
   const auto *ParentDecl = Result.Nodes.getNodeAs(ParentDeclName);
+
+  if (!ParentDecl) {
+const auto *ParentDeclStmt = 
Result.Nodes.getNodeAs(DeclStmtName);
+ParentDecl = ParentDeclStmt->getSingleDecl();
+  }
+
   if (!ParentDecl)
 return;
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 6fd01ed9d471c5..2b36ae4acb9f1d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -183,6 +183,9 @@ Changes in existing checks
   ` check to also remove any trailing
   whitespace when deleting the ``virtual`` keyword.
 
+- Improved :doc:`modernize-use-using `
+  check by fixing false-negative in functions.
+
 - Improved :doc:`readability-implicit-bool-conversion
   ` check to provide
   valid fix suggestions for ``static_cast`` without a preceding space and
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
index 462bc984fd3ad5..230c7c94cda1cf 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
@@ -342,3 +342,21 @@ typedef int InExternCPP;
 // CHECK-FIXES: using InExternCPP = int;
 
 }
+
+namespace ISSUE_72179
+{  
+  void foo()
+  {
+typedef int a;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 
'typedef' [modernize-use-using]
+// CHECK-FIXES: using a = int;
+
+  }
+
+  void foo2()
+  {
+typedef struct { int a; union { int b; }; } c;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 
'typedef' [modernize-use-using]
+// CHECK-FIXES: using c = struct { int a; union { int b; }; };
+  }
+}

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


[clang-tools-extra] [clang-tidy] Improved modernize-use-using by fixing a false-negative (PR #82947)

2024-02-25 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 created 
https://github.com/llvm/llvm-project/pull/82947

The check needs a parent decl to match but if the typedef is in a function, the 
parent is a declStmt which is not a decl by itself. Improved the matcher to 
match on either a decl or a declstmt and extract the decl from the stmt in the 
latter case.

fixes #72179

CC: @FabianWolff @PiotrZSL @njames93 

From 003ec9d5b75cdf6de3140612e1c4b42322a52413 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 25 Feb 2024 20:20:59 -0500
Subject: [PATCH] [clang-tidy] Improved modernize-use-using by fixing a
 false-positive

The check needs a parent decl to match but if the typedef is in a function,
the parent is a declStmt which is not a decl by itself.
Improved the matcher to match on either a decl or a declstmt and extract
the decl from the stmt in the latter case.

fixes #72179
---
 .../clang-tidy/modernize/UseUsingCheck.cpp | 16 +---
 clang-tools-extra/docs/ReleaseNotes.rst|  3 +++
 .../checkers/modernize/use-using.cpp   | 18 ++
 3 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
index bb05f206c717ce..50a07fc02e31b4 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -24,6 +24,7 @@ static constexpr llvm::StringLiteral ExternCDeclName = 
"extern-c-decl";
 static constexpr llvm::StringLiteral ParentDeclName = "parent-decl";
 static constexpr llvm::StringLiteral TagDeclName = "tag-decl";
 static constexpr llvm::StringLiteral TypedefName = "typedef";
+static constexpr llvm::StringLiteral DeclStmtName = "decl-stmt";
 
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
@@ -41,7 +42,8 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   unless(isInstantiated()),
   optionally(hasAncestor(
   linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))),
-  hasParent(decl().bind(ParentDeclName)))
+  anyOf(hasParent(decl().bind(ParentDeclName)),
+hasParent(declStmt().bind(DeclStmtName
   .bind(TypedefName),
   this);
 
@@ -51,17 +53,25 @@ void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
   tagDecl(
   anyOf(allOf(unless(anyOf(isImplicit(),
classTemplateSpecializationDecl())),
-  hasParent(decl().bind(ParentDeclName))),
+  anyOf(hasParent(decl().bind(ParentDeclName)),
+hasParent(declStmt().bind(DeclStmtName,
 // We want the parent of the ClassTemplateDecl, not the parent
 // of the specialization.
 classTemplateSpecializationDecl(hasAncestor(classTemplateDecl(
-hasParent(decl().bind(ParentDeclName)))
+anyOf(hasParent(decl().bind(ParentDeclName)),
+  hasParent(declStmt().bind(DeclStmtName
   .bind(TagDeclName),
   this);
 }
 
 void UseUsingCheck::check(const MatchFinder::MatchResult ) {
   const auto *ParentDecl = Result.Nodes.getNodeAs(ParentDeclName);
+
+  if (!ParentDecl) {
+const auto *ParentDeclStmt = 
Result.Nodes.getNodeAs(DeclStmtName);
+ParentDecl = ParentDeclStmt->getSingleDecl();
+  }
+
   if (!ParentDecl)
 return;
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 6fd01ed9d471c5..2b36ae4acb9f1d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -183,6 +183,9 @@ Changes in existing checks
   ` check to also remove any trailing
   whitespace when deleting the ``virtual`` keyword.
 
+- Improved :doc:`modernize-use-using `
+  check by fixing false-negative in functions.
+
 - Improved :doc:`readability-implicit-bool-conversion
   ` check to provide
   valid fix suggestions for ``static_cast`` without a preceding space and
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
index 462bc984fd3ad5..230c7c94cda1cf 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
@@ -342,3 +342,21 @@ typedef int InExternCPP;
 // CHECK-FIXES: using InExternCPP = int;
 
 }
+
+namespace ISSUE_72179
+{  
+  void foo()
+  {
+typedef int a;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 
'typedef' [modernize-use-using]
+// CHECK-FIXES: using a = int;
+
+  }
+
+  void foo2()
+  {
+typedef struct { int a; union { int b; }; } c;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use 'using' instead of 
'typedef' 

[clang-tools-extra] [clang-tidy] Removed redundant-inline-specifier warning on static data members (PR #81423)

2024-02-11 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/81423

From 0fa56a4176205337270d15049dc34a8508488905 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Thu, 8 Feb 2024 17:07:38 -0500
Subject: [PATCH 1/2] =?UTF-8?q?[clang-tidy]=C2=A0Removed=20redundant-inlin?=
 =?UTF-8?q?e-specifier=20warning=20on=20static=20data=20members?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Updated the check to ignore point static data members with
in class initializer since removing the inline specifier would generate
a compilation error

Fixes #80684
---
 .../RedundantInlineSpecifierCheck.cpp   | 17 +++--
 clang-tools-extra/docs/ReleaseNotes.rst |  4 
 .../readability/redundant-inline-specifier.cpp  | 14 ++
 3 files changed, 29 insertions(+), 6 deletions(-)

diff --git 
a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
index 0e8d17d4442478..3d6b052686c20c 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
@@ -47,6 +47,9 @@ AST_POLYMORPHIC_MATCHER_P(isInternalLinkage,
 return VD->isInAnonymousNamespace();
   llvm_unreachable("Not a valid polymorphic type");
 }
+
+AST_MATCHER(clang::VarDecl, hasInitialization) { return Node.hasInit(); }
+
 } // namespace
 
 static SourceLocation getInlineTokenLocation(SourceRange RangeLocation,
@@ -88,12 +91,14 @@ void 
RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) {
 this);
 
   if (getLangOpts().CPlusPlus17) {
-Finder->addMatcher(
-varDecl(isInlineSpecified(),
-anyOf(isInternalLinkage(StrictMode),
-  allOf(isConstexpr(), hasAncestor(recordDecl()
-.bind("var_decl"),
-this);
+const auto IsPartOfRecordDecl = hasAncestor(recordDecl());
+Finder->addMatcher(varDecl(isInlineSpecified(),
+   anyOf(allOf(isInternalLinkage(StrictMode),
+   unless(allOf(hasInitialization(),
+IsPartOfRecordDecl))),
+ allOf(isConstexpr(), IsPartOfRecordDecl)))
+   .bind("var_decl"),
+   this);
   }
 }
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index ee68c8f49b3df2..303bfef17015a0 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -160,6 +160,10 @@ Changes in existing checks
   `AllowStringArrays` option, enabling the exclusion of array types with 
deduced
   length initialized from string literals.
 
+- Improved :doc:`readability-redundant-inline-specifier
+  ` check to properly
+  emit warnings for static data member with an in-class initializer.
+
 Removed checks
 ^^
 
diff --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
index cdd98d8fdc20f5..14f9e88f7e7218 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
@@ -135,3 +135,17 @@ INLINE_MACRO()
 
 #define INLINE_KW inline
 INLINE_KW void fn10() { }
+
+namespace {
+class A
+{
+public:
+  static inline float test = 3.0F;
+  static inline double test2 = 3.0;
+  static inline int test3 = 3;
+
+  static inline float test4;
+  // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:10: warning: variable 'test4' has 
inline specifier but is implicitly inlined 
[readability-redundant-inline-specifier]
+  // CHECK-FIXES-STRICT: static float test4;
+};
+}

From 3b4684a3003793e80d25e8a75c8252dbb5514cea Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 11 Feb 2024 21:02:59 -0500
Subject: [PATCH 2/2] =?UTF-8?q?fixup!=20[clang-tidy]=C2=A0Removed=20redund?=
 =?UTF-8?q?ant-inline-specifier=20warning=20on=20static=20data=20members?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Improved AST Matcher based on comments
---
 .../RedundantInlineSpecifierCheck.cpp | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git 
a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
index 3d6b052686c20c..1693e5c5e9cd45 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
@@ -47,9 +47,6 @@ 

[clang-tools-extra] [clang-tidy] Removed redundant-inline-specifier warning on static data members (PR #81423)

2024-02-11 Thread Félix-Antoine Constantin via cfe-commits

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


[clang-tools-extra] [clang-tidy] Removed redundant-inline-specifier warning on static data members (PR #81423)

2024-02-11 Thread Félix-Antoine Constantin via cfe-commits


@@ -47,6 +47,9 @@ AST_POLYMORPHIC_MATCHER_P(isInternalLinkage,
 return VD->isInAnonymousNamespace();
   llvm_unreachable("Not a valid polymorphic type");
 }
+
+AST_MATCHER(clang::VarDecl, hasInitialization) { return Node.hasInit(); }

felix642 wrote:

Simply didn't know this exists. I'll update my code with your suggestion. Thank 
you 

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


[clang-tools-extra] [clang-tidy] Removed redundant-inline-specifier warning on static data members (PR #81423)

2024-02-11 Thread Félix-Antoine Constantin via cfe-commits


@@ -88,12 +91,14 @@ void 
RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) {
 this);
 
   if (getLangOpts().CPlusPlus17) {
-Finder->addMatcher(
-varDecl(isInlineSpecified(),
-anyOf(isInternalLinkage(StrictMode),
-  allOf(isConstexpr(), hasAncestor(recordDecl()
-.bind("var_decl"),
-this);
+const auto IsPartOfRecordDecl = hasAncestor(recordDecl());
+Finder->addMatcher(varDecl(isInlineSpecified(),
+   anyOf(allOf(isInternalLinkage(StrictMode),
+   unless(allOf(hasInitialization(),

felix642 wrote:

I thought about this, but I can't find an example where a class member would 
have the inline keyword and not be static.
If you look at the test case I added, the static keyword is required for the 
code to compile otherwise you will get a compilation error saying that inline 
can only appear on non-local variable (Which makes sense tbf).

Do you have an example in mind where the inline keyword is used on non-static 
class member? 

godbolt: https://godbolt.org/z/aEGGvs9Kc

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


[clang-tools-extra] [clang-tidy] Removed redundant-inline-specifier warning on static data members (PR #81423)

2024-02-11 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/81423

From 0fa56a4176205337270d15049dc34a8508488905 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Thu, 8 Feb 2024 17:07:38 -0500
Subject: [PATCH] =?UTF-8?q?[clang-tidy]=C2=A0Removed=20redundant-inline-sp?=
 =?UTF-8?q?ecifier=20warning=20on=20static=20data=20members?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Updated the check to ignore point static data members with
in class initializer since removing the inline specifier would generate
a compilation error

Fixes #80684
---
 .../RedundantInlineSpecifierCheck.cpp   | 17 +++--
 clang-tools-extra/docs/ReleaseNotes.rst |  4 
 .../readability/redundant-inline-specifier.cpp  | 14 ++
 3 files changed, 29 insertions(+), 6 deletions(-)

diff --git 
a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
index 0e8d17d4442478..3d6b052686c20c 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
@@ -47,6 +47,9 @@ AST_POLYMORPHIC_MATCHER_P(isInternalLinkage,
 return VD->isInAnonymousNamespace();
   llvm_unreachable("Not a valid polymorphic type");
 }
+
+AST_MATCHER(clang::VarDecl, hasInitialization) { return Node.hasInit(); }
+
 } // namespace
 
 static SourceLocation getInlineTokenLocation(SourceRange RangeLocation,
@@ -88,12 +91,14 @@ void 
RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) {
 this);
 
   if (getLangOpts().CPlusPlus17) {
-Finder->addMatcher(
-varDecl(isInlineSpecified(),
-anyOf(isInternalLinkage(StrictMode),
-  allOf(isConstexpr(), hasAncestor(recordDecl()
-.bind("var_decl"),
-this);
+const auto IsPartOfRecordDecl = hasAncestor(recordDecl());
+Finder->addMatcher(varDecl(isInlineSpecified(),
+   anyOf(allOf(isInternalLinkage(StrictMode),
+   unless(allOf(hasInitialization(),
+IsPartOfRecordDecl))),
+ allOf(isConstexpr(), IsPartOfRecordDecl)))
+   .bind("var_decl"),
+   this);
   }
 }
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index ee68c8f49b3df2..303bfef17015a0 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -160,6 +160,10 @@ Changes in existing checks
   `AllowStringArrays` option, enabling the exclusion of array types with 
deduced
   length initialized from string literals.
 
+- Improved :doc:`readability-redundant-inline-specifier
+  ` check to properly
+  emit warnings for static data member with an in-class initializer.
+
 Removed checks
 ^^
 
diff --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
index cdd98d8fdc20f5..14f9e88f7e7218 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
@@ -135,3 +135,17 @@ INLINE_MACRO()
 
 #define INLINE_KW inline
 INLINE_KW void fn10() { }
+
+namespace {
+class A
+{
+public:
+  static inline float test = 3.0F;
+  static inline double test2 = 3.0;
+  static inline int test3 = 3;
+
+  static inline float test4;
+  // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:10: warning: variable 'test4' has 
inline specifier but is implicitly inlined 
[readability-redundant-inline-specifier]
+  // CHECK-FIXES-STRICT: static float test4;
+};
+}

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


[clang-tools-extra] [clang-tidy] Removed redundant-inline-specifier warning on static data members (PR #81423)

2024-02-11 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 created 
https://github.com/llvm/llvm-project/pull/81423

Updated the check to ignore point static data members with in class initializer 
since removing the inline specifier would generate a compilation error

Fixes #80684



From 45c89e269bd37e298dd12035dc78c986fa47e824 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Thu, 8 Feb 2024 17:07:38 -0500
Subject: [PATCH] =?UTF-8?q?[clang-tidy]=C2=A0Removed=20redundant-inline-sp?=
 =?UTF-8?q?ecifier=20warning=20on=20static=20data=20members?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Updated the check to ignore floating point static data members with
in class initializer since removing the inline specifier would generate
a compilation error

Fixes #80684
---
 .../RedundantInlineSpecifierCheck.cpp   | 17 +++--
 clang-tools-extra/docs/ReleaseNotes.rst |  4 
 .../readability/redundant-inline-specifier.cpp  | 15 +++
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git 
a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
index 0e8d17d4442478..3d6b052686c20c 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
@@ -47,6 +47,9 @@ AST_POLYMORPHIC_MATCHER_P(isInternalLinkage,
 return VD->isInAnonymousNamespace();
   llvm_unreachable("Not a valid polymorphic type");
 }
+
+AST_MATCHER(clang::VarDecl, hasInitialization) { return Node.hasInit(); }
+
 } // namespace
 
 static SourceLocation getInlineTokenLocation(SourceRange RangeLocation,
@@ -88,12 +91,14 @@ void 
RedundantInlineSpecifierCheck::registerMatchers(MatchFinder *Finder) {
 this);
 
   if (getLangOpts().CPlusPlus17) {
-Finder->addMatcher(
-varDecl(isInlineSpecified(),
-anyOf(isInternalLinkage(StrictMode),
-  allOf(isConstexpr(), hasAncestor(recordDecl()
-.bind("var_decl"),
-this);
+const auto IsPartOfRecordDecl = hasAncestor(recordDecl());
+Finder->addMatcher(varDecl(isInlineSpecified(),
+   anyOf(allOf(isInternalLinkage(StrictMode),
+   unless(allOf(hasInitialization(),
+IsPartOfRecordDecl))),
+ allOf(isConstexpr(), IsPartOfRecordDecl)))
+   .bind("var_decl"),
+   this);
   }
 }
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index ee68c8f49b3df2..303bfef17015a0 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -160,6 +160,10 @@ Changes in existing checks
   `AllowStringArrays` option, enabling the exclusion of array types with 
deduced
   length initialized from string literals.
 
+- Improved :doc:`readability-redundant-inline-specifier
+  ` check to properly
+  emit warnings for static data member with an in-class initializer.
+
 Removed checks
 ^^
 
diff --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
index cdd98d8fdc20f5..4c0e063c00e73f 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp
@@ -135,3 +135,18 @@ INLINE_MACRO()
 
 #define INLINE_KW inline
 INLINE_KW void fn10() { }
+
+namespace {
+class A
+{
+public:
+  static inline float test = 3.0F;
+  static inline double test2 = 3.0;
+  static inline int test3 = 3;
+
+  static inline float test4;
+  // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:10: warning: variable 'test4' has 
inline specifier but is implicitly inlined 
[readability-redundant-inline-specifier]
+  // CHECK-FIXES-STRICT: static float test4;
+};
+}
+

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


[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-02-04 Thread Félix-Antoine Constantin via cfe-commits

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

LGTM,

There is still some work to do to support more cases like ternary operators and 
if/else statements, but I think we have a decent foundation to work on and we 
can add these cases later in other PRs.

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


[clang-tools-extra] [llvm] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-27 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,189 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include 
+using namespace std;
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+class ExprVisitor : public clang::RecursiveASTVisitor {
+public:
+  explicit ExprVisitor(clang::ASTContext *Context) : Context(Context) {}
+  bool visitStmt(const clang::Stmt *S, bool ,
+ clang::QualType ) {
+
+if (isa(S) && !found) {
+  found = true;
+  const clang::ImplicitCastExpr *ImplicitCast =
+  cast(S);
+  GlobalImplicitCastType = ImplicitCast->getType();
+  // Stop visiting children.
+  return false;
+}
+// Continue visiting children.
+for (const clang::Stmt *Child : S->children()) {
+  if (Child) {
+this->visitStmt(Child, found, GlobalImplicitCastType);
+  }
+}
+
+return true; // Continue visiting other nodes.
+  }
+
+private:
+  clang::ASTContext *Context;

felix642 wrote:

I can see that, but you're not using that variable anywhere. What's the point 
of declaring it if you're only initializing it and not using it?

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


[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-27 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,189 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include 
+using namespace std;
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+class ExprVisitor : public clang::RecursiveASTVisitor {
+public:
+  explicit ExprVisitor(clang::ASTContext *Context) : Context(Context) {}
+  bool visitStmt(const clang::Stmt *S, bool ,
+ clang::QualType ) {
+
+if (isa(S) && !found) {
+  found = true;
+  const clang::ImplicitCastExpr *ImplicitCast =
+  cast(S);
+  GlobalImplicitCastType = ImplicitCast->getType();
+  // Stop visiting children.
+  return false;
+}
+// Continue visiting children.
+for (const clang::Stmt *Child : S->children()) {
+  if (Child) {
+this->visitStmt(Child, found, GlobalImplicitCastType);
+  }
+}
+
+return true; // Continue visiting other nodes.
+  }
+
+private:
+  clang::ASTContext *Context;

felix642 wrote:

Would you be able to tell me where are you using this ASTContext variable? I'm 
not seeing any usage and I feel like I'm missing something

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


[clang] [clang-tools-extra] [llvm] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-27 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,238 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+static bool isImplicitCastType(const clang::CastKind castKind) {
+  switch (castKind) {
+  case clang::CK_CPointerToObjCPointerCast:
+  case clang::CK_BlockPointerToObjCPointerCast:
+  case clang::CK_BitCast:
+  case clang::CK_AnyPointerToBlockPointerCast:
+  case clang::CK_NullToMemberPointer:
+  case clang::CK_NullToPointer:
+  case clang::CK_IntegralToPointer:
+  case clang::CK_PointerToIntegral:
+  case clang::CK_IntegralCast:
+  case clang::CK_BooleanToSignedIntegral:
+  case clang::CK_IntegralToFloating:
+  case clang::CK_FloatingToIntegral:
+  case clang::CK_FloatingCast:
+  case clang::CK_ObjCObjectLValueCast:
+  case clang::CK_FloatingRealToComplex:
+  case clang::CK_FloatingComplexToReal:
+  case clang::CK_FloatingComplexCast:
+  case clang::CK_FloatingComplexToIntegralComplex:
+  case clang::CK_IntegralRealToComplex:
+  case clang::CK_IntegralComplexToReal:
+  case clang::CK_IntegralComplexCast:
+  case clang::CK_IntegralComplexToFloatingComplex:
+  case clang::CK_FloatingToFixedPoint:
+  case clang::CK_FixedPointToFloating:
+  case clang::CK_FixedPointCast:
+  case clang::CK_FixedPointToIntegral:
+  case clang::CK_IntegralToFixedPoint:
+  case clang::CK_MatrixCast:
+  case clang::CK_PointerToBoolean:
+  case clang::CK_IntegralToBoolean:
+  case clang::CK_FloatingToBoolean:
+  case clang::CK_MemberPointerToBoolean:
+  case clang::CK_FloatingComplexToBoolean:
+  case clang::CK_IntegralComplexToBoolean:
+return true;
+  default:
+return false;
+  }
+}
+
+class ExprVisitor : public clang::RecursiveASTVisitor {
+public:
+  explicit ExprVisitor(clang::ASTContext *Context) : Context(Context) {}
+  bool visitStmt(const clang::Stmt *S, bool ,
+ clang::QualType ) {
+
+if (isa(S) && !found) {
+  const auto CastKind = cast(S)->getCastKind();
+  if (isImplicitCastType(CastKind)) {
+found = true;
+const clang::ImplicitCastExpr *ImplicitCast =
+cast(S);
+GlobalImplicitCastType = ImplicitCast->getType();
+// Stop visiting children.
+return false;
+  }
+}
+// Continue visiting children.
+for (const clang::Stmt *Child : S->children()) {
+  if (Child) {
+this->visitStmt(Child, found, GlobalImplicitCastType);
+  }
+}
+
+return true; // Continue visiting other nodes.
+  }
+
+private:
+  clang::ASTContext *Context;
+};
+
+static const llvm::StringRef AlgorithmHeader("");
+
+static bool minCondition(const BinaryOperator::Opcode Op, const Expr *CondLhs,
+ const Expr *CondRhs, const Expr *AssignLhs,
+ const Expr *AssignRhs, const ASTContext ) {
+  if ((Op == BO_LT || Op == BO_LE) &&
+  (tidy::utils::areStatementsIdentical(CondLhs, AssignRhs, Context) &&
+   tidy::utils::areStatementsIdentical(CondRhs, AssignLhs, Context)))
+return true;
+
+  if ((Op == BO_GT || Op == BO_GE) &&
+  (tidy::utils::areStatementsIdentical(CondLhs, AssignLhs, Context) &&
+   tidy::utils::areStatementsIdentical(CondRhs, AssignRhs, Context)))
+return true;
+
+  return false;
+}
+
+static bool maxCondition(const BinaryOperator::Opcode Op, const Expr *CondLhs,
+ const Expr *CondRhs, const Expr *AssignLhs,
+ const Expr *AssignRhs, const ASTContext ) {
+  if ((Op == BO_LT || Op == BO_LE) &&
+  (tidy::utils::areStatementsIdentical(CondLhs, AssignLhs, Context) &&
+   tidy::utils::areStatementsIdentical(CondRhs, AssignRhs, Context)))
+return true;
+
+  if ((Op == BO_GT || Op == BO_GE) &&
+  (tidy::utils::areStatementsIdentical(CondLhs, AssignRhs, Context) &&
+   tidy::utils::areStatementsIdentical(CondRhs, AssignLhs, Context)))
+return true;
+
+  return false;
+}
+
+static std::string
+createReplacement(const BinaryOperator::Opcode Op, const Expr *CondLhs,
+  const Expr *CondRhs, const Expr *AssignLhs,
+  const ASTContext , const SourceManager ,
+  const LangOptions , StringRef FunctionName,
+  QualType GlobalImplicitCastType) {
+  const llvm::StringRef CondLhsStr = Lexer::getSourceText(
+  Source.getExpansionRange(CondLhs->getSourceRange()), Source, LO);
+  const llvm::StringRef CondRhsStr = 

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-27 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 requested changes to this pull request.


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


[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-27 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,238 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+static bool isImplicitCastType(const clang::CastKind castKind) {
+  switch (castKind) {
+  case clang::CK_CPointerToObjCPointerCast:
+  case clang::CK_BlockPointerToObjCPointerCast:
+  case clang::CK_BitCast:
+  case clang::CK_AnyPointerToBlockPointerCast:
+  case clang::CK_NullToMemberPointer:
+  case clang::CK_NullToPointer:
+  case clang::CK_IntegralToPointer:
+  case clang::CK_PointerToIntegral:
+  case clang::CK_IntegralCast:
+  case clang::CK_BooleanToSignedIntegral:
+  case clang::CK_IntegralToFloating:
+  case clang::CK_FloatingToIntegral:
+  case clang::CK_FloatingCast:
+  case clang::CK_ObjCObjectLValueCast:
+  case clang::CK_FloatingRealToComplex:
+  case clang::CK_FloatingComplexToReal:
+  case clang::CK_FloatingComplexCast:
+  case clang::CK_FloatingComplexToIntegralComplex:
+  case clang::CK_IntegralRealToComplex:
+  case clang::CK_IntegralComplexToReal:
+  case clang::CK_IntegralComplexCast:
+  case clang::CK_IntegralComplexToFloatingComplex:
+  case clang::CK_FloatingToFixedPoint:
+  case clang::CK_FixedPointToFloating:
+  case clang::CK_FixedPointCast:
+  case clang::CK_FixedPointToIntegral:
+  case clang::CK_IntegralToFixedPoint:
+  case clang::CK_MatrixCast:
+  case clang::CK_PointerToBoolean:
+  case clang::CK_IntegralToBoolean:
+  case clang::CK_FloatingToBoolean:
+  case clang::CK_MemberPointerToBoolean:
+  case clang::CK_FloatingComplexToBoolean:
+  case clang::CK_IntegralComplexToBoolean:
+return true;
+  default:
+return false;
+  }
+}
+
+class ExprVisitor : public clang::RecursiveASTVisitor {
+public:
+  explicit ExprVisitor(clang::ASTContext *Context) : Context(Context) {}
+  bool visitStmt(const clang::Stmt *S, bool ,
+ clang::QualType ) {
+
+if (isa(S) && !found) {
+  const auto CastKind = cast(S)->getCastKind();
+  if (isImplicitCastType(CastKind)) {
+found = true;
+const clang::ImplicitCastExpr *ImplicitCast =
+cast(S);
+GlobalImplicitCastType = ImplicitCast->getType();
+// Stop visiting children.
+return false;
+  }
+}
+// Continue visiting children.
+for (const clang::Stmt *Child : S->children()) {
+  if (Child) {
+this->visitStmt(Child, found, GlobalImplicitCastType);
+  }
+}
+
+return true; // Continue visiting other nodes.
+  }
+
+private:
+  clang::ASTContext *Context;
+};
+
+static const llvm::StringRef AlgorithmHeader("");
+
+static bool minCondition(const BinaryOperator::Opcode Op, const Expr *CondLhs,
+ const Expr *CondRhs, const Expr *AssignLhs,
+ const Expr *AssignRhs, const ASTContext ) {
+  if ((Op == BO_LT || Op == BO_LE) &&
+  (tidy::utils::areStatementsIdentical(CondLhs, AssignRhs, Context) &&
+   tidy::utils::areStatementsIdentical(CondRhs, AssignLhs, Context)))
+return true;
+
+  if ((Op == BO_GT || Op == BO_GE) &&
+  (tidy::utils::areStatementsIdentical(CondLhs, AssignLhs, Context) &&
+   tidy::utils::areStatementsIdentical(CondRhs, AssignRhs, Context)))
+return true;
+
+  return false;
+}
+
+static bool maxCondition(const BinaryOperator::Opcode Op, const Expr *CondLhs,
+ const Expr *CondRhs, const Expr *AssignLhs,
+ const Expr *AssignRhs, const ASTContext ) {
+  if ((Op == BO_LT || Op == BO_LE) &&
+  (tidy::utils::areStatementsIdentical(CondLhs, AssignLhs, Context) &&
+   tidy::utils::areStatementsIdentical(CondRhs, AssignRhs, Context)))
+return true;
+
+  if ((Op == BO_GT || Op == BO_GE) &&
+  (tidy::utils::areStatementsIdentical(CondLhs, AssignRhs, Context) &&
+   tidy::utils::areStatementsIdentical(CondRhs, AssignLhs, Context)))
+return true;
+
+  return false;
+}
+
+static std::string
+createReplacement(const BinaryOperator::Opcode Op, const Expr *CondLhs,
+  const Expr *CondRhs, const Expr *AssignLhs,
+  const ASTContext , const SourceManager ,
+  const LangOptions , StringRef FunctionName,
+  QualType GlobalImplicitCastType) {
+  const llvm::StringRef CondLhsStr = Lexer::getSourceText(
+  Source.getExpansionRange(CondLhs->getSourceRange()), Source, LO);
+  const llvm::StringRef CondRhsStr = 

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-27 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,195 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include 
+using namespace std;
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+class ExprVisitor : public clang::RecursiveASTVisitor {
+public:
+  explicit ExprVisitor(clang::ASTContext *Context) : Context(Context) {}
+  bool visitStmt(const clang::Stmt *S, bool ,
+ clang::QualType ) {
+
+if (isa(S) && !found) {

felix642 wrote:

You are missing `CK_UserDefinedConversion`

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


[clang] [llvm] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-27 Thread Félix-Antoine Constantin via cfe-commits

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


[clang] [clang-tools-extra] [llvm] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-26 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,195 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include 

felix642 wrote:

Also, do not include iostream

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


[clang] [clang-tools-extra] [llvm] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-26 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 requested changes to this pull request.


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


[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-26 Thread Félix-Antoine Constantin via cfe-commits

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


[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-26 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,195 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include 
+using namespace std;
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+class ExprVisitor : public clang::RecursiveASTVisitor {
+public:
+  explicit ExprVisitor(clang::ASTContext *Context) : Context(Context) {}
+  bool visitStmt(const clang::Stmt *S, bool ,
+ clang::QualType ) {
+
+if (isa(S) && !found) {

felix642 wrote:

You might want to (ignore/filter) some implicit cast types otherwise code like 
this one might give you some unexpected results:

```
#include 

class Foo
{};

int bar()
{
std::vector> a;
unsigned int b = a[0].size();
if(a[0].size() > b)
b = a[0].size();
}
```
https://godbolt.org/z/M67o5hxrG

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


[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-25 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 requested changes to this pull request.

Added a few more comments regarding some nits but we're almost there

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


[clang-tools-extra] [clang] [llvm] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-23 Thread Félix-Antoine Constantin via cfe-commits

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


[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-23 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 requested changes to this pull request.

Looks good! I was able run your code on llvm's project and I'm happy with the 
changes that I see. There is maybe one more use case that we should check 
before I am ready to approve (Except what Piotr has already highlighted) 

The following code generates an invalid fix :

```
#include 

int main()
{
  std::vector a;
  int b;
  if(a[0] < b)
  {
a[0] = b;
  }
```
```
#include 

int main()
{
  std::vector a;
  int b;
  a[0] = std::max(a[0], b);
}
```
The type of `a` and `b` are considered different by your check. `a` is resolved 
has value_type instead of int. 

If we are able to fix that I would be ready accept this PR!

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


[clang-tools-extra] [clang] [llvm] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-23 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,137 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+static const llvm::StringRef AlgorithmHeader("");
+
+UseStdMinMaxCheck::UseStdMinMaxCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+   utils::IncludeSorter::IS_LLVM),
+  areDiagsSelfContained()) {}
+
+void UseStdMinMaxCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
+}
+
+void UseStdMinMaxCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+  ifStmt(
+  stmt().bind("if"),
+  hasCondition(binaryOperator(hasAnyOperatorName("<", ">", "<=", ">="),
+  hasLHS(expr().bind("CondLhs")),
+  hasRHS(expr().bind("CondRhs",
+  hasThen(anyOf(stmt(binaryOperator(hasOperatorName("="),
+hasLHS(expr().bind("AssignLhs")),
+hasRHS(expr().bind("AssignRhs",
+compoundStmt(statementCountIs(1),
+ has(binaryOperator(

felix642 wrote:

The binaryOperator AST matcher seems to be present twice. Might be a good idea 
to extract it into a variable ? 

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


[clang] [clang-tools-extra] [llvm] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-23 Thread Félix-Antoine Constantin via cfe-commits

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


[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-23 Thread Félix-Antoine Constantin via cfe-commits

felix642 wrote:

 @11happy I was testing with `d5b8dc25598` but this seems to be working 
properly now. I would assume that this has been fixed in your latest commits

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


[clang-tools-extra] [clang-tidy] Ignore user-defined literals in google-runtime-int (PR #78859)

2024-01-22 Thread Félix-Antoine Constantin via cfe-commits

felix642 wrote:

@PiotrZSL would you be able to commit those changes for me please  ? 

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


[clang-tools-extra] [llvm] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-22 Thread Félix-Antoine Constantin via cfe-commits

felix642 wrote:

@11happy I've compiled your code to test it on llvm's project and clang-tidy 
seems to sometimes crash.  Would you be able to have a look before we merge 
this PR ?
```
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and 
include the crash backtrace.
Stack dump:
0.  Program arguments: /home/felix/git/llvm-project/build/bin/clang-tidy 
-checks=-*,readability-use-std-* -export-fixes 
/tmp/tmpswi2gkux/tmpbofvt76t.yaml -p=build 
/home/felix/git/llvm-project/llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
1.   parser at end of file
2.  ASTMatcher: Processing 'readability-use-std-min-max' against:
IfStmt : 

--- Bound Nodes Begin ---
AssignLhs - { DeclRefExpr : 

 }
AssignRhs - { CXXBoolLiteralExpr : 

 }
CondLhs - { CXXMemberCallExpr : 
 }
CondRhs - { IntegerLiteral : 

 }
if - { IfStmt : 
 }
--- Bound Nodes End ---
 #0 0x04208bab llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(/home/felix/git/llvm-project/build/bin/clang-tidy+0x4208bab)
 #1 0x042062bb SignalHandler(int) Signals.cpp:0:0
 #2 0x7f0a8f4e7420 __restore_rt 
(/lib/x86_64-linux-gnu/libpthread.so.0+0x14420)
 #3 0x0112e7e9 
clang::tidy::readability::UseStdMinMaxCheck::check(clang::ast_matchers::MatchFinder::MatchResult
 const&) (/home/felix/git/llvm-project/build/bin/clang-tidy+0x112e7e9)
 #4 0x02f5ef43 clang::ast_matchers::internal::(anonymous 
namespace)::MatchASTVisitor::MatchVisitor::visitMatch(clang::ast_matchers::BoundNodes
 const&) ASTMatchFinder.cpp:0:0
 #5 0x02f89f21 
clang::ast_matchers::internal::BoundNodesTreeBuilder::visitMatches(clang::ast_matchers::internal::BoundNodesTreeBuilder::Visitor*)
 (/home/felix/git/llvm-project/build/bin/clang-tidy+0x2f89f21)
 #6 0x02f60b6e clang::ast_matchers::internal::(anonymous 
namespace)::MatchASTVisitor::matchWithFilter(clang::DynTypedNode const&) 
ASTMatchFinder.cpp:0:0
 #7 0x02f84b10 
clang::RecursiveASTVisitor::TraverseMSPropertySubscriptExpr(clang::MSPropertySubscriptExpr*,
 llvm::SmallVectorImpl, 
llvm::PointerIntPairInfo>>>*) ASTMatchFinder.cpp:0:0
 #8 0x02f7aaf8 
clang::RecursiveASTVisitor::TraverseStmt(clang::Stmt*, 
llvm::SmallVectorImpl, 
llvm::PointerIntPairInfo>>>*) ASTMatchFinder.cpp:0:0
 #9 0x02f7ad0b clang::ast_matchers::internal::(anonymous 
namespace)::MatchASTVisitor::TraverseStmt(clang::Stmt*, 
llvm::SmallVectorImpl, 
llvm::PointerIntPairInfo>>>*) (.constprop.0) 
ASTMatchFinder.cpp:0:0
#10 0x02f7ec42 
clang::RecursiveASTVisitor::TraverseFunctionHelper(clang::FunctionDecl*) 
ASTMatchFinder.cpp:0:0
```

This is the bit of code that your check doesn't seems to like :
```
bool NeedExt = false;
if(EltVT.getSizeInBits() < 16)
NeedExt = true;
```

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


[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-21 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,175 @@
+// RUN: %check_clang_tidy %s readability-use-std-min-max %t
+
+#define MY_MACRO_MIN(a, b) ((a) < (b) ? (a) : (b))
+
+constexpr int myConstexprMin(int a, int b) {
+  return a < b ? a : b;
+}
+
+constexpr int myConstexprMax(int a, int b) {
+  return a > b ? a : b;
+}
+
+#define MY_IF_MACRO(condition, statement) \
+  if (condition) {\
+statement \
+  }
+
+class MyClass {
+public:
+  int member1;
+  int member2;
+};
+
+void foo() {
+  int value1,value2,value3;
+  short value4;
+  MyClass obj;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value2);
+  if (value1 < value2)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value1, value2);
+  if (value1 < value2)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value2, value1);
+  if (value2 > value1)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` 
[readability-use-std-min-max
+  // CHECK-FIXES: value1 = std::max(value2, value1);
+  if (value2 > value1)
+value1 = value2; 
+
+  // No suggestion needed here
+  if (value1 == value2)
+value1 = value2;
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value4);
+  if(value1` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::min(value1, myConstexprMax(value2, value3));
+  if (value1 > myConstexprMax(value2, value3))
+value1 = myConstexprMax(value2, value3); 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value1, value2);
+  if (value1 <= value2)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value2);
+  if (value1 <= value2)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value2, value1);
+  if (value2 >= value1)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value2, value1);
+  if (value2 >= value1)
+value2 = value1; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member1, obj.member2);
+  if (obj.member1 < obj.member2)
+obj.member1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member1, obj.member2);
+  if (obj.member1 < obj.member2)
+obj.member2 = obj.member1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member2, obj.member1);
+  if (obj.member2 > obj.member1)
+obj.member2 = obj.member1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member2, obj.member1);
+  if (obj.member2 > obj.member1)
+obj.member1 = obj.member2; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member1, value4);
+  if (obj.member1 < value4)
+obj.member1 = value4; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value3 = std::min(obj.member1 + value2, value3);
+  if (obj.member1 + value2 < value3)
+value3 = obj.member1 + value2; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(value1, obj.member2);
+  if (value1 <= obj.member2)
+obj.member2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, obj.member2);
+  if (value1 <= obj.member2)
+value1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(obj.member2, value1);
+  if (obj.member2 >= value1)
+value1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` 

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-21 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,175 @@
+// RUN: %check_clang_tidy %s readability-use-std-min-max %t
+
+#define MY_MACRO_MIN(a, b) ((a) < (b) ? (a) : (b))
+
+constexpr int myConstexprMin(int a, int b) {
+  return a < b ? a : b;
+}
+
+constexpr int myConstexprMax(int a, int b) {
+  return a > b ? a : b;
+}
+
+#define MY_IF_MACRO(condition, statement) \
+  if (condition) {\
+statement \
+  }
+
+class MyClass {
+public:
+  int member1;
+  int member2;
+};
+
+void foo() {
+  int value1,value2,value3;
+  short value4;
+  MyClass obj;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value2);
+  if (value1 < value2)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value1, value2);
+  if (value1 < value2)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value2, value1);
+  if (value2 > value1)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` 
[readability-use-std-min-max
+  // CHECK-FIXES: value1 = std::max(value2, value1);
+  if (value2 > value1)
+value1 = value2; 
+
+  // No suggestion needed here
+  if (value1 == value2)
+value1 = value2;
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value4);
+  if(value1` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::min(value1, myConstexprMax(value2, value3));
+  if (value1 > myConstexprMax(value2, value3))
+value1 = myConstexprMax(value2, value3); 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value1, value2);
+  if (value1 <= value2)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value2);
+  if (value1 <= value2)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value2, value1);
+  if (value2 >= value1)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value2, value1);
+  if (value2 >= value1)
+value2 = value1; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member1, obj.member2);
+  if (obj.member1 < obj.member2)
+obj.member1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member1, obj.member2);
+  if (obj.member1 < obj.member2)
+obj.member2 = obj.member1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member2, obj.member1);
+  if (obj.member2 > obj.member1)
+obj.member2 = obj.member1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member2, obj.member1);
+  if (obj.member2 > obj.member1)
+obj.member1 = obj.member2; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member1, value4);
+  if (obj.member1 < value4)
+obj.member1 = value4; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value3 = std::min(obj.member1 + value2, value3);
+  if (obj.member1 + value2 < value3)
+value3 = obj.member1 + value2; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(value1, obj.member2);
+  if (value1 <= obj.member2)
+obj.member2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, obj.member2);
+  if (value1 <= obj.member2)
+value1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(obj.member2, value1);
+  if (obj.member2 >= value1)
+value1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` 

[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-21 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 requested changes to this pull request.


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


[clang-tools-extra] [llvm] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-21 Thread Félix-Antoine Constantin via cfe-commits

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


[clang-tools-extra] [clang-tidy] Ignore user-defined literals in google-runtime-int (PR #78859)

2024-01-21 Thread Félix-Antoine Constantin via cfe-commits

felix642 wrote:

Rebased with main to fix merge conflicts.

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


[clang-tools-extra] [clang-tidy] Ignore user-defined literals in google-runtime-int (PR #78859)

2024-01-21 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/78859

From 57cbce14d5836e57f677fa1fef7214dd2663262a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sat, 20 Jan 2024 15:29:06 -0500
Subject: [PATCH 1/2] [clang-tidy] Ignore user-defined literals in
 google-runtime-int

User-defined literals do not accept u?intXX(_t)? variables.
So the check should not emit a warning.

Fixes #54546 #25214
---
 .../clang-tidy/google/IntegerTypesCheck.cpp | 13 -
 clang-tools-extra/docs/ReleaseNotes.rst |  3 +++
 .../test/clang-tidy/checkers/google/runtime-int.cpp | 10 ++
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp 
b/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
index bb4e1de8cc4b15..b60f2d0ed63112 100644
--- a/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -56,11 +56,14 @@ void IntegerTypesCheck::registerMatchers(MatchFinder 
*Finder) {
   // http://google.github.io/styleguide/cppguide.html#64-bit_Portability
   // "Where possible, avoid passing arguments of types specified by
   // bitwidth typedefs to printf-based APIs."
-  Finder->addMatcher(typeLoc(loc(isInteger()),
- unless(hasAncestor(callExpr(
- 
callee(functionDecl(hasAttr(attr::Format)))
- .bind("tl"),
- this);
+  Finder->addMatcher(
+  typeLoc(loc(isInteger()),
+  unless(anyOf(hasAncestor(callExpr(
+   callee(functionDecl(hasAttr(attr::Format),
+   hasParent(parmVarDecl(hasAncestor(functionDecl(
+   matchesName("operator\"\".*$"
+  .bind("tl"),
+  this);
   IdentTable = std::make_unique(getLangOpts());
 }
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 2822b17b435143..0706a1f8d152d7 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -374,6 +374,9 @@ Changes in existing checks
   ` check to ignore constructor
   calls disguised as functional casts.
 
+- Improved :doc:`google-runtime-int` check to ignore
+  false positives on user defined-literals.
+
 - Improved :doc:`llvm-namespace-comment
   ` check to provide fixes for
   ``inline`` namespaces in the same format as :program:`clang-format`.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/runtime-int.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/google/runtime-int.cpp
index 4bb739876b7f4c..88f0b519e9cbee 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/google/runtime-int.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/google/runtime-int.cpp
@@ -59,11 +59,13 @@ void qux() {
 // CHECK-MESSAGES: [[@LINE-1]]:3: warning: consider replacing 'short' with 
'int16'
 }
 
-// FIXME: This shouldn't warn, as UD-literal operators require one of a handful
-// of types as an argument.
 struct some_value {};
-constexpr some_value operator"" _some_literal(unsigned long long int i);
-// CHECK-MESSAGES: [[@LINE-1]]:47: warning: consider replacing 'unsigned long 
long'
+constexpr some_value operator"" _some_literal(unsigned long long int i)
+{
+  short j;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: consider replacing 'short' with 
'int16'
+  return some_value();
+}
 
 struct A { A& operator=(const A&); };
 class B { A a[0]; };

From 5495d864bc351e2c888a32cb10f16f935799470f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sat, 20 Jan 2024 21:02:42 -0500
Subject: [PATCH 2/2] fixup! [clang-tidy] Ignore user-defined literals in
 google-runtime-int

Fixed Documentation
Improved ast matcher to use getLiteralIdentifier
---
 .../clang-tidy/google/IntegerTypesCheck.cpp| 10 --
 clang-tools-extra/docs/ReleaseNotes.rst|  4 ++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp 
b/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
index b60f2d0ed63112..ef511e9108f2ee 100644
--- a/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -36,6 +36,12 @@ static Token getTokenAtLoc(SourceLocation Loc,
   return Tok;
 }
 
+namespace {
+AST_MATCHER(FunctionDecl, isUserDefineLiteral) {
+  return Node.getLiteralIdentifier() != nullptr;
+}
+} // namespace
+
 namespace tidy::google::runtime {
 
 IntegerTypesCheck::IntegerTypesCheck(StringRef Name, ClangTidyContext *Context)
@@ -60,8 +66,8 @@ void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) 
{
   typeLoc(loc(isInteger()),
   unless(anyOf(hasAncestor(callExpr(
callee(functionDecl(hasAttr(attr::Format),

[clang-tools-extra] [clang-tidy] Ignore user-defined literals in google-runtime-int (PR #78859)

2024-01-20 Thread Félix-Antoine Constantin via cfe-commits


@@ -56,11 +56,14 @@ void IntegerTypesCheck::registerMatchers(MatchFinder 
*Finder) {
   // http://google.github.io/styleguide/cppguide.html#64-bit_Portability
   // "Where possible, avoid passing arguments of types specified by
   // bitwidth typedefs to printf-based APIs."
-  Finder->addMatcher(typeLoc(loc(isInteger()),
- unless(hasAncestor(callExpr(
- 
callee(functionDecl(hasAttr(attr::Format)))
- .bind("tl"),
- this);
+  Finder->addMatcher(
+  typeLoc(loc(isInteger()),
+  unless(anyOf(hasAncestor(callExpr(
+   callee(functionDecl(hasAttr(attr::Format),
+   hasParent(parmVarDecl(hasAncestor(functionDecl(
+   matchesName("operator\"\".*$"

felix642 wrote:

Thank you, didn't think about this 

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


[clang-tools-extra] [clang-tidy] Ignore user-defined literals in google-runtime-int (PR #78859)

2024-01-20 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/78859

From 8ea205ae2324d67f9adaf30d5662d2cde2096534 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sat, 20 Jan 2024 15:29:06 -0500
Subject: [PATCH 1/2] [clang-tidy] Ignore user-defined literals in
 google-runtime-int

User-defined literals do not accept u?intXX(_t)? variables.
So the check should not emit a warning.

Fixes #54546 #25214
---
 .../clang-tidy/google/IntegerTypesCheck.cpp | 13 -
 clang-tools-extra/docs/ReleaseNotes.rst |  3 +++
 .../test/clang-tidy/checkers/google/runtime-int.cpp | 10 ++
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp 
b/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
index bb4e1de8cc4b15..b60f2d0ed63112 100644
--- a/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -56,11 +56,14 @@ void IntegerTypesCheck::registerMatchers(MatchFinder 
*Finder) {
   // http://google.github.io/styleguide/cppguide.html#64-bit_Portability
   // "Where possible, avoid passing arguments of types specified by
   // bitwidth typedefs to printf-based APIs."
-  Finder->addMatcher(typeLoc(loc(isInteger()),
- unless(hasAncestor(callExpr(
- 
callee(functionDecl(hasAttr(attr::Format)))
- .bind("tl"),
- this);
+  Finder->addMatcher(
+  typeLoc(loc(isInteger()),
+  unless(anyOf(hasAncestor(callExpr(
+   callee(functionDecl(hasAttr(attr::Format),
+   hasParent(parmVarDecl(hasAncestor(functionDecl(
+   matchesName("operator\"\".*$"
+  .bind("tl"),
+  this);
   IdentTable = std::make_unique(getLangOpts());
 }
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index a235a7d02592e8..5c857824c2b021 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -359,6 +359,9 @@ Changes in existing checks
   to ignore unused parameters when they are marked as unused and parameters of
   deleted functions and constructors.
 
+- Improved :doc:`google-runtime-int` check to ignore
+  false positives on user defined-literals.
+
 - Improved :doc:`llvm-namespace-comment
   ` check to provide fixes for
   ``inline`` namespaces in the same format as :program:`clang-format`.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/runtime-int.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/google/runtime-int.cpp
index 4bb739876b7f4c..88f0b519e9cbee 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/google/runtime-int.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/google/runtime-int.cpp
@@ -59,11 +59,13 @@ void qux() {
 // CHECK-MESSAGES: [[@LINE-1]]:3: warning: consider replacing 'short' with 
'int16'
 }
 
-// FIXME: This shouldn't warn, as UD-literal operators require one of a handful
-// of types as an argument.
 struct some_value {};
-constexpr some_value operator"" _some_literal(unsigned long long int i);
-// CHECK-MESSAGES: [[@LINE-1]]:47: warning: consider replacing 'unsigned long 
long'
+constexpr some_value operator"" _some_literal(unsigned long long int i)
+{
+  short j;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: consider replacing 'short' with 
'int16'
+  return some_value();
+}
 
 struct A { A& operator=(const A&); };
 class B { A a[0]; };

From b54e033b0a4cbc72d345521c9b5e2ff96211d589 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sat, 20 Jan 2024 21:02:42 -0500
Subject: [PATCH 2/2] fixup! [clang-tidy] Ignore user-defined literals in
 google-runtime-int

Fixed Documentation
Improved ast matcher to use getLiteralIdentifier
---
 .../clang-tidy/google/IntegerTypesCheck.cpp| 10 --
 clang-tools-extra/docs/ReleaseNotes.rst|  4 ++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp 
b/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
index b60f2d0ed63112..ef511e9108f2ee 100644
--- a/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -36,6 +36,12 @@ static Token getTokenAtLoc(SourceLocation Loc,
   return Tok;
 }
 
+namespace {
+AST_MATCHER(FunctionDecl, isUserDefineLiteral) {
+  return Node.getLiteralIdentifier() != nullptr;
+}
+} // namespace
+
 namespace tidy::google::runtime {
 
 IntegerTypesCheck::IntegerTypesCheck(StringRef Name, ClangTidyContext *Context)
@@ -60,8 +66,8 @@ void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) 
{
   typeLoc(loc(isInteger()),
   unless(anyOf(hasAncestor(callExpr(

[clang-tools-extra] [clang-tidy] Ignore user-defined literals in google-runtime-int (PR #78859)

2024-01-20 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 created 
https://github.com/llvm/llvm-project/pull/78859

User-defined literals do not accept u?intXX(_t)? variables. So the check should 
not emit a warning.

Fixes #54546 #25214

From 8ea205ae2324d67f9adaf30d5662d2cde2096534 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sat, 20 Jan 2024 15:29:06 -0500
Subject: [PATCH] [clang-tidy] Ignore user-defined literals in
 google-runtime-int

User-defined literals do not accept u?intXX(_t)? variables.
So the check should not emit a warning.

Fixes #54546 #25214
---
 .../clang-tidy/google/IntegerTypesCheck.cpp | 13 -
 clang-tools-extra/docs/ReleaseNotes.rst |  3 +++
 .../test/clang-tidy/checkers/google/runtime-int.cpp | 10 ++
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp 
b/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
index bb4e1de8cc4b15..b60f2d0ed63112 100644
--- a/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -56,11 +56,14 @@ void IntegerTypesCheck::registerMatchers(MatchFinder 
*Finder) {
   // http://google.github.io/styleguide/cppguide.html#64-bit_Portability
   // "Where possible, avoid passing arguments of types specified by
   // bitwidth typedefs to printf-based APIs."
-  Finder->addMatcher(typeLoc(loc(isInteger()),
- unless(hasAncestor(callExpr(
- 
callee(functionDecl(hasAttr(attr::Format)))
- .bind("tl"),
- this);
+  Finder->addMatcher(
+  typeLoc(loc(isInteger()),
+  unless(anyOf(hasAncestor(callExpr(
+   callee(functionDecl(hasAttr(attr::Format),
+   hasParent(parmVarDecl(hasAncestor(functionDecl(
+   matchesName("operator\"\".*$"
+  .bind("tl"),
+  this);
   IdentTable = std::make_unique(getLangOpts());
 }
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index a235a7d02592e8..5c857824c2b021 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -359,6 +359,9 @@ Changes in existing checks
   to ignore unused parameters when they are marked as unused and parameters of
   deleted functions and constructors.
 
+- Improved :doc:`google-runtime-int` check to ignore
+  false positives on user defined-literals.
+
 - Improved :doc:`llvm-namespace-comment
   ` check to provide fixes for
   ``inline`` namespaces in the same format as :program:`clang-format`.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/runtime-int.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/google/runtime-int.cpp
index 4bb739876b7f4c..88f0b519e9cbee 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/google/runtime-int.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/google/runtime-int.cpp
@@ -59,11 +59,13 @@ void qux() {
 // CHECK-MESSAGES: [[@LINE-1]]:3: warning: consider replacing 'short' with 
'int16'
 }
 
-// FIXME: This shouldn't warn, as UD-literal operators require one of a handful
-// of types as an argument.
 struct some_value {};
-constexpr some_value operator"" _some_literal(unsigned long long int i);
-// CHECK-MESSAGES: [[@LINE-1]]:47: warning: consider replacing 'unsigned long 
long'
+constexpr some_value operator"" _some_literal(unsigned long long int i)
+{
+  short j;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: consider replacing 'short' with 
'int16'
+  return some_value();
+}
 
 struct A { A& operator=(const A&); };
 class B { A a[0]; };

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


[clang-tools-extra] [clang] [llvm] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-20 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 requested changes to this pull request.


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


[clang] [clang-tools-extra] [llvm] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-20 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,135 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include 
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+static const llvm::StringRef AlgorithmHeader("");
+
+UseStdMinMaxCheck::UseStdMinMaxCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+   utils::IncludeSorter::IS_LLVM),
+  areDiagsSelfContained()) {}
+
+void UseStdMinMaxCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
+}
+
+void UseStdMinMaxCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+  ifStmt(
+  hasCondition(binaryOperator(hasAnyOperatorName("<", ">", "<=", ">="),
+  hasLHS(expr().bind("CondLhs")),
+  hasRHS(expr().bind("CondRhs",
+  hasThen(anyOf(stmt(binaryOperator(hasOperatorName("="),
+hasLHS(expr().bind("AssignLhs")),
+hasRHS(expr().bind("AssignRhs",
+compoundStmt(has(binaryOperator(
+ hasOperatorName("="),
+ hasLHS(expr().bind("AssignLhs")),
+ hasRHS(expr().bind("AssignRhs")
+.bind("compound"
+  .bind("if"),
+  this);
+}
+
+void UseStdMinMaxCheck::registerPPCallbacks(const SourceManager ,
+Preprocessor *PP,
+Preprocessor *ModuleExpanderPP) {
+  IncludeInserter.registerPreprocessor(PP);
+}
+
+void UseStdMinMaxCheck::check(const MatchFinder::MatchResult ) {
+  const auto *CondLhs = Result.Nodes.getNodeAs("CondLhs");
+  const auto *CondRhs = Result.Nodes.getNodeAs("CondRhs");
+  const auto *AssignLhs = Result.Nodes.getNodeAs("AssignLhs");
+  const auto *AssignRhs = Result.Nodes.getNodeAs("AssignRhs");
+  const auto *If = Result.Nodes.getNodeAs("if");
+  const auto *Compound = Result.Nodes.getNodeAs("compound");
+  const auto  = *Result.Context;
+  const auto  = Context.getLangOpts();
+  const SourceManager  = Context.getSourceManager();
+
+  const auto *BinaryOp = dyn_cast(If->getCond());
+  if (!BinaryOp || If->hasElseStorage())
+return;
+
+  if (Compound) {
+if (Compound->size() > 1)
+  return;
+  }
+
+  const SourceLocation IfLocation = If->getIfLoc();
+  const SourceLocation ThenLocation = If->getEndLoc();
+
+  if (IfLocation.isMacroID() || ThenLocation.isMacroID())
+return;
+
+  const auto CreateReplacement = [&](bool UseMax) {
+const auto *FunctionName = UseMax ? "std::max" : "std::min";
+const auto CondLhsStr = Lexer::getSourceText(
+Source.getExpansionRange(CondLhs->getSourceRange()), Source, LO);
+const auto CondRhsStr = Lexer::getSourceText(
+Source.getExpansionRange(CondRhs->getSourceRange()), Source, LO);
+const auto AssignLhsStr = Lexer::getSourceText(
+Source.getExpansionRange(AssignLhs->getSourceRange()), Source, LO);
+const auto AssignLhsType =
+AssignLhs->getType().getCanonicalType().getAsString();
+return (AssignLhsStr + " = " + FunctionName + "<" + AssignLhsType + ">(" +

felix642 wrote:

We should only specify the template type if the compiler is unable to deduce 
it. Otherwise it is redundant

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


[clang-tools-extra] [llvm] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-20 Thread Félix-Antoine Constantin via cfe-commits

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


[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-20 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,139 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include 
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+UseStdMinMaxCheck::UseStdMinMaxCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+   utils::IncludeSorter::IS_LLVM),

felix642 wrote:

No worries, in the constructor of the check you should initialize the variable 
```
AlgorithmHeader(Options.get(...));
```
and then store the value of the variable in the `Store` method
```
Options.store(Opts, "AlgorithmHeader", AlgorithmHeader);
```

The logic is the same has `IncludeInserter`, the only difference is that you 
should use `Options.get` rather than `Options.getLocalOrGlobal`.

(To be transparent, I still think this option shouldn't exist since there is no 
reason to overwrite the header)

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


[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-20 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,139 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include 
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+UseStdMinMaxCheck::UseStdMinMaxCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+   utils::IncludeSorter::IS_LLVM),

felix642 wrote:

The variable `AlgorithmHeader` is not even present in the snippet that you 
provided. So I highly doubt that it's initialized

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


[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-20 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,139 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include 
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+UseStdMinMaxCheck::UseStdMinMaxCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+   utils::IncludeSorter::IS_LLVM),

felix642 wrote:

You are missing the initialization of the field `AlgorithmHeader`

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


[clang-tools-extra] [clang] [llvm] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-20 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 requested changes to this pull request.

@11happy I've left a few more comments regarding some minor issues with your 
check. Please have a look and let me know what you think.

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


[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-20 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,175 @@
+// RUN: %check_clang_tidy %s readability-use-std-min-max %t
+
+#define MY_MACRO_MIN(a, b) ((a) < (b) ? (a) : (b))
+
+constexpr int myConstexprMin(int a, int b) {
+  return a < b ? a : b;
+}
+
+constexpr int myConstexprMax(int a, int b) {
+  return a > b ? a : b;
+}
+
+#define MY_IF_MACRO(condition, statement) \
+  if (condition) {\
+statement \
+  }
+
+class MyClass {
+public:
+  int member1;
+  int member2;
+};
+
+void foo() {
+  int value1,value2,value3;
+  short value4;
+  MyClass obj;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value2);
+  if (value1 < value2)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value1, value2);
+  if (value1 < value2)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value2, value1);
+  if (value2 > value1)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` 
[readability-use-std-min-max
+  // CHECK-FIXES: value1 = std::max(value2, value1);
+  if (value2 > value1)
+value1 = value2; 
+
+  // No suggestion needed here
+  if (value1 == value2)
+value1 = value2;
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value4);
+  if(value1` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::min(value1, myConstexprMax(value2, value3));
+  if (value1 > myConstexprMax(value2, value3))
+value1 = myConstexprMax(value2, value3); 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value1, value2);
+  if (value1 <= value2)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value2);
+  if (value1 <= value2)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value2, value1);
+  if (value2 >= value1)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value2, value1);
+  if (value2 >= value1)
+value2 = value1; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member1, obj.member2);
+  if (obj.member1 < obj.member2)
+obj.member1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member1, obj.member2);
+  if (obj.member1 < obj.member2)
+obj.member2 = obj.member1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member2, obj.member1);
+  if (obj.member2 > obj.member1)
+obj.member2 = obj.member1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member2, obj.member1);
+  if (obj.member2 > obj.member1)
+obj.member1 = obj.member2; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member1, value4);
+  if (obj.member1 < value4)
+obj.member1 = value4; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value3 = std::min(obj.member1 + value2, value3);
+  if (obj.member1 + value2 < value3)
+value3 = obj.member1 + value2; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(value1, obj.member2);
+  if (value1 <= obj.member2)
+obj.member2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, obj.member2);
+  if (value1 <= obj.member2)
+value1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(obj.member2, value1);
+  if (obj.member2 >= value1)
+value1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` 

[clang] [clang-tools-extra] [llvm] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-20 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,129 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include 
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+UseStdMinMaxCheck::UseStdMinMaxCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+   utils::IncludeSorter::IS_LLVM),
+  areDiagsSelfContained()) {}
+
+void UseStdMinMaxCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
+  Options.store(Opts, "AlgorithmHeader",
+Options.get("AlgorithmHeader", ""));
+}
+
+void UseStdMinMaxCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+  ifStmt(
+  hasCondition(binaryOperator(hasAnyOperatorName("<", ">", "<=", ">="),
+  hasLHS(expr().bind("CondLhs")),
+  hasRHS(expr().bind("CondRhs",
+  hasThen(anyOf(stmt(binaryOperator(hasOperatorName("="),
+hasLHS(expr().bind("AssignLhs")),
+hasRHS(expr().bind("AssignRhs",
+compoundStmt(has(binaryOperator(
+ hasOperatorName("="),
+ hasLHS(expr().bind("AssignLhs")),
+ hasRHS(expr().bind("AssignRhs")
+.bind("compound"
+  .bind("if"),
+  this);
+}
+
+void UseStdMinMaxCheck::registerPPCallbacks(const SourceManager ,
+Preprocessor *PP,
+Preprocessor *ModuleExpanderPP) {
+  IncludeInserter.registerPreprocessor(PP);
+}
+
+void UseStdMinMaxCheck::check(const MatchFinder::MatchResult ) {
+  const auto *CondLhs = Result.Nodes.getNodeAs("CondLhs");
+  const auto *CondRhs = Result.Nodes.getNodeAs("CondRhs");
+  const auto *AssignLhs = Result.Nodes.getNodeAs("AssignLhs");
+  const auto *AssignRhs = Result.Nodes.getNodeAs("AssignRhs");
+  const auto *If = Result.Nodes.getNodeAs("if");
+  const auto *Compound = Result.Nodes.getNodeAs("compound");
+  const auto  = *Result.Context;
+  const auto  = Context.getLangOpts();
+  const SourceManager  = Context.getSourceManager();
+
+  const auto *BinaryOp = dyn_cast(If->getCond());
+  if (!BinaryOp || If->hasElseStorage())
+return;
+
+  if (Compound) {
+if (Compound->size() > 1)
+  return;
+  }
+
+  const SourceLocation IfLocation = If->getIfLoc();
+  const SourceLocation ThenLocation = If->getEndLoc();

felix642 wrote:

The thenLocation might not be valid in some cases.
Let's take for example this code :
```
if(a < b)
a = b;
```

I would assume that the ThenLocation will match on the character 'b', but in 
your case you want to match on the ';' otherwise you'll add an extra semicolon 
at the end of the line.

You'll need to add a call to `Lexer::getLocForEndOfToken` to resolve this 
issue. Please have a look on how other checks do it.

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


[clang-tools-extra] [clang] [llvm] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-20 Thread Félix-Antoine Constantin via cfe-commits

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


[clang-tools-extra] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)

2024-01-19 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/73069

From 286c4445f8cba6ea2f49fb9e8f732f04ebdb6c97 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Thu, 16 Nov 2023 22:03:15 -0500
Subject: [PATCH] =?UTF-8?q?[clang-tidy]=C2=A0Added=20check=20to=20detect?=
 =?UTF-8?q?=20redundant=20inline=20keyword?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This checks find usages of the inline keywork where it is
already implicitly defined by the compiler and suggests it's removal.

Fixes #72397
---
 .../clang-tidy/readability/CMakeLists.txt |   1 +
 .../readability/ReadabilityTidyModule.cpp |   3 +
 .../RedundantInlineSpecifierCheck.cpp | 132 +
 .../RedundantInlineSpecifierCheck.h   |  42 ++
 clang-tools-extra/docs/ReleaseNotes.rst   |   5 +
 .../docs/clang-tidy/checks/list.rst   |   1 +
 .../redundant-inline-specifier.rst|  32 
 .../redundant-inline-specifier.cpp| 137 ++
 8 files changed, 353 insertions(+)
 create mode 100644 
clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
 create mode 100644 
clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.h
 create mode 100644 
clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-inline-specifier.cpp

diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index fa571d5dd7650d1..1d15228da694510 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -22,6 +22,7 @@ add_clang_library(clangTidyReadabilityModule
   IdentifierLengthCheck.cpp
   IdentifierNamingCheck.cpp
   ImplicitBoolConversionCheck.cpp
+  RedundantInlineSpecifierCheck.cpp
   InconsistentDeclarationParameterNameCheck.cpp
   IsolateDeclarationCheck.cpp
   MagicNumbersCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp 
b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index f769752c5de5fa7..521dacd6f9df3e2 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -41,6 +41,7 @@
 #include "RedundantControlFlowCheck.h"
 #include "RedundantDeclarationCheck.h"
 #include "RedundantFunctionPtrDereferenceCheck.h"
+#include "RedundantInlineSpecifierCheck.h"
 #include "RedundantMemberInitCheck.h"
 #include "RedundantPreprocessorCheck.h"
 #include "RedundantSmartptrGetCheck.h"
@@ -99,6 +100,8 @@ class ReadabilityModule : public ClangTidyModule {
 "readability-identifier-naming");
 CheckFactories.registerCheck(
 "readability-implicit-bool-conversion");
+CheckFactories.registerCheck(
+"readability-redundant-inline-specifier");
 CheckFactories.registerCheck(
 "readability-inconsistent-declaration-parameter-name");
 CheckFactories.registerCheck(
diff --git 
a/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
new file mode 100644
index 000..0e8d17d4442478c
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/RedundantInlineSpecifierCheck.cpp
@@ -0,0 +1,132 @@
+//===--- RedundantInlineSpecifierCheck.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 "RedundantInlineSpecifierCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Lex/Token.h"
+
+#include "../utils/LexerUtils.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+namespace {
+AST_POLYMORPHIC_MATCHER(isInlineSpecified,
+AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
+VarDecl)) {
+  if (const auto *FD = dyn_cast())
+return FD->isInlineSpecified();
+  if (const auto *VD = dyn_cast())
+return VD->isInlineSpecified();
+  llvm_unreachable("Not a valid polymorphic type");
+}
+
+AST_POLYMORPHIC_MATCHER_P(isInternalLinkage,
+  AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
+   

[clang] [clang-tools-extra] [llvm] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,149 @@
+// RUN: %check_clang_tidy %s readability-use-std-min-max %t
+
+#define MY_MACRO_MIN(a, b) ((a) < (b) ? (a) : (b))
+
+constexpr int myConstexprMin(int a, int b) {
+  return a < b ? a : b;
+}
+
+constexpr int myConstexprMax(int a, int b) {
+  return a > b ? a : b;
+}
+
+class MyClass {
+public:
+  int member1;
+  int member2;
+};
+
+void foo() {
+  int value1,value2,value3;
+  short value4;
+  MyClass obj;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value2);
+  if (value1 < value2)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value1, value2);
+  if (value1 < value2)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value2, value1);
+  if (value2 > value1)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` 
[readability-use-std-min-max
+  // CHECK-FIXES: value1 = std::max(value2, value1);
+  if (value2 > value1)
+value1 = value2; 
+
+  // No suggestion needed here
+  if (value1 == value2)
+value1 = value2;
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value4);
+  if(value1` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::min(value1, myConstexprMax(value2, value3));
+  if (value1 > myConstexprMax(value2, value3))
+value1 = myConstexprMax(value2, value3); 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value1, value2);
+  if (value1 <= value2)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value2);
+  if (value1 <= value2)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value2, value1);
+  if (value2 >= value1)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value2, value1);
+  if (value2 >= value1)
+value2 = value1; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member1, obj.member2);
+  if (obj.member1 < obj.member2)
+obj.member1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member1, obj.member2);
+  if (obj.member1 < obj.member2)
+obj.member2 = obj.member1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member2, obj.member1);
+  if (obj.member2 > obj.member1)
+obj.member2 = obj.member1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member2, obj.member1);
+  if (obj.member2 > obj.member1)
+obj.member1 = obj.member2; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member1, value4);
+  if (obj.member1 < value4)
+obj.member1 = value4; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value3 = std::min(obj.member1 + value2, value3);
+  if (obj.member1 + value2 < value3)
+value3 = obj.member1 + value2; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(value1, obj.member2);
+  if (value1 <= obj.member2)
+obj.member2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, obj.member2);
+  if (value1 <= obj.member2)
+value1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(obj.member2, value1);
+  if (obj.member2 >= value1)
+value1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member2, value1);
+  if (obj.member2 >= value1)
+obj.member2 = 

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,149 @@
+// RUN: %check_clang_tidy %s readability-use-std-min-max %t
+
+#define MY_MACRO_MIN(a, b) ((a) < (b) ? (a) : (b))
+
+constexpr int myConstexprMin(int a, int b) {
+  return a < b ? a : b;
+}
+
+constexpr int myConstexprMax(int a, int b) {
+  return a > b ? a : b;
+}
+
+class MyClass {
+public:
+  int member1;
+  int member2;
+};
+
+void foo() {
+  int value1,value2,value3;
+  short value4;
+  MyClass obj;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value2);
+  if (value1 < value2)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value1, value2);
+  if (value1 < value2)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value2, value1);
+  if (value2 > value1)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` 
[readability-use-std-min-max
+  // CHECK-FIXES: value1 = std::max(value2, value1);
+  if (value2 > value1)
+value1 = value2; 
+
+  // No suggestion needed here
+  if (value1 == value2)
+value1 = value2;
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value4);
+  if(value1` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::min(value1, myConstexprMax(value2, value3));
+  if (value1 > myConstexprMax(value2, value3))
+value1 = myConstexprMax(value2, value3); 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value1, value2);
+  if (value1 <= value2)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value2);
+  if (value1 <= value2)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value2, value1);
+  if (value2 >= value1)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value2, value1);
+  if (value2 >= value1)
+value2 = value1; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member1, obj.member2);
+  if (obj.member1 < obj.member2)
+obj.member1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member1, obj.member2);
+  if (obj.member1 < obj.member2)
+obj.member2 = obj.member1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member2, obj.member1);
+  if (obj.member2 > obj.member1)
+obj.member2 = obj.member1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member2, obj.member1);
+  if (obj.member2 > obj.member1)
+obj.member1 = obj.member2; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member1, value4);
+  if (obj.member1 < value4)
+obj.member1 = value4; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value3 = std::min(obj.member1 + value2, value3);
+  if (obj.member1 + value2 < value3)
+value3 = obj.member1 + value2; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(value1, obj.member2);
+  if (value1 <= obj.member2)
+obj.member2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, obj.member2);
+  if (value1 <= obj.member2)
+value1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(obj.member2, value1);
+  if (obj.member2 >= value1)
+value1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member2, value1);
+  if (obj.member2 >= value1)
+obj.member2 = 

[clang-tools-extra] [llvm] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,149 @@
+// RUN: %check_clang_tidy %s readability-use-std-min-max %t
+
+#define MY_MACRO_MIN(a, b) ((a) < (b) ? (a) : (b))
+
+constexpr int myConstexprMin(int a, int b) {
+  return a < b ? a : b;
+}
+
+constexpr int myConstexprMax(int a, int b) {
+  return a > b ? a : b;
+}
+
+class MyClass {
+public:
+  int member1;
+  int member2;
+};
+
+void foo() {
+  int value1,value2,value3;
+  short value4;
+  MyClass obj;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value2);
+  if (value1 < value2)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value1, value2);
+  if (value1 < value2)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value2, value1);
+  if (value2 > value1)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` 
[readability-use-std-min-max
+  // CHECK-FIXES: value1 = std::max(value2, value1);
+  if (value2 > value1)
+value1 = value2; 
+
+  // No suggestion needed here
+  if (value1 == value2)
+value1 = value2;
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value4);
+  if(value1` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::min(value1, myConstexprMax(value2, value3));
+  if (value1 > myConstexprMax(value2, value3))
+value1 = myConstexprMax(value2, value3); 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value1, value2);
+  if (value1 <= value2)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value2);
+  if (value1 <= value2)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value2, value1);
+  if (value2 >= value1)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value2, value1);
+  if (value2 >= value1)
+value2 = value1; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member1, obj.member2);
+  if (obj.member1 < obj.member2)
+obj.member1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member1, obj.member2);
+  if (obj.member1 < obj.member2)
+obj.member2 = obj.member1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member2, obj.member1);
+  if (obj.member2 > obj.member1)
+obj.member2 = obj.member1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member2, obj.member1);
+  if (obj.member2 > obj.member1)
+obj.member1 = obj.member2; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member1, value4);
+  if (obj.member1 < value4)
+obj.member1 = value4; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value3 = std::min(obj.member1 + value2, value3);
+  if (obj.member1 + value2 < value3)
+value3 = obj.member1 + value2; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(value1, obj.member2);
+  if (value1 <= obj.member2)
+obj.member2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, obj.member2);
+  if (value1 <= obj.member2)
+value1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(obj.member2, value1);
+  if (obj.member2 >= value1)
+value1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member2, value1);
+  if (obj.member2 >= value1)
+obj.member2 = 

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,149 @@
+// RUN: %check_clang_tidy %s readability-use-std-min-max %t
+
+#define MY_MACRO_MIN(a, b) ((a) < (b) ? (a) : (b))
+
+constexpr int myConstexprMin(int a, int b) {
+  return a < b ? a : b;
+}
+
+constexpr int myConstexprMax(int a, int b) {
+  return a > b ? a : b;
+}
+
+class MyClass {
+public:
+  int member1;
+  int member2;
+};
+
+void foo() {
+  int value1,value2,value3;
+  short value4;
+  MyClass obj;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value2);
+  if (value1 < value2)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value1, value2);
+  if (value1 < value2)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value2, value1);
+  if (value2 > value1)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` 
[readability-use-std-min-max
+  // CHECK-FIXES: value1 = std::max(value2, value1);
+  if (value2 > value1)
+value1 = value2; 
+
+  // No suggestion needed here
+  if (value1 == value2)
+value1 = value2;
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value4);
+  if(value1` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::min(value1, myConstexprMax(value2, value3));
+  if (value1 > myConstexprMax(value2, value3))
+value1 = myConstexprMax(value2, value3); 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value1, value2);
+  if (value1 <= value2)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value2);
+  if (value1 <= value2)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value2, value1);
+  if (value2 >= value1)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value2, value1);
+  if (value2 >= value1)
+value2 = value1; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member1, obj.member2);
+  if (obj.member1 < obj.member2)
+obj.member1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member1, obj.member2);
+  if (obj.member1 < obj.member2)
+obj.member2 = obj.member1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member2, obj.member1);
+  if (obj.member2 > obj.member1)
+obj.member2 = obj.member1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member2, obj.member1);
+  if (obj.member2 > obj.member1)
+obj.member1 = obj.member2; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member1, value4);
+  if (obj.member1 < value4)
+obj.member1 = value4; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value3 = std::min(obj.member1 + value2, value3);
+  if (obj.member1 + value2 < value3)
+value3 = obj.member1 + value2; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(value1, obj.member2);
+  if (value1 <= obj.member2)
+obj.member2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, obj.member2);
+  if (value1 <= obj.member2)
+value1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(obj.member2, value1);
+  if (obj.member2 >= value1)
+value1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member2, value1);
+  if (obj.member2 >= value1)
+obj.member2 = 

[llvm] [clang] [clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Félix-Antoine Constantin via cfe-commits

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


[clang-tools-extra] [clang] [llvm] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,149 @@
+// RUN: %check_clang_tidy %s readability-use-std-min-max %t
+
+#define MY_MACRO_MIN(a, b) ((a) < (b) ? (a) : (b))
+
+constexpr int myConstexprMin(int a, int b) {
+  return a < b ? a : b;
+}
+
+constexpr int myConstexprMax(int a, int b) {
+  return a > b ? a : b;
+}
+
+class MyClass {
+public:
+  int member1;
+  int member2;
+};
+
+void foo() {
+  int value1,value2,value3;
+  short value4;
+  MyClass obj;
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value2);
+  if (value1 < value2)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value1, value2);
+  if (value1 < value2)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value2, value1);
+  if (value2 > value1)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` 
[readability-use-std-min-max
+  // CHECK-FIXES: value1 = std::max(value2, value1);
+  if (value2 > value1)
+value1 = value2; 
+
+  // No suggestion needed here
+  if (value1 == value2)
+value1 = value2;
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value4);
+  if(value1` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::min(value1, myConstexprMax(value2, value3));
+  if (value1 > myConstexprMax(value2, value3))
+value1 = myConstexprMax(value2, value3); 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value1, value2);
+  if (value1 <= value2)
+value2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, value2);
+  if (value1 <= value2)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value2, value1);
+  if (value2 >= value1)
+value1 = value2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value2 = std::min(value2, value1);
+  if (value2 >= value1)
+value2 = value1; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member1, obj.member2);
+  if (obj.member1 < obj.member2)
+obj.member1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member1, obj.member2);
+  if (obj.member1 < obj.member2)
+obj.member2 = obj.member1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member2, obj.member1);
+  if (obj.member2 > obj.member1)
+obj.member2 = obj.member1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member2, obj.member1);
+  if (obj.member2 > obj.member1)
+obj.member1 = obj.member2; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member1 = std::max(obj.member1, value4);
+  if (obj.member1 < value4)
+obj.member1 = value4; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value3 = std::min(obj.member1 + value2, value3);
+  if (obj.member1 + value2 < value3)
+value3 = obj.member1 + value2; 
+  
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(value1, obj.member2);
+  if (value1 <= obj.member2)
+obj.member2 = value1; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `<=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(value1, obj.member2);
+  if (value1 <= obj.member2)
+value1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::max` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: value1 = std::max(obj.member2, value1);
+  if (obj.member2 >= value1)
+value1 = obj.member2; 
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use `std::min` instead of `>=` 
[readability-use-std-min-max]
+  // CHECK-FIXES: obj.member2 = std::min(obj.member2, value1);
+  if (obj.member2 >= value1)
+obj.member2 = 

[llvm] [clang-tools-extra] [clang] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-18 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 commented:

Hey @11happy I left 2 more comments related to some tests that we should maybe 
add. 

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


[clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-16 Thread Félix-Antoine Constantin via cfe-commits

felix642 wrote:

@11happy what are you trying to achieve exactly and what is not working ? 
Is this related to the example that you provided above? 

>``` 
>auto lhsVar1Str = Lexer::getSourceText( 
>>CharSourceRange::getTokenRange(Source.getSpellingLoc(lhsVar1->getBeginLoc()),Source.getSpellingLoc(lhsVar1->getEn>dLoc())),
>  Context.getSourceManager(), Context.getLangOpts());
>```
>it outputs this :
>```
 >//
>-  if (MY_MIN(value1, value2) < value3)
>-value3 = MY_MIN(value1, value2); //
>+  value3 = std::min((a) < (b) ? (a) : (b), value3); //
> }
>```

If that's the case I'm guessing that you simply forgot to update the begin and 
end location of the if/then stmt with the spelling loc ?  

Regarding the request for a test case with macros, my intention was to 
understand how your check handles them. I believe updating a macro is risk 
free, but there might be use cases that I'm overlooking.
 One potential issue could arise if someone is using the macro in another 
translation unit without directly importing it. In that case, updating the 
macro without the necessary include might cause compilation failures. However, 
I'm unsure if this is a use case that needs consideration.

It might be worth to look if other checks are currently updating macros. This 
might be a decent indicator of the risk of doing so (If a few checks are 
already doing it then I might be overthinking it). Also, @PiotrZSL might be 
able to give us some insight on this since he has more experience with 
clang-tidy.

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


[clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-15 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,121 @@
+// RUN: %check_clang_tidy %s readability-use-std-min-max %t
+
+constexpr int myConstexprMin(int a, int b) {
+  return a < b ? a : b;
+}
+
+constexpr int myConstexprMax(int a, int b) {
+  return a > b ? a : b;
+}
+
+int bar(int x, int y) {
+  return x < y ? x : y;
+}
+
+class MyClass {
+public:
+  int member1;
+  int member2;
+};
+
+void foo() {
+  int value1,value2,value3;
+  short value4;
+  MyClass obj;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  if (value1 < value2)
+value1 = value2; // CHECK-FIXES: value1 = std::max(value1, value2);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  if (value1 < value2)
+value2 = value1; // CHECK-FIXES: value2 = std::min(value1, value2);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `>` 
[readability-use-std-min-max]
+  if (value2 > value1)
+value2 = value1; // CHECK-FIXES: value2 = std::min(value2, value1);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `>` 
[readability-use-std-min-max
+  if (value2 > value1)
+value1 = value2; // CHECK-FIXES: value1 = std::max(value2, value1);
+
+  // No suggestion needed here
+  if (value1 == value2)
+value1 = value2;
+  
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  if(value1` 
[readability-use-std-min-max]
+  if (value1 > myConstexprMax(value2, value3))
+value1 = myConstexprMax(value2, value3); // CHECK-FIXES: value1 = 
std::min(value1, myConstexprMax(value2, value3));
+  
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  if (value1 < bar(value2, value3))
+value1 = bar(value2, value3); // CHECK-FIXES: value1 = std::max(value1, 
bar(value2, value3));
+  
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `<=` 
[readability-use-std-min-max]
+  if (value1 <= value2)
+value2 = value1; // CHECK-FIXES: value2 = std::min(value1, value2);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<=` 
[readability-use-std-min-max]
+  if (value1 <= value2)
+value1 = value2; // CHECK-FIXES: value1 = std::max(value1, value2);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `>=` 
[readability-use-std-min-max]
+  if (value2 >= value1)
+value1 = value2; // CHECK-FIXES: value1 = std::max(value2, value1);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `>=` 
[readability-use-std-min-max]
+  if (value2 >= value1)
+value2 = value1; // CHECK-FIXES: value2 = std::min(value2, value1);
+  
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  if (obj.member1 < obj.member2)
+obj.member1 = obj.member2; // CHECK-FIXES: obj.member1 = 
std::max(obj.member1, obj.member2);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  if (obj.member1 < obj.member2)
+obj.member2 = obj.member1; // CHECK-FIXES: obj.member2 = 
std::min(obj.member1, obj.member2);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `>` 
[readability-use-std-min-max]
+  if (obj.member2 > obj.member1)
+obj.member2 = obj.member1; // CHECK-FIXES: obj.member2 = 
std::min(obj.member2, obj.member1);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `>` 
[readability-use-std-min-max]
+  if (obj.member2 > obj.member1)
+obj.member1 = obj.member2; // CHECK-FIXES: obj.member1 = 
std::max(obj.member2, obj.member1);
+  
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  if (obj.member1 < value4)
+obj.member1 = value4; // CHECK-FIXES: obj.member1 = std::max(obj.member1, 
value4);
+  
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  if (obj.member1 + value2 < value3)
+value3 = obj.member1 + value2; // CHECK-FIXES: value3 = 
std::min(obj.member1 + value2, value3);
+  
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `<=` 
[readability-use-std-min-max]
+  if (value1 <= obj.member2)
+obj.member2 = value1; // CHECK-FIXES: obj.member2 = std::min(value1, 
obj.member2);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<=` 
[readability-use-std-min-max]
+  if (value1 <= obj.member2)
+value1 = obj.member2; // CHECK-FIXES: value1 = std::max(value1, 
obj.member2);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `>=` 
[readability-use-std-min-max]
+  if (obj.member2 >= value1)
+value1 = obj.member2; // CHECK-FIXES: value1 = std::max(obj.member2, 
value1);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `>=` 

[clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-15 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,128 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include 
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+UseStdMinMaxCheck::UseStdMinMaxCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+   utils::IncludeSorter::IS_LLVM),
+  areDiagsSelfContained()),
+  AlgotithmHeader(Options.get("AlgorithmHeader", "")) {}

felix642 wrote:

Also, I do not think we need an option for this header. std::min/max uses 
\ and should not change in the future

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


[clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-15 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,121 @@
+// RUN: %check_clang_tidy %s readability-use-std-min-max %t
+
+constexpr int myConstexprMin(int a, int b) {
+  return a < b ? a : b;
+}
+
+constexpr int myConstexprMax(int a, int b) {
+  return a > b ? a : b;
+}
+
+int bar(int x, int y) {
+  return x < y ? x : y;
+}
+
+class MyClass {
+public:
+  int member1;
+  int member2;
+};
+
+void foo() {
+  int value1,value2,value3;
+  short value4;
+  MyClass obj;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  if (value1 < value2)
+value1 = value2; // CHECK-FIXES: value1 = std::max(value1, value2);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `<` 
[readability-use-std-min-max]
+  if (value1 < value2)
+value2 = value1; // CHECK-FIXES: value2 = std::min(value1, value2);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::min` instead of `>` 
[readability-use-std-min-max]
+  if (value2 > value1)
+value2 = value1; // CHECK-FIXES: value2 = std::min(value2, value1);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `>` 
[readability-use-std-min-max
+  if (value2 > value1)
+value1 = value2; // CHECK-FIXES: value1 = std::max(value2, value1);
+
+  // No suggestion needed here
+  if (value1 == value2)
+value1 = value2;
+  
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  if(value1` 
[readability-use-std-min-max]
+  if (value1 > myConstexprMax(value2, value3))
+value1 = myConstexprMax(value2, value3); // CHECK-FIXES: value1 = 
std::min(value1, myConstexprMax(value2, value3));
+  
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use `std::max` instead of `<` 
[readability-use-std-min-max]
+  if (value1 < bar(value2, value3))

felix642 wrote:

I'm not quite sure about this one. What if bar() has some side-effects ? This 
will break the code since the method will be called once rather than twice. 

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


[clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-15 Thread Félix-Antoine Constantin via cfe-commits

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


[clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-15 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,128 @@
+//===--- UseStdMinMaxCheck.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 "UseStdMinMaxCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Preprocessor.h"
+#include 
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+UseStdMinMaxCheck::UseStdMinMaxCheck(StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+   utils::IncludeSorter::IS_LLVM),
+  areDiagsSelfContained()),
+  AlgotithmHeader(Options.get("AlgorithmHeader", "")) {}

felix642 wrote:

There's a typo in the name of the variable `algotithmHeader`

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


[clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-15 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 requested changes to this pull request.

Overall looks good, but left a few comments regarding some nits that I think 
should be addressed before landing this PR.

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


[llvm] [clang-tools-extra] [clang-tidy] Fix missing parentheses in readability-implicit-bool-conversion fixes (PR #74891)

2024-01-15 Thread Félix-Antoine Constantin via cfe-commits

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


https://github.com/llvm/llvm-project/pull/74891
___
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 missing parentheses in readability-implicit-bool-conversion fixes (PR #74891)

2024-01-15 Thread Félix-Antoine Constantin via cfe-commits

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


https://github.com/llvm/llvm-project/pull/74891
___
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 missing parentheses in readability-implicit-bool-conversion fixes (PR #74891)

2024-01-15 Thread Félix-Antoine Constantin via cfe-commits

felix642 wrote:

LGTM !
You'll need to update your branch with master but the PR looks good.

The only thing that seems odd is the changes that you've made to the method 
`areParensNeededForOverloadedOperator` where the Operators `OO_New`, 
`OO_Delete`, `OO_Array_New` and `OO_ArrayDelete`  now need parenthesis where 
they did not before. 
I'm assuming that you have a used case in mind where parenthesis would be 
needed and this is why you've changed the behaviour.

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


[clang-tools-extra] [clang] [clang-tidy] Added new check to detect redundant inline keyword (PR #73069)

2024-01-15 Thread Félix-Antoine Constantin via cfe-commits

felix642 wrote:

ping @EugeneZelenko 

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


[clang-tools-extra] [clang-tidy] Invalid Fix-It generated for implicit-widening-multiplication-result (PR #76315)

2024-01-12 Thread Félix-Antoine Constantin via cfe-commits

felix642 wrote:

@PiotrZSL thank you for the review. 
I would greatly appreciate if you could merge this PR for me since I do not 
have the rights to do it.

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


[clang-tools-extra] [clang-tidy] Invalid Fix-It generated for implicit-widening-multiplication-result (PR #76315)

2024-01-12 Thread Félix-Antoine Constantin via cfe-commits


@@ -18,7 +18,7 @@ char *t0(char *base, int a, int b) {
   // CHECK-NOTES-CXX:  static_cast( )
   // CHECK-NOTES-ALL: :[[@LINE-5]]:17: note: perform multiplication in a wider 
type
   // CHECK-NOTES-C:(ptrdiff_t)
-  // CHECK-NOTES-CXX:  static_cast()
+  // CHECK-NOTES-CXX:  static_cast( )

felix642 wrote:

I agree with you, like I've mentionned :

> Since both suggestions are overlapping clang-tidy refuses to apply them [...]

This is probably the reason why they are currently not using `CHECK-FIXES`. It 
would be a good idea to update the tests to use `CHECK-FIXES` but we would 
probably need to remove one of the FIX-IT for it to work.


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


[clang-tools-extra] Add clang-tidy check to suggest replacement of conditional statement with std::min/std::max (PR #77816)

2024-01-11 Thread Félix-Antoine Constantin via cfe-commits


@@ -0,0 +1,27 @@
+// RUN: %check_clang_tidy %s readability-ConditionalToStdMinMax %t
+
+void foo() {
+  int value1,value2;
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::max instead of < 
[readability-ConditionalToStdMinMax]
+  if (value1 < value2)
+value1 = value2; // CHECK-FIXES: value1 = std::max(value1, value2);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::min instead of < 
[readability-ConditionalToStdMinMax]
+  if (value1 < value2)
+value2 = value1; // CHECK-FIXES: value2 = std::min(value1, value2);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::min instead of > 
[readability-ConditionalToStdMinMax]
+  if (value2 > value1)
+value2 = value1; // CHECK-FIXES: value2 = std::min(value2, value1);
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use std::max instead of > 
[readability-ConditionalToStdMinMax]
+  if (value2 > value1)
+value1 = value2; // CHECK-FIXES: value1 = std::max(value2, value1);
+
+  // No suggestion needed here
+  if (value1 == value2)
+value1 = value2;
+
+  

felix642 wrote:

What about classes ? 
We should add a test case that includes a comparison with a class member.

```
struct Foo
{
int  x;
};

void func()
{
int bar;
Foo foo;
if(bar < foo.x)
bar = foo.x;
}
```
I'm guessing that ideally we would also need to support this type of operation :
```
struct Foo
{
int x;
auto operator<=>(const Foo& rhs) const = default;
};

void func()
{
Foo foo1;
Foo foo2;
if(foo2 < foo1)
foo2 = foo1;
}
```

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


[clang-tools-extra] [clang] [clang][tidy] Fixed clang-tidy rewriter not properly handling symlinks (PR #76350)

2023-12-24 Thread Félix-Antoine Constantin via cfe-commits

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


[clang-tools-extra] [clang] Fixed clang-tidy rewriter not properly handling symlinks (PR #76350)

2023-12-24 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 updated 
https://github.com/llvm/llvm-project/pull/76350

From 0a5ae9bfff7597794889c93e4da5789714be3a4f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 24 Dec 2023 21:01:32 -0500
Subject: [PATCH] [clang][tidy] Fixed clang-tidy rewriter not properly handling
 symlinks

Rewriter would not properly fix files if they were symlinked and using
--fix with clang-tidy would overwrite the symlink with the corrections
rather than the file.

With these changes the Rewriter now properly resolves the symlink before
applying the fixes.

Fixes #60845
---
 clang-tools-extra/docs/ReleaseNotes.rst   |  2 ++
 clang/lib/Rewrite/Rewriter.cpp| 24 +-
 clang/unittests/libclang/LibclangTest.cpp | 25 +++
 3 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 6d91748e4cef18..8a16ce1a7988a2 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -119,6 +119,8 @@ Improvements to clang-tidy
 
 - Improved `--dump-config` to print check options in alphabetical order.
 
+- Improved `--fix` to properly apply corrections on files that are symlinked.
+
 - Improved :program:`clang-tidy-diff.py` script. It now returns exit code `1`
   if any :program:`clang-tidy` subprocess exits with a non-zero code or if
   exporting fixes fails. It now accepts a directory as a value for
diff --git a/clang/lib/Rewrite/Rewriter.cpp b/clang/lib/Rewrite/Rewriter.cpp
index 0e6ae365064463..edc147ec304801 100644
--- a/clang/lib/Rewrite/Rewriter.cpp
+++ b/clang/lib/Rewrite/Rewriter.cpp
@@ -14,6 +14,7 @@
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/FileEntry.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
@@ -412,16 +413,19 @@ bool Rewriter::overwriteChangedFiles() {
   unsigned OverwriteFailure = Diag.getCustomDiagID(
   DiagnosticsEngine::Error, "unable to overwrite file %0: %1");
   for (buffer_iterator I = buffer_begin(), E = buffer_end(); I != E; ++I) {
-OptionalFileEntryRef Entry = getSourceMgr().getFileEntryRefForID(I->first);
-llvm::SmallString<128> Path(Entry->getName());
-getSourceMgr().getFileManager().makeAbsolutePath(Path);
-if (auto Error = llvm::writeToOutput(Path, [&](llvm::raw_ostream ) {
-  I->second.write(OS);
-  return llvm::Error::success();
-})) {
-  Diag.Report(OverwriteFailure)
-  << Entry->getName() << llvm::toString(std::move(Error));
-  AllWritten = false;
+if (OptionalFileEntryRef fileEntry =
+getSourceMgr().getFileEntryRefForID(I->first)) {
+  llvm::StringRef FileName =
+  getSourceMgr().getFileManager().getCanonicalName(*fileEntry);
+  if (auto Error =
+  llvm::writeToOutput(FileName, [&](llvm::raw_ostream ) {
+I->second.write(OS);
+return llvm::Error::success();
+  })) {
+Diag.Report(OverwriteFailure)
+<< FileName << llvm::toString(std::move(Error));
+AllWritten = false;
+  }
 }
   }
   return !AllWritten;
diff --git a/clang/unittests/libclang/LibclangTest.cpp 
b/clang/unittests/libclang/LibclangTest.cpp
index 87075a46d75187..cde19d9004cd81 100644
--- a/clang/unittests/libclang/LibclangTest.cpp
+++ b/clang/unittests/libclang/LibclangTest.cpp
@@ -16,6 +16,7 @@
 #include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1379,3 +1380,27 @@ TEST_F(LibclangRewriteTest, RewriteRemove) {
   ASSERT_EQ(clang_CXRewriter_overwriteChangedFiles(Rew), 0);
   EXPECT_EQ(getFileContent(Filename), "int () { return 0; }");
 }
+
+TEST_F(LibclangRewriteTest, Symlink) {
+  std::filesystem::path Symlink = "link.cpp";
+  std::filesystem::create_symlink(Filename, Symlink);
+  ASSERT_TRUE(std::filesystem::exists(Symlink));
+  FilesAndDirsToRemove.emplace(Symlink);
+
+  CXTranslationUnit SymTu = clang_parseTranslationUnit(
+  Index, Symlink.c_str(), nullptr, 0, nullptr, 0, TUFlags);
+  CXFile SymlinkFile = clang_getFile(SymTu, Symlink.c_str());
+  CXRewriter SymRew = clang_CXRewriter_create(SymTu);
+
+  CXSourceLocation B = clang_getLocation(SymTu, SymlinkFile, 1, 5);
+  CXSourceLocation E = clang_getLocation(SymTu, SymlinkFile, 1, 9);
+  CXSourceRange Rng = clang_getRange(B, E);
+
+  clang_CXRewriter_removeText(SymRew, Rng);
+
+  ASSERT_EQ(clang_CXRewriter_overwriteChangedFiles(SymRew), 0);
+  EXPECT_EQ(getFileContent(Filename), "int () { return 0; }");
+  EXPECT_TRUE(std::filesystem::is_symlink(Symlink));
+
+  clang_CXRewriter_dispose(SymRew);
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[clang-tools-extra] [clang] Fixed clang-tidy rewriter not properly handling symlinks (PR #76350)

2023-12-24 Thread Félix-Antoine Constantin via cfe-commits

https://github.com/felix642 created 
https://github.com/llvm/llvm-project/pull/76350

Rewriter would not properly fix files if they were symlinked and using --fix 
with clang-tidy would overwrite the symlink with the corrections rather than 
the file.

With these changes the Rewriter now properly resolves the symlink before 
applying the fixes.

Fixes #60845

From 37bde4bedaa6a80fb5c855a06d790cb0180fa5bd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?F=C3=A9lix-Antoine=20Constantin?=
 
Date: Sun, 24 Dec 2023 21:01:32 -0500
Subject: [PATCH] Fixed clang-tidy rewriter not properly handling symlinks

Rewriter would not properly fix files if they were symlinked and using
--fix with clang-tidy would overwrite the symlink with the corrections
rather than the file.

With these changes the Rewriter now properly resolves the symlink before
applying the fixes.

Fixes #60845
---
 clang-tools-extra/docs/ReleaseNotes.rst   |  2 ++
 clang/lib/Rewrite/Rewriter.cpp| 24 +-
 clang/unittests/libclang/LibclangTest.cpp | 25 +++
 3 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 6d91748e4cef18..8a16ce1a7988a2 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -119,6 +119,8 @@ Improvements to clang-tidy
 
 - Improved `--dump-config` to print check options in alphabetical order.
 
+- Improved `--fix` to properly apply corrections on files that are symlinked.
+
 - Improved :program:`clang-tidy-diff.py` script. It now returns exit code `1`
   if any :program:`clang-tidy` subprocess exits with a non-zero code or if
   exporting fixes fails. It now accepts a directory as a value for
diff --git a/clang/lib/Rewrite/Rewriter.cpp b/clang/lib/Rewrite/Rewriter.cpp
index 0e6ae365064463..edc147ec304801 100644
--- a/clang/lib/Rewrite/Rewriter.cpp
+++ b/clang/lib/Rewrite/Rewriter.cpp
@@ -14,6 +14,7 @@
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/FileEntry.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
@@ -412,16 +413,19 @@ bool Rewriter::overwriteChangedFiles() {
   unsigned OverwriteFailure = Diag.getCustomDiagID(
   DiagnosticsEngine::Error, "unable to overwrite file %0: %1");
   for (buffer_iterator I = buffer_begin(), E = buffer_end(); I != E; ++I) {
-OptionalFileEntryRef Entry = getSourceMgr().getFileEntryRefForID(I->first);
-llvm::SmallString<128> Path(Entry->getName());
-getSourceMgr().getFileManager().makeAbsolutePath(Path);
-if (auto Error = llvm::writeToOutput(Path, [&](llvm::raw_ostream ) {
-  I->second.write(OS);
-  return llvm::Error::success();
-})) {
-  Diag.Report(OverwriteFailure)
-  << Entry->getName() << llvm::toString(std::move(Error));
-  AllWritten = false;
+if (OptionalFileEntryRef fileEntry =
+getSourceMgr().getFileEntryRefForID(I->first)) {
+  llvm::StringRef FileName =
+  getSourceMgr().getFileManager().getCanonicalName(*fileEntry);
+  if (auto Error =
+  llvm::writeToOutput(FileName, [&](llvm::raw_ostream ) {
+I->second.write(OS);
+return llvm::Error::success();
+  })) {
+Diag.Report(OverwriteFailure)
+<< FileName << llvm::toString(std::move(Error));
+AllWritten = false;
+  }
 }
   }
   return !AllWritten;
diff --git a/clang/unittests/libclang/LibclangTest.cpp 
b/clang/unittests/libclang/LibclangTest.cpp
index 87075a46d75187..cde19d9004cd81 100644
--- a/clang/unittests/libclang/LibclangTest.cpp
+++ b/clang/unittests/libclang/LibclangTest.cpp
@@ -16,6 +16,7 @@
 #include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1379,3 +1380,27 @@ TEST_F(LibclangRewriteTest, RewriteRemove) {
   ASSERT_EQ(clang_CXRewriter_overwriteChangedFiles(Rew), 0);
   EXPECT_EQ(getFileContent(Filename), "int () { return 0; }");
 }
+
+TEST_F(LibclangRewriteTest, Symlink) {
+  std::filesystem::path Symlink = "link.cpp";
+  std::filesystem::create_symlink(Filename, Symlink);
+  ASSERT_TRUE(std::filesystem::exists(Symlink));
+  FilesAndDirsToRemove.emplace(Symlink);
+
+  CXTranslationUnit SymTu = clang_parseTranslationUnit(
+  Index, Symlink.c_str(), nullptr, 0, nullptr, 0, TUFlags);
+  CXFile SymlinkFile = clang_getFile(SymTu, Symlink.c_str());
+  CXRewriter SymRew = clang_CXRewriter_create(SymTu);
+
+  CXSourceLocation B = clang_getLocation(SymTu, SymlinkFile, 1, 5);
+  CXSourceLocation E = clang_getLocation(SymTu, SymlinkFile, 1, 9);
+  CXSourceRange Rng = clang_getRange(B, E);
+
+  clang_CXRewriter_removeText(SymRew, Rng);
+
+  ASSERT_EQ(clang_CXRewriter_overwriteChangedFiles(SymRew), 0);
+  EXPECT_EQ(getFileContent(Filename), "int () 

  1   2   >