[clang-tools-extra] [clang-tidy] Improved --verify-config when using literal style in config file (PR #85591)
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)
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)
@@ -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)
@@ -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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
@@ -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)
@@ -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)
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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
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)
@@ -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)
@@ -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)
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)
@@ -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)
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)
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)
@@ -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)
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)
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)
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)
@@ -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)
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)
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)
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)
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)
@@ -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)
@@ -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)
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)
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)
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)
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)
@@ -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)
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)
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)
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)
@@ -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)
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)
@@ -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)
@@ -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)
@@ -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)
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)
@@ -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)
@@ -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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
@@ -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)
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)
@@ -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)
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)
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)
@@ -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)
@@ -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)
@@ -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)
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)
@@ -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)
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)
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)
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)
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)
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)
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)
@@ -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)
@@ -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)
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)
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)
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 ()