[clang] [clang-tools-extra] [llvm] Add ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)
@@ -41,14 +41,17 @@ void
StringLiteralWithEmbeddedNulCheck::registerMatchers(MatchFinder *Finder) {
hasDeclaration(cxxMethodDecl(hasName("basic_string",
// If present, the second argument is the alloc object which must not
// be present explicitly.
- cxxConstructExpr(argumentCountIs(2),
- hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
- hasArgument(1, cxxDefaultArgExpr();
+ cxxConstructExpr(
+ argumentCountIs(2),
+ hasDeclaration(cxxMethodDecl(hasName("basic_string"))),
+ hasArgument(1, ignoringParenImpCasts(cxxDefaultArgExpr());
// Detect passing a suspicious string literal to a string constructor.
// example: std::string str = "abc\0def";
- Finder->addMatcher(traverse(TK_AsIs,
- cxxConstructExpr(StringConstructorExpr, hasArgument(0, StrLitWithNul))),
+ Finder->addMatcher(
+ traverse(TK_AsIs, cxxConstructExpr(StringConstructorExpr,
+ hasArgument(0, ignoringParenImpCasts(
+StrLitWithNul,
5chmidti wrote:
The `StrLitWithNul` already ignores these implicit nodes, so the
`ignoringParenImpCasts` is not needed.
https://github.com/llvm/llvm-project/pull/89553
___
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 ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)
@@ -36,19 +36,21 @@ void
DurationUnnecessaryConversionCheck::registerMatchers(MatchFinder *Finder) {
// e.g. `absl::ToDoubleSeconds(dur)`.
auto InverseFunctionMatcher = callExpr(
callee(functionDecl(hasAnyName(FloatConversion, IntegerConversion))),
-hasArgument(0, expr().bind("arg")));
+hasArgument(0, ignoringParenImpCasts(expr().bind("arg";
// Matcher which matches a duration divided by the factory_matcher above,
// e.g. `dur / absl::Seconds(1)`.
auto DivisionOperatorMatcher = cxxOperatorCallExpr(
-hasOverloadedOperatorName("/"), hasArgument(0, expr().bind("arg")),
-hasArgument(1, FactoryMatcher));
+hasOverloadedOperatorName("/"),
+hasArgument(0, ignoringParenImpCasts(expr().bind("arg"))),
+hasArgument(1, ignoringParenImpCasts(FactoryMatcher)));
// Matcher which matches a duration argument to `FDivDuration`,
// e.g. `absl::FDivDuration(dur, absl::Seconds(1))`
-auto FdivMatcher = callExpr(
-callee(functionDecl(hasName("::absl::FDivDuration"))),
-hasArgument(0, expr().bind("arg")), hasArgument(1, FactoryMatcher));
+auto FdivMatcher =
+callExpr(callee(functionDecl(hasName("::absl::FDivDuration"))),
+ hasArgument(0, ignoringParenImpCasts(expr().bind("arg"))),
+ hasArgument(1, ignoringParenImpCasts(FactoryMatcher)));
5chmidti wrote:
Please remove the `ignoringParentImpCasts` around the `expr().bind("arg")`
matchers, they are not needed.
https://github.com/llvm/llvm-project/pull/89553
___
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 ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)
@@ -98,16 +100,18 @@ void
UpgradeDurationConversionsCheck::registerMatchers(MatchFinder *Finder) {
// `absl::Hours(x)`
// where `x` is not of a built-in type.
Finder->addMatcher(
- traverse(TK_AsIs, implicitCastExpr(
-anyOf(hasCastKind(CK_UserDefinedConversion),
- has(implicitCastExpr(
- hasCastKind(CK_UserDefinedConversion,
-hasParent(callExpr(
-callee(functionDecl(
-DurationFactoryFunction(),
-
unless(hasParent(functionTemplateDecl(),
-hasArgument(0, expr().bind("arg")
-.bind("OuterExpr")),
+ traverse(
+ TK_AsIs,
+ implicitCastExpr(
+ anyOf(
+ hasCastKind(CK_UserDefinedConversion),
+
has(implicitCastExpr(hasCastKind(CK_UserDefinedConversion,
+ hasParent(callExpr(
+ callee(
+ functionDecl(DurationFactoryFunction(),
+ unless(hasParent(functionTemplateDecl(),
+ hasArgument(0, ignoringParenImpCasts(expr().bind("arg"))
5chmidti wrote:
This change is not needed, the check does not if implicit nodes are ignored or
not, so we don't need to do the extra work.
https://github.com/llvm/llvm-project/pull/89553
___
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 ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)
@@ -74,7 +74,7 @@ RewriteRuleWith StringviewNullptrCheckImpl() {
auto BasicStringViewConstructingFromNullExpr =
cxxConstructExpr(
HasBasicStringViewType, argumentCountIs(1),
- hasAnyArgument(/* `hasArgument` would skip over parens */ anyOf(
+ hasArgument(/* `hasArgument` would skip over parens */ anyOf(
5chmidti wrote:
Please remove the comment in all three matchers regarding `hasArgument`
https://github.com/llvm/llvm-project/pull/89553
___
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 ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)
@@ -54,9 +54,10 @@ makeRewriteRule(ArrayRef StringLikeClassNames,
hasParameter(
0, parmVarDecl(anyOf(hasType(StringType), hasType(CharStarType),
hasType(CharType)),
- on(hasType(StringType)), hasArgument(0,
expr().bind("parameter_to_find")),
- anyOf(hasArgument(1, integerLiteral(equals(0))),
-hasArgument(1, cxxDefaultArgExpr())),
+ on(hasType(StringType)),
+ hasArgument(0, ignoringParenImpCasts(expr().bind("parameter_to_find"))),
5chmidti wrote:
Please remove the `ignoringParenImpCasts` from the `parameter_to_find` `expr`,
it is not needed for this check.
https://github.com/llvm/llvm-project/pull/89553
___
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 ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)
@@ -49,7 +49,8 @@ void
UpgradeDurationConversionsCheck::registerMatchers(MatchFinder *Finder) {
hasParent(functionTemplateDecl()),
unless(hasTemplateArgument(0, refersToType(builtinType(,
hasAnyName("operator*=", "operator/="))),
- argumentCountIs(1), hasArgument(0, expr().bind("arg")))
+ argumentCountIs(1),
+ hasArgument(0, ignoringParenImpCasts(expr().bind("arg"
5chmidti wrote:
This change is not needed, the check does not if implicit nodes are ignored or
not, so we don't need to do the extra work.
https://github.com/llvm/llvm-project/pull/89553
___
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 ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)
@@ -62,9 +63,9 @@ void StringFindStartswithCheck::registerMatchers(MatchFinder
*Finder) {
callee(cxxMethodDecl(hasName("rfind")).bind("findfun")),
on(hasType(StringType)),
// ... with some search expression ...
- hasArgument(0, expr().bind("needle")),
+ hasArgument(0, expr().ignoringParenImpCasts().bind("needle")),
5chmidti wrote:
Please remove the `ignoringParenImpCasts` from this `needle` as well.
https://github.com/llvm/llvm-project/pull/89553
___
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 ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)
@@ -21,9 +21,10 @@ void DurationSubtractionCheck::registerMatchers(MatchFinder
*Finder) {
Finder->addMatcher(
binaryOperator(
hasOperatorName("-"),
- hasLHS(callExpr(callee(functionDecl(DurationConversionFunction())
- .bind("function_decl")),
- hasArgument(0, expr().bind("lhs_arg")
+ hasLHS(callExpr(
+ callee(functionDecl(DurationConversionFunction())
+ .bind("function_decl")),
+ hasArgument(0, ignoringParenImpCasts(expr().bind("lhs_arg"))
5chmidti wrote:
This change is not needed, the check does not if implicit nodes are ignored or
not, so we don't need to do the extra work.
https://github.com/llvm/llvm-project/pull/89553
___
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 ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)
@@ -75,10 +75,11 @@ rewriteInverseDurationCall(const MatchFinder::MatchResult
,
getDurationInverseForScale(Scale);
if (const auto *MaybeCallArg = selectFirst(
"e",
- match(callExpr(callee(functionDecl(hasAnyName(
- InverseFunctions.first,
InverseFunctions.second))),
- hasArgument(0, expr().bind("e"))),
-Node, *Result.Context))) {
+ match(
+ callExpr(callee(functionDecl(hasAnyName(
+ InverseFunctions.first, InverseFunctions.second))),
+ hasArgument(0,
ignoringParenImpCasts(expr().bind("e",
5chmidti wrote:
This change is not needed, the check does not care if implicit nodes are
ignored or not, so we don't need to do the extra work, and it will keep the
user's parentheses in-place, if they wrote them.
https://github.com/llvm/llvm-project/pull/89553
___
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 ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)
@@ -20,7 +20,7 @@ namespace clang::tidy::abseil {
void DurationConversionCastCheck::registerMatchers(MatchFinder *Finder) {
auto CallMatcher = ignoringImpCasts(callExpr(
callee(functionDecl(DurationConversionFunction()).bind("func_decl")),
- hasArgument(0, expr().bind("arg";
+ hasArgument(0, ignoringParenImpCasts(expr().bind("arg");
5chmidti wrote:
This change is not needed, the check does not care if implicit nodes are
ignored or not, so we don't need to do the extra work. (This will leave in
place the user's parentheses around the argument, if they wrote any)
https://github.com/llvm/llvm-project/pull/89553
___
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 ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)
@@ -93,7 +94,8 @@ rewriteInverseTimeCall(const MatchFinder::MatchResult ,
llvm::StringRef InverseFunction = getTimeInverseForScale(Scale);
if (const auto *MaybeCallArg = selectFirst(
"e", match(callExpr(callee(functionDecl(hasName(InverseFunction))),
- hasArgument(0, expr().bind("e"))),
+ hasArgument(
+ 0, ignoringParenImpCasts(expr().bind("e",
5chmidti wrote:
Same
https://github.com/llvm/llvm-project/pull/89553
___
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 ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/89553 ___ 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 ``ignoringParenImpCasts`` in arguments of hasArgument (PR #89553)
https://github.com/5chmidti requested changes to this pull request. Thanks for looking into this. Because this is touching a lot of checks, there was bound to be some conversation about which matchers need the `ignoringParenImpCasts` and which don't. I think we should check that now instead of later, so don't be alarmed about the number of comments. I'm just trying to make sure we only add `ignoringParenImpCasts` where it is needed. Maybe other reviewers can chime in about this and confirm my comments before you act on them, in case there are any that are incorrect or if we defer this for a later cleanup pr. That way, you don't do work that is potentially undone again. I added these comments by only looking at these matchers. While I hope all are correct, there may be some that are not, so let me know. Those matchers(/checks) do not care about the implicit nodes or only care about a type in a certain way, so they can be removed. However, there are some, where I think that removing the `ignoringParenImpCasts` may actually fix issues related to extra parentheses added by the user, if there are fix-it's involved (e.g., `UseStartsEndsWithChecl.cpp`). Please add a release note to: https://github.com/llvm/llvm-project/blob/main/clang/docs/ReleaseNotes.rst#ast-matchers about the change to the AST matcher. The changes to the matchers (in e.g. clang-tidy checks) don't need a release note, because they should be essentially non-functional changes (IMO). w.r.t. formatting: The general stance on formatting in LLVM is to only format in the vicinity of a committed change (think 'only the line +- a few lines if it makes sense'). Please clean up unrelated formatting changes from this pr. Check your editor's settings regarding formatting. There should probably be a setting available to only format changed lines (which you should also be able to only enable for LLVM instead of all of your projects). Regarding the failing CI: There are more consumers of AST matchers than just clang-tidy checks. If you search through the failure log, you'll see which tests are failing (or checkout this: https://github.com/llvm/llvm-project/issues/75754#issuecomment-1913568669), and you can backtrack from there. You could also search for `hasArgument` outside the `clang-tidy` folder. The relevant tests to run should be: `ninja check-clang check-clang-tools` to get everything. It is also what the CI runs (minus not relevant tests to this pr). https://github.com/llvm/llvm-project/pull/89553 ___ 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 false-negative for macros in `readability-math-missing-parentheses` (PR #90279)
5chmidti wrote: CC @11happy (couldn't add you as a reviewer) https://github.com/llvm/llvm-project/pull/90279 ___ 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 false-negative for macros in `readability-math-missing-parentheses` (PR #90279)
https://github.com/5chmidti created
https://github.com/llvm/llvm-project/pull/90279
When a binary operator is the last operand of a macro, the end location
that is past the `BinaryOperator` will be inside the macro and therefore an
invalid location to insert a `FixIt` into, which is why the check bails
when encountering such a pattern.
However, the end location is only required for the `FixIt` and the
diagnostic can still be emitted, just without an attached fix.
>From 0b6b64b1ab203e037d55839e84ca31b8d0230ae6 Mon Sep 17 00:00:00 2001
From: Julian Schmidt
Date: Fri, 26 Apr 2024 23:42:40 +0200
Subject: [PATCH] [clang-tidy] fix false-negative for macros in
`readability-math-missing-parentheses`
When a binary operator is the last operand of a macro, the end location
that is past the `BinaryOperator` will be inside the macro and therefore an
invalid location to insert a `FixIt` into, which is why the check bails
when encountering such a pattern.
However, the end location is only required for the `FixIt` and the
diagnostic can still be emitted, just without an attached fix.
---
.../MathMissingParenthesesCheck.cpp | 16 --
.../readability/math-missing-parentheses.cpp | 22 +++
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp
b/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp
index d1e20b9074cec1..65fd296094915b 100644
--- a/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/MathMissingParenthesesCheck.cpp
@@ -61,19 +61,21 @@ static void addParantheses(const BinaryOperator *BinOp,
const clang::SourceLocation StartLoc = BinOp->getBeginLoc();
const clang::SourceLocation EndLoc =
clang::Lexer::getLocForEndOfToken(BinOp->getEndLoc(), 0, SM, LangOpts);
-if (EndLoc.isInvalid())
- return;
-Check->diag(StartLoc,
-"'%0' has higher precedence than '%1'; add parentheses to "
-"explicitly specify the order of operations")
+auto Diag =
+Check->diag(StartLoc,
+"'%0' has higher precedence than '%1'; add parentheses to "
+"explicitly specify the order of operations")
<< (Precedence1 > Precedence2 ? BinOp->getOpcodeStr()
: ParentBinOp->getOpcodeStr())
<< (Precedence1 > Precedence2 ? ParentBinOp->getOpcodeStr()
: BinOp->getOpcodeStr())
-<< FixItHint::CreateInsertion(StartLoc, "(")
-<< FixItHint::CreateInsertion(EndLoc, ")")
<< SourceRange(StartLoc, EndLoc);
+
+if (EndLoc.isValid()) {
+ Diag << FixItHint::CreateInsertion(StartLoc, "(")
+ << FixItHint::CreateInsertion(EndLoc, ")");
+}
}
addParantheses(dyn_cast(BinOp->getLHS()->IgnoreImpCasts()),
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp
b/clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp
index edbe2e1c37c770..a6045c079a4823 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/readability/math-missing-parentheses.cpp
@@ -16,6 +16,13 @@ int bar(){
return 4;
}
+int sink(int);
+#define FUN(ARG) (sink(ARG))
+#define FUN2(ARG) sink((ARG))
+#define FUN3(ARG) sink(ARG)
+#define FUN4(ARG) sink(1 + ARG)
+#define FUN5(ARG) sink(4 * ARG)
+
class fun{
public:
int A;
@@ -117,4 +124,19 @@ void f(){
//CHECK-MESSAGES: :[[@LINE+2]]:94: warning: '/' has higher precedence than
'-'; add parentheses to explicitly specify the order of operations
[readability-math-missing-parentheses]
//CHECK-FIXES: int q = (1 MACRO_ADD (2 MACRO_MULTIPLY 3)) MACRO_OR ((4
MACRO_AND 5) MACRO_XOR (6 MACRO_SUBTRACT (7 MACRO_DIVIDE 8)));
int q = 1 MACRO_ADD 2 MACRO_MULTIPLY 3 MACRO_OR 4 MACRO_AND 5 MACRO_XOR 6
MACRO_SUBTRACT 7 MACRO_DIVIDE 8; // No warning
+
+//CHECK-MESSAGES: :[[@LINE+1]]:21: warning: '*' has higher precedence than
'+'; add parentheses to explicitly specify the order of operations
[readability-math-missing-parentheses]
+int r = FUN(0 + 1 * 2);
+
+//CHECK-MESSAGES: :[[@LINE+1]]:22: warning: '*' has higher precedence than
'+'; add parentheses to explicitly specify the order of operations
[readability-math-missing-parentheses]
+int s = FUN2(0 + 1 * 2);
+
+//CHECK-MESSAGES: :[[@LINE+1]]:22: warning: '*' has higher precedence than
'+'; add parentheses to explicitly specify the order of operations
[readability-math-missing-parentheses]
+int t = FUN3(0 + 1 * 2);
+
+//CHECK-MESSAGES: :[[@LINE+1]]:18: warning: '*' has higher precedence than
'+'; add parentheses to explicitly specify the order of operations
[readability-math-missing-parentheses]
+int u = FUN4(1 * 2);
+
+
[clang-tools-extra] [clang-tidy] fix false-positives for templates in `bugprone-return-const-ref-from-parameter` (PR #90273)
5chmidti wrote: Do we add `improvement` documentation for checks that landed in the same release cycle? https://github.com/llvm/llvm-project/pull/90273 ___ 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 false-positives for templates in `bugprone-return-const-ref-from-parameter` (PR #90273)
5chmidti wrote: The false positives that are fixed by implementing this fix are in lines: 98, 101, 110, 113. The other tests are for completeness. https://github.com/llvm/llvm-project/pull/90273 ___ 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 false-positives for templates in `bugprone-return-const-ref-from-parameter` (PR #90273)
5chmidti wrote: @HerrCai0907 I was looking at false positives for templates regarding this check and ended up implementing the fix. https://github.com/llvm/llvm-project/pull/90273 ___ 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 false-positives for templates in `bugprone-return-const-ref-from-parameter` (PR #90273)
https://github.com/5chmidti created
https://github.com/llvm/llvm-project/pull/90273
In the AST for function templates, the return will be a DeclRefExpr,
even if the return type differs from that of the returned variable.
Protect against false-positives by constraining the canonical return
type to be that of the parameter.
Also streams the source range of the returned expression into the
diagnostic.
>From 9b5bf4e1d53b3a55f9290199aeccb02c20a1e2cc Mon Sep 17 00:00:00 2001
From: Julian Schmidt
Date: Fri, 26 Apr 2024 22:49:38 +0200
Subject: [PATCH] [clang-tidy] fix false-positives for templates in
`bugprone-return-const-ref-from-parameter`
In the AST for function templates, the return will be a DeclRefExpr,
even if the return type differs from that of the returned variable.
Protect against false-positives by constraining the canonical return
type to be that of the parameter.
Also streams the source range of the returned expression into the
diagnostic.
---
.../ReturnConstRefFromParameterCheck.cpp | 10 +-
.../return-const-ref-from-parameter.cpp | 114 ++
2 files changed, 121 insertions(+), 3 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
index 8ae37d4f774d23..b3f7dd6d1c86f8 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
@@ -17,8 +17,11 @@ namespace clang::tidy::bugprone {
void ReturnConstRefFromParameterCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
- returnStmt(hasReturnValue(declRefExpr(to(parmVarDecl(hasType(
- hasCanonicalType(matchers::isReferenceToConst(
+ returnStmt(
+ hasReturnValue(declRefExpr(to(parmVarDecl(hasType(hasCanonicalType(
+ qualType(matchers::isReferenceToConst()).bind("type"))),
+ hasAncestor(functionDecl(hasReturnTypeLoc(
+ loc(qualType(hasCanonicalType(equalsBoundNode("type"
.bind("ret"),
this);
}
@@ -28,7 +31,8 @@ void ReturnConstRefFromParameterCheck::check(
const auto *R = Result.Nodes.getNodeAs("ret");
diag(R->getRetValue()->getBeginLoc(),
"returning a constant reference parameter may cause a use-after-free "
- "when the parameter is constructed from a temporary");
+ "when the parameter is constructed from a temporary")
+ << R->getRetValue()->getSourceRange();
}
} // namespace clang::tidy::bugprone
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp
index a83a019ec7437d..205d93606993e1 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp
@@ -4,6 +4,15 @@ using T = int;
using TConst = int const;
using TConstRef = int const&;
+template
+struct Wrapper { Wrapper(T); };
+
+template
+struct Identity { using type = T; };
+
+template
+struct ConstRef { using type = const T&; };
+
namespace invalid {
int const (int const ) { return a; }
@@ -18,8 +27,59 @@ int const (TConstRef a) { return a; }
int const (TConst ) { return a; }
// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: returning a constant reference
parameter
+template
+const T& tf1(const T ) { return a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: returning a constant reference
parameter
+
+template
+const T& itf1(const T ) { return a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:36: warning: returning a constant reference
parameter
+
+template
+typename ConstRef::type itf2(const T ) { return a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:54: warning: returning a constant reference
parameter
+
+template
+typename ConstRef::type itf3(typename ConstRef::type a) { return a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:72: warning: returning a constant reference
parameter
+
+template
+const T& itf4(typename ConstRef::type a) { return a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:54: warning: returning a constant reference
parameter
+
+void instantiate(const int , const float , int _param, float
_paramf) {
+itf1(0);
+itf1(param);
+itf1(paramf);
+itf2(0);
+itf2(param);
+itf2(paramf);
+itf3(0);
+itf3(param);
+itf3(paramf);
+itf4(0);
+itf4(param);
+itf4(paramf);
+}
+
+struct C {
+const C& foo(const C) { return c; }
+// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: returning a constant reference
parameter
+};
+
} // namespace invalid
+namespace false_negative_because_dependent_and_not_instantiated {
+template
+typename ConstRef::type tf2(const T ) { return a; }
+
+template
+typename ConstRef::type tf3(typename
[clang-tools-extra] [clang-tidy] Ensure nullable variable is not accessed without validity test (PR #90173)
https://github.com/5chmidti approved this pull request. https://github.com/llvm/llvm-project/pull/90173 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add ignoring paren imp casts in has any argument (PR #89509)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/89509 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [tidy] add new check bugprone-return-const-ref-from-parameter (PR #89497)
@@ -0,0 +1,34 @@
+//===--- ReturnConstRefFromParameterCheck.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 "ReturnConstRefFromParameterCheck.h"
+#include "../utils/Matchers.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+void ReturnConstRefFromParameterCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ returnStmt(hasReturnValue(declRefExpr(to(parmVarDecl(hasType(
+ hasCanonicalType(matchers::isReferenceToConst(
+ .bind("ret"),
+ this);
+}
+
+void ReturnConstRefFromParameterCheck::check(
+const MatchFinder::MatchResult ) {
+ const auto *R = Result.Nodes.getNodeAs("ret");
+ diag(R->getRetValue()->getBeginLoc(),
+ "return const reference parameter cause potential use-after-free "
+ "when function accepts immediately constructed value.");
5chmidti wrote:
What do you think about `returning a const reference parameter may cause a
use-after-free when the parameter is constructed from a temporary` (`const` vs
`constant` argument in the other thread aside)
I'm not sure if it should be `may` or `will`, because it is only a
use-after-free if the returned value is actually used (FWICT).
https://github.com/llvm/llvm-project/pull/89497
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [tidy] add new check bugprone-return-const-ref-from-parameter (PR #89497)
@@ -0,0 +1,35 @@
+//===--- ReturnConstRefFromParameterCheck.h - clang-tidy *- C++
-*-===//
+//
+// 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
+//
+//===--===//
+
+#ifndef
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RETURNCONSTREFFROMPARAMETERCHECK_H
+#define
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RETURNCONSTREFFROMPARAMETERCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Detects the function which returns the const reference from parameter which
+/// causes potential use after free if the caller uses xvalue as argument.
5chmidti wrote:
I think `Detects statements that return constant` should actually be `Detects
return statements that return constant`.
As for consistency: in docs/clang-tidy, there are ~116 const's (minus `const`
in code snippets and check names) where ~50% of them are in ``, and there are
91 constants (although 60 are in `identifier-naming.rst`.
`might cause potential` sounds not right to me, the `might` and `potential`
mean the same thing IMO.
I would suggest: `This might cause use-after-free errors ...` (or with `may`
instead of `might`)
https://github.com/llvm/llvm-project/pull/89497
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add ignoring paren imp casts in has any argument (PR #89509)
@@ -90,8 +91,9 @@ RewriteRuleWith StringviewNullptrCheckImpl() {
auto HandleTemporaryCXXTemporaryObjectExprAndCompoundLiteralExpr = makeRule(
cxxTemporaryObjectExpr(cxxConstructExpr(
HasBasicStringViewType, argumentCountIs(1),
- hasAnyArgument(/* `hasArgument` would skip over parens */ anyOf(
- NullLiteral, NullInitList, EmptyInitList)),
+ hasAnyArgument(
+ /* `hasArgument` would skip over parens */ ignoringParenImpCasts(
+ anyOf(NullLiteral, NullInitList, EmptyInitList))),
5chmidti wrote:
Same as above
https://github.com/llvm/llvm-project/pull/89509
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add ignoring paren imp casts in has any argument (PR #89509)
@@ -74,8 +74,9 @@ RewriteRuleWith StringviewNullptrCheckImpl() {
auto BasicStringViewConstructingFromNullExpr =
cxxConstructExpr(
HasBasicStringViewType, argumentCountIs(1),
- hasAnyArgument(/* `hasArgument` would skip over parens */ anyOf(
- NullLiteral, NullInitList, EmptyInitList)),
+ hasAnyArgument(
+ /* `hasArgument` would skip over parens */ ignoringParenImpCasts(
+ anyOf(NullLiteral, NullInitList, EmptyInitList))),
5chmidti wrote:
Checkout this file later for this and the other section I highlighted, here it
looks like the only reason that `hasAnyArgument` was chosen is because of this
differing behavior, instead, this can be replaced with `hasArgument(0, ...)`
after the `IgnoreParenImpCasts` is removed from the `hasArgument` matcher.
https://github.com/llvm/llvm-project/pull/89509
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add ignoring paren imp casts in has any argument (PR #89509)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/89509 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] Add ignoring paren imp casts in has any argument (PR #89509)
https://github.com/5chmidti requested changes to this pull request. I started taking a look at this and realized you switched up which argument matcher needs the extra `ignoringParenImpCasts`, so that it can be removed from the matcher definition. See https://github.com/llvm/llvm-project/blob/d674f45d51bffbba474b12e07f7d57a2390d2f31/clang/include/clang/ASTMatchers/ASTMatchers.h#L4891-L4907 vs https://github.com/llvm/llvm-project/blob/d674f45d51bffbba474b12e07f7d57a2390d2f31/clang/include/clang/ASTMatchers/ASTMatchers.h#L4553-L4564. We don't want the `IgnoreParenImpCasts()` inside the `hasArgument` matcher, because that is not it's job. So we want to a) add `ignoringParenImpCasts` to arguments of the `hasArgument` matcher, and b) remove the call to `IgnoreParenImpCasts` from the `hasArgument` matcher. See this comment from the original issue: https://github.com/llvm/llvm-project/issues/75754#issuecomment-1887818096 This is also the reason why the tests are failing, you are actually changing the behavior of these checks. https://github.com/llvm/llvm-project/pull/89509 ___ 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-lambda-function-name ignore macro in captures (PR #89076)
https://github.com/5chmidti approved this pull request. https://github.com/llvm/llvm-project/pull/89076 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Index] check `TemplateTypeParmTypeLoc::getDecl()` against `nullptr` in `TypeIndexer` and `CursorVisitor` (PR #89392)
5chmidti wrote: CC @hokein https://github.com/llvm/llvm-project/pull/89392 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Index] check `TemplateTypeParmTypeLoc::getDecl()` against `nullptr` in `TypeIndexer` and `CursorVisitor` (PR #89392)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/89392 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Index] check `TemplateTypeParmTypeLoc ::getDecl()` against `nullptr` in `TypeIndexer` and `CursorVisitor` (PR #89392)
https://github.com/5chmidti created
https://github.com/llvm/llvm-project/pull/89392
In the added testcase, which is invalid code, the result of `getDecl()` called
on a `TemplateTypeParmTypeLoc` was
a `nullptr`. However, `IndexingContext::handleReference` expects the parameter
`D` to not be a `nullptr` and
passed it to `isa<>` without checking it. Similarly `MakeCursorTypeRef` expects
`D` to not be a `nullptr` as well.
Fixes: clangd/clangd#2016, #89389
>From 2ebb3ec2f0f654b39283947e39adf182fc799683 Mon Sep 17 00:00:00 2001
From: Julian Schmidt
Date: Fri, 19 Apr 2024 16:41:50 +0200
Subject: [PATCH] [clang][Index] check `TemplateTypeParmTypeLoc ::getDecl()`
against `nullptr` in `TypeIndexer` and `CursorVisitor`
In the added testcase, which is invalid code, the result of `getDecl()` called
on a `TemplateTypeParmTypeLoc` was
a `nullptr`. However, `IndexingContext::handleReference` expects the parameter
`D` to not be a `nullptr` and
passed it to `isa<>` without checking it. Similarly `MakeCursorTypeRef` expects
`D` to not be a `nullptr` as well.
Fixes: clangd/clangd#2016, #89389
---
clang/lib/Index/IndexTypeSourceInfo.cpp | 3 +++
clang/test/Index/gh89389.cpp| 16 +++
clang/tools/libclang/CIndex.cpp | 8 ++--
clang/unittests/Index/IndexTests.cpp| 27 +
4 files changed, 52 insertions(+), 2 deletions(-)
create mode 100644 clang/test/Index/gh89389.cpp
diff --git a/clang/lib/Index/IndexTypeSourceInfo.cpp
b/clang/lib/Index/IndexTypeSourceInfo.cpp
index b986ccde574525..58ad0799062ca3 100644
--- a/clang/lib/Index/IndexTypeSourceInfo.cpp
+++ b/clang/lib/Index/IndexTypeSourceInfo.cpp
@@ -52,6 +52,9 @@ class TypeIndexer : public RecursiveASTVisitor {
bool VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc TTPL) {
SourceLocation Loc = TTPL.getNameLoc();
TemplateTypeParmDecl *TTPD = TTPL.getDecl();
+if (!TTPD)
+ return false;
+
return IndexCtx.handleReference(TTPD, Loc, Parent, ParentDC,
SymbolRoleSet());
}
diff --git a/clang/test/Index/gh89389.cpp b/clang/test/Index/gh89389.cpp
new file mode 100644
index 00..0458d0a64083d8
--- /dev/null
+++ b/clang/test/Index/gh89389.cpp
@@ -0,0 +1,16 @@
+// RUN: c-index-test -test-load-source all %s -std=gnu++20
-fno-delayed-template-parsing
+
+namespace test18 {
+template
+concept False = false;
+
+template struct Foo { T t; };
+
+template requires False
+Foo(T) -> Foo;
+
+template
+using Bar = Foo;
+
+Bar s = {1};
+} // namespace test18
diff --git a/clang/tools/libclang/CIndex.cpp b/clang/tools/libclang/CIndex.cpp
index cbc1d85bb33dfc..8fd723a90e5565 100644
--- a/clang/tools/libclang/CIndex.cpp
+++ b/clang/tools/libclang/CIndex.cpp
@@ -1693,12 +1693,16 @@ bool CursorVisitor::VisitTagTypeLoc(TagTypeLoc TL) {
}
bool CursorVisitor::VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc TL) {
- if (const auto *TC = TL.getDecl()->getTypeConstraint()) {
+ TemplateTypeParmDecl *D = TL.getDecl();
+ if (!D)
+return true;
+
+ if (const auto *TC = D->getTypeConstraint()) {
if (VisitTypeConstraint(*TC))
return true;
}
- return Visit(MakeCursorTypeRef(TL.getDecl(), TL.getNameLoc(), TU));
+ return Visit(MakeCursorTypeRef(D, TL.getNameLoc(), TU));
}
bool CursorVisitor::VisitObjCInterfaceTypeLoc(ObjCInterfaceTypeLoc TL) {
diff --git a/clang/unittests/Index/IndexTests.cpp
b/clang/unittests/Index/IndexTests.cpp
index 8e9a1c6bf88245..0db8a4b98c2025 100644
--- a/clang/unittests/Index/IndexTests.cpp
+++ b/clang/unittests/Index/IndexTests.cpp
@@ -453,6 +453,33 @@ TEST(IndexTest, ReadWriteRoles) {
WrittenAt(Position(6, 17));
}
+TEST(IndexTest, gh89389) {
+ std::string Code = R"cpp(
+namespace test18 {
+template
+concept False = false;
+
+template struct Foo { T t; };
+
+template requires False
+Foo(T) -> Foo;
+
+template
+using Bar = Foo;
+
+Bar s = {1};
+} // namespace test18
+ )cpp";
+ auto Index = std::make_shared();
+ IndexingOptions Opts;
+ Opts.IndexTemplateParameters = true;
+
+ // This test case is invalid code, so the expected return value is `false`.
+ // What is being tested is that there is no crash.
+ EXPECT_FALSE(tooling::runToolOnCodeWithArgs(
+ std::make_unique(Index, Opts), Code, {"-std=c++20"}));
+}
+
} // namespace
} // namespace index
} // namespace clang
___
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 readability-math-missing-parentheses (PR #84481)
5chmidti wrote:
I let it run for a few minutes and didn't observe any crashes (on a release
build though). However, I did find an issue with macros:
```c++
int sink(int);
#define FUN(ARG) (sink(ARG))
#define FUN2(ARG) sink((ARG))
#define FUN3(ARG) sink(ARG)
void f() {
...
//CHECK-MESSAGES: :[[@LINE+4]]:17: warning: '*' has higher precedence than
'+'; add parentheses to explicitly specify the order of operations
[readability-math-missing-parentheses]
//CHECK-FIXES: int r = FUN(0 + (1 * 2));
int r = FUN(0 + 1 * 2);
//CHECK-MESSAGES: :[[@LINE+4]]:17: warning: '*' has higher precedence than
'+'; add parentheses to explicitly specify the order of operations
[readability-math-missing-parentheses]
//CHECK-FIXES: int s = FUN2(0 + (1 * 2));
int s = FUN2(0 + 1 * 2);
//CHECK-MESSAGES: :[[@LINE+4]]:17: warning: '*' has higher precedence than
'+'; add parentheses to explicitly specify the order of operations
[readability-math-missing-parentheses]
//CHECK-FIXES: int s = FUN3(0 + (1 * 2));
int t = FUN3(0 + 1 * 2);
}
```
All three of these fail, because the closing parentheses is not added.
Files with issues:
- `polly/lib/External/isl/isl_map.c`
-
`/home/julian/dev/llvm/llvm-tmp/llvm/unittests/DebugInfo/MSF/MSFBuilderTest.cpp`
- unittest files will generally have issues with this, because of the test
macros
- found by invoking this inside the build dir: `run-clang-tidy
-clang-tidy-binary /path/to/clang-tidy -checks="-*,readability-math-missing*"
-quiet unittests`
Checking `EndLoc.isValid()` reveals that the location is invalid for all of
these cases (`llvm::errs() << "EndLoc: " << EndLoc.isValid() << "\n";`).
The documentation for `getLocForEndOfToken` also explicitly mentions this case
for macros:
```
/// token where it expected something different that it received. If
/// the returned source location would not be meaningful (e.g., if
/// it points into a macro), this routine returns an invalid
/// source location.
```
https://github.com/llvm/llvm-project/pull/84481
___
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-lambda-function-name ignore macro in captures (PR #89076)
@@ -69,9 +73,13 @@ void
LambdaFunctionNameCheck::storeOptions(ClangTidyOptions::OptionMap ) {
}
void LambdaFunctionNameCheck::registerMatchers(MatchFinder *Finder) {
- // Match on PredefinedExprs inside a lambda.
- Finder->addMatcher(predefinedExpr(hasAncestor(lambdaExpr())).bind("E"),
- this);
+ Finder->addMatcher(
+ functionDecl(cxxMethodDecl(isInLambda()),
+ hasBody(hasDescendant(expr(
+ predefinedExpr(hasAncestor(functionDecl().bind("fn")))
+ .bind("E",
+ functionDecl(equalsBoundNode("fn"))),
+ this);
5chmidti wrote:
If you make the outer `functionDecl` the `cxxMethodDecl`, then you can remove
the inner `cxxMethodDecl` that encloses the lambda. Or was there a reason for
doing it this way?
You can also drop the `expr` matcher that surrounds the `predefinedExpr`, and
the `functionDecl` surrounding the `equalsBoundNode`.
I.e.:
```c++
cxxMethodDecl(isInLambda(),
hasBody(hasDescendant(
predefinedExpr(hasAncestor(functionDecl().bind("fn")))
.bind("E"))),
equalsBoundNode("fn"))
```
https://github.com/llvm/llvm-project/pull/89076
___
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-lambda-function-name ignore macro in captures (PR #89076)
@@ -151,6 +151,10 @@ Changes in existing checks ` check to ignore code within unevaluated contexts, such as ``decltype``. +- Improved :doc:`bugprone-lambda-function-name + ` check by ignoring + ``__func__`` macro in lambda captures and nested function declaration. 5chmidti wrote: I think the default parameter part should be included as well (+nits): `check by ignoring the ``__func__`` macro in lambda captures, initializers of default parameters and nested function declarations.` https://github.com/llvm/llvm-project/pull/89076 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] use existing function for code locations in the scopify enum tweak (PR #88737)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/88737 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Extract Function: add hoisting support (PR #75533)
https://github.com/5chmidti updated
https://github.com/llvm/llvm-project/pull/75533
>From b8f3f364310ca6094a6944f4f3ee9349c8aa77d6 Mon Sep 17 00:00:00 2001
From: Julian Schmidt <44101708+5chmi...@users.noreply.github.com>
Date: Sat, 21 Jan 2023 14:49:58 +0100
Subject: [PATCH 1/3] [clangd] Extract Function: add hoisting support
Adds support to hoist variables declared inside the selected region
and used afterwards back out of the extraced function for later use.
Uses the explicit variable type if only one decl needs hoisting,
otherwise uses std::pair or std::tuple with auto return type
deduction (requires c++14) and a structured binding (requires c++17)
or explicitly unpacking the variables with get<>.
---
.../refactor/tweaks/ExtractFunction.cpp | 159 +--
.../unittests/tweaks/ExtractFunctionTests.cpp | 393 +-
clang-tools-extra/docs/ReleaseNotes.rst | 3 +
3 files changed, 523 insertions(+), 32 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
index 0302839c58252e..02d1a6d0996a53 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -79,6 +79,13 @@ namespace {
using Node = SelectionTree::Node;
+struct HoistSetComparator {
+ bool operator()(const Decl *const Lhs, const Decl *const Rhs) const {
+return Lhs->getLocation() < Rhs->getLocation();
+ }
+};
+using HoistSet = llvm::SmallSet;
+
// ExtractionZone is the part of code that is being extracted.
// EnclosingFunction is the function/method inside which the zone lies.
// We split the file into 4 parts relative to extraction zone.
@@ -171,12 +178,13 @@ struct ExtractionZone {
// semicolon after the extraction.
const Node *getLastRootStmt() const { return Parent->Children.back(); }
- // Checks if declarations inside extraction zone are accessed afterwards.
+ // Checks if declarations inside extraction zone are accessed afterwards and
+ // adds these declarations to the returned set.
//
// This performs a partial AST traversal proportional to the size of the
// enclosing function, so it is possibly expensive.
- bool requiresHoisting(const SourceManager ,
-const HeuristicResolver *Resolver) const {
+ HoistSet getDeclsToHoist(const SourceManager ,
+ const HeuristicResolver *Resolver) const {
// First find all the declarations that happened inside extraction zone.
llvm::SmallSet DeclsInExtZone;
for (auto *RootStmt : RootStmts) {
@@ -191,29 +199,28 @@ struct ExtractionZone {
}
// Early exit without performing expensive traversal below.
if (DeclsInExtZone.empty())
- return false;
-// Then make sure they are not used outside the zone.
+ return {};
+// Add any decl used after the selection to the returned set
+HoistSet DeclsToHoist{};
for (const auto *S : EnclosingFunction->getBody()->children()) {
if (SM.isBeforeInTranslationUnit(S->getSourceRange().getEnd(),
ZoneRange.getEnd()))
continue;
- bool HasPostUse = false;
findExplicitReferences(
S,
[&](const ReferenceLoc ) {
-if (HasPostUse ||
-SM.isBeforeInTranslationUnit(Loc.NameLoc, ZoneRange.getEnd()))
+if (SM.isBeforeInTranslationUnit(Loc.NameLoc, ZoneRange.getEnd()))
return;
-HasPostUse = llvm::any_of(Loc.Targets,
- [](const Decl *Target) {
-return DeclsInExtZone.contains(Target);
- });
+for (const NamedDecl *const PostUse : llvm::make_filter_range(
+ Loc.Targets, [](const Decl *Target) {
+ return DeclsInExtZone.contains(Target);
+ })) {
+ DeclsToHoist.insert(PostUse);
+}
},
Resolver);
- if (HasPostUse)
-return true;
}
-return false;
+return DeclsToHoist;
}
};
@@ -367,14 +374,17 @@ struct NewFunction {
bool Static = false;
ConstexprSpecKind Constexpr = ConstexprSpecKind::Unspecified;
bool Const = false;
+ const HoistSet
// Decides whether the extracted function body and the function call need a
// semicolon after extraction.
tooling::ExtractionSemicolonPolicy SemicolonPolicy;
const LangOptions *LangOpts;
- NewFunction(tooling::ExtractionSemicolonPolicy SemicolonPolicy,
+ NewFunction(const HoistSet ,
+ tooling::ExtractionSemicolonPolicy SemicolonPolicy,
const LangOptions *LangOpts)
- : SemicolonPolicy(SemicolonPolicy), LangOpts(LangOpts) {}
+ : ToHoist(ToHoist), SemicolonPolicy(SemicolonPolicy), LangOpts(LangOpts)
{
+ }
// Render the call for this function.
[clang-tools-extra] [clangd] use existing function for code locations in the scopify enum tweak (PR #88737)
5chmidti wrote: @ckandeler while opening up your other changes in my editor, I wanted to see if some of the logic around `Position`s and `Offset`s could be simplified. It looks like `clangd` already implements some of the functionality in the `SourceCode.h` file. This shouldn't actually change the behavior, so I didn't add a release note. https://github.com/llvm/llvm-project/pull/88737 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] use existing function for code locations in the scopify enum tweak (PR #88737)
https://github.com/5chmidti created
https://github.com/llvm/llvm-project/pull/88737
Clangd already implements some utility functions for converting between
SourceLocations, Positions and Offsets into a buffer.
>From 3ee0dfe6292146d188f7d14f717c1e989c668e1c Mon Sep 17 00:00:00 2001
From: Julian Schmidt
Date: Mon, 15 Apr 2024 15:43:07 +0200
Subject: [PATCH] [clangd] use existing function for code locations in the
scopify enum tweak
Clangd already implements some utility functions for converting between
SourceLocations, Positions and Offsets into a buffer.
---
.../clangd/refactor/tweaks/ScopifyEnum.cpp| 38 ++-
1 file changed, 11 insertions(+), 27 deletions(-)
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
b/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
index e36b3249bc7b92..0c51c1b39a8401 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ScopifyEnum.cpp
@@ -65,12 +65,10 @@ class ScopifyEnum : public Tweak {
llvm::Error scopifyEnumValues();
llvm::Error scopifyEnumValue(const EnumConstantDecl , StringRef Prefix);
llvm::Expected getContentForFile(StringRef FilePath);
- unsigned getOffsetFromPosition(const Position , StringRef Content) const;
llvm::Error addReplacementForReference(const ReferencesResult::Reference
,
const MakeReplacement
);
llvm::Error addReplacement(StringRef FilePath, StringRef Content,
const tooling::Replacement );
- Position getPosition(const Decl ) const;
const EnumDecl *D = nullptr;
const Selection *S = nullptr;
@@ -109,7 +107,8 @@ Expected ScopifyEnum::apply(const Selection
) {
llvm::Error ScopifyEnum::addClassKeywordToDeclarations() {
for (const auto :
- findReferences(*S->AST, getPosition(*D), 0, S->Index, false)
+ findReferences(*S->AST, sourceLocToPosition(*SM, D->getBeginLoc()), 0,
+ S->Index, false)
.References) {
if (!(Ref.Attributes & ReferencesResult::Declaration))
continue;
@@ -137,7 +136,8 @@ llvm::Error ScopifyEnum::scopifyEnumValues() {
llvm::Error ScopifyEnum::scopifyEnumValue(const EnumConstantDecl ,
StringRef Prefix) {
for (const auto :
- findReferences(*S->AST, getPosition(CD), 0, S->Index, false)
+ findReferences(*S->AST, sourceLocToPosition(*SM, CD.getBeginLoc()), 0,
+ S->Index, false)
.References) {
if (Ref.Attributes & ReferencesResult::Declaration)
continue;
@@ -187,27 +187,19 @@ llvm::Expected
ScopifyEnum::getContentForFile(StringRef FilePath) {
return Content;
}
-unsigned int ScopifyEnum::getOffsetFromPosition(const Position ,
-StringRef Content) const {
- unsigned int Offset = 0;
-
- for (std::size_t LinesRemaining = Pos.line;
- Offset < Content.size() && LinesRemaining;) {
-if (Content[Offset++] == '\n')
- --LinesRemaining;
- }
- return Offset + Pos.character;
-}
-
llvm::Error
ScopifyEnum::addReplacementForReference(const ReferencesResult::Reference ,
const MakeReplacement )
{
StringRef FilePath = Ref.Loc.uri.file();
- auto Content = getContentForFile(FilePath);
+ llvm::Expected Content = getContentForFile(FilePath);
if (!Content)
return Content.takeError();
- unsigned Offset = getOffsetFromPosition(Ref.Loc.range.start, *Content);
- tooling::Replacement Replacement = GetReplacement(FilePath, *Content,
Offset);
+ llvm::Expected Offset =
+ positionToOffset(*Content, Ref.Loc.range.start);
+ if (!Offset)
+return Offset.takeError();
+ tooling::Replacement Replacement =
+ GetReplacement(FilePath, *Content, *Offset);
if (Replacement.isApplicable())
return addReplacement(FilePath, *Content, Replacement);
return llvm::Error::success();
@@ -223,13 +215,5 @@ ScopifyEnum::addReplacement(StringRef FilePath, StringRef
Content,
return llvm::Error::success();
}
-Position ScopifyEnum::getPosition(const Decl ) const {
- const SourceLocation Loc = D.getLocation();
- Position Pos;
- Pos.line = SM->getSpellingLineNumber(Loc) - 1;
- Pos.character = SM->getSpellingColumnNumber(Loc) - 1;
- return Pos;
-}
-
} // namespace
} // namespace clang::clangd
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/83412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
https://github.com/5chmidti commented: I think this change could use a release note in the docs, could you please add one? https://github.com/llvm/llvm-project/pull/83412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
@@ -125,25 +123,42 @@ llvm::Error ScopifyEnum::addClassKeywordToDeclarations() {
}
llvm::Error ScopifyEnum::scopifyEnumValues() {
- std::string PrefixToInsert(D->getName());
- PrefixToInsert += "::";
+ StringRef EnumName(D->getName());
+ bool StripPrefix = true;
+ for (auto E : D->enumerators()) {
5chmidti wrote:
Please use `const EnumConstantDecl *` instead of `auto`.
https://github.com/llvm/llvm-project/pull/83412
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Remove potential prefix from enum value names (PR #83412)
@@ -125,25 +123,42 @@ llvm::Error ScopifyEnum::addClassKeywordToDeclarations() {
}
llvm::Error ScopifyEnum::scopifyEnumValues() {
- std::string PrefixToInsert(D->getName());
- PrefixToInsert += "::";
+ StringRef EnumName(D->getName());
+ bool StripPrefix = true;
+ for (auto E : D->enumerators()) {
+if (!E->getName().starts_with(EnumName)) {
+ StripPrefix = false;
+ break;
+}
+ }
for (auto E : D->enumerators()) {
5chmidti wrote:
This is not in your changed lines, but please change this `auto` to `const
EnumConstantDecl *` as well.
https://github.com/llvm/llvm-project/pull/83412
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
https://github.com/5chmidti approved this pull request. Looks good to me (others are still reviewing), thanks https://github.com/llvm/llvm-project/pull/85572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,255 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang;
+
+namespace {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ SmallVector Args;
+};
+
+} // anonymous namespace
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+static FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ // check if the function has initializer list argument
+ if (Call->getNumArgs() < 3) {
+auto ArgIterator = Call->arguments().begin();
+
+const auto *InitListExpr =
+dyn_cast(*ArgIterator);
+const auto *InitList =
+InitListExpr != nullptr
+? dyn_cast(
+ InitListExpr->getSubExpr()->IgnoreImplicit())
+: nullptr;
+
+if (InitList) {
+ Result.Args.append(InitList->inits().begin(), InitList->inits().end());
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ // check if there is a comparison argument
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end())
+Result.Compare = *ArgIterator;
+
+ return Result;
+}
+ } else {
+// if it has 3 arguments then the last will be the comparison
+Result.Compare = *(std::next(Call->arguments().begin(), 2));
+ }
+
+ if (Result.Compare)
+Result.Args = SmallVector(llvm::drop_end(Call->arguments()));
+ else
+Result.Args = SmallVector(Call->arguments());
+
+ Result.First = Result.Args.front();
+ Result.Last = Result.Args.back();
+
+ return Result;
+}
+
+static SmallVector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult ) {
+ SmallVector FixItHints;
+
+ const QualType ResultType = TopCall->getDirectCallee()
+ ->getReturnType()
+ .getNonReferenceType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+ const SourceManager = *Match.SourceManager;
+ const LangOptions = Match.Context->getLangOpts();
+
+ for (const Expr *Arg : Result.Args) {
+const auto *InnerCall = dyn_cast(Arg->IgnoreParenImpCasts());
+
+// If the argument is not a nested call
+if (!InnerCall) {
+ // check if typecast is required
+ const QualType ArgType = Arg->IgnoreParenImpCasts()
+ ->getType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+
+ if (ArgType == ResultType)
+continue;
+
+ const StringRef ArgText = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Arg->getSourceRange()), SourceMngr,
+ LanguageOpts);
+
+ Twine Replacement = llvm::Twine("static_cast<")
+ .concat(ResultType.getAsString(LanguageOpts))
+ .concat(">(")
+ .concat(ArgText)
+ .concat(")");
+
+ FixItHints.push_back(FixItHint::CreateReplacement(Arg->getSourceRange(),
+Replacement.str()));
+
+ continue;
+}
+
+const FindArgsResult InnerResult = findArgs(InnerCall);
+
+// if the nested call doesn't have arguments skip it
+if (!InnerResult.First || !InnerResult.Last)
+ continue;
+
+// if the nested call is not the same as the top call
+if (InnerCall->getDirectCallee()->getQualifiedNameAsString() !=
+TopCall->getDirectCallee()->getQualifiedNameAsString())
+ continue;
+
+// if the nested call doesn't have the same compare function
+if ((Result.Compare || InnerResult.Compare) &&
+!utils::areStatementsIdentical(Result.Compare, InnerResult.Compare,
+ *Match.Context))
+ continue;
+
+// remove the function call
+FixItHints.push_back(
+FixItHint::CreateRemoval(InnerCall->getCallee()->getSourceRange()));
+
+// remove the parentheses
+const auto LParen = utils::lexer::findNextTokenSkippingComments(
+InnerCall->getCallee()->getEndLoc(), SourceMngr, LanguageOpts);
+FixItHints.push_back(
+
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/85572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,278 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang;
+
+namespace {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ SmallVector Args;
+};
+
+} // anonymous namespace
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+static FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ // check if the function has initializer list argument
+ if (Call->getNumArgs() < 3) {
+auto ArgIterator = Call->arguments().begin();
+
+const auto *InitListExpr =
+dyn_cast(*ArgIterator);
+const auto *InitList =
+InitListExpr != nullptr
+? dyn_cast(
+ InitListExpr->getSubExpr()->IgnoreImplicit())
+: nullptr;
+
+if (InitListExpr && InitList) {
+ Result.Args.insert(Result.Args.begin(), InitList->inits().begin(),
+ InitList->inits().end());
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ // check if there is a comparison argument
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+Result.Compare = *ArgIterator;
+ }
+
+ return Result;
+}
+// if it has 3 arguments then the last will be the comparison
+ } else {
+Result.Compare = *(std::next(Call->arguments().begin(), 2));
+ }
+
+ for (const Expr *Arg : Call->arguments()) {
+if (!Result.First)
+ Result.First = Arg;
+
+if (Arg == Result.Compare)
+ continue;
+
+Result.Args.push_back(Arg);
+Result.Last = Arg;
+ }
+
+ return Result;
+}
+
+static SmallVector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult ) {
+ SmallVector FixItHints;
+
+ const QualType ResultType = TopCall->getDirectCallee()
+ ->getReturnType()
+ .getNonReferenceType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+ const auto = *Match.SourceManager;
+ const auto LanguageOpts = Match.Context->getLangOpts();
+ const bool IsInitializerList = Result.First == Result.Last;
+
+ // add { and } if the top call doesn't have an initializer list arg
+ if (!IsInitializerList) {
+FixItHints.push_back(
+FixItHint::CreateInsertion(Result.First->getBeginLoc(), "{"));
+
+if (Result.Compare)
+ FixItHints.push_back(FixItHint::CreateInsertion(
+ Lexer::getLocForEndOfToken(Result.Last->getEndLoc(), 0, SourceMngr,
+ LanguageOpts),
5chmidti wrote:
You're right. Maybe I switched something up.
https://github.com/llvm/llvm-project/pull/85572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,255 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang;
+
+namespace {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ SmallVector Args;
+};
+
+} // anonymous namespace
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+static FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ // check if the function has initializer list argument
+ if (Call->getNumArgs() < 3) {
+auto ArgIterator = Call->arguments().begin();
+
+const auto *InitListExpr =
+dyn_cast(*ArgIterator);
+const auto *InitList =
+InitListExpr != nullptr
+? dyn_cast(
+ InitListExpr->getSubExpr()->IgnoreImplicit())
+: nullptr;
+
+if (InitList) {
+ Result.Args.append(InitList->inits().begin(), InitList->inits().end());
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ // check if there is a comparison argument
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end())
+Result.Compare = *ArgIterator;
+
+ return Result;
+}
+ } else {
+// if it has 3 arguments then the last will be the comparison
+Result.Compare = *(std::next(Call->arguments().begin(), 2));
+ }
+
+ if (Result.Compare)
+Result.Args = SmallVector(llvm::drop_end(Call->arguments()));
+ else
+Result.Args = SmallVector(Call->arguments());
+
+ Result.First = Result.Args.front();
+ Result.Last = Result.Args.back();
+
+ return Result;
+}
+
+static SmallVector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult ) {
+ SmallVector FixItHints;
+
+ const QualType ResultType = TopCall->getDirectCallee()
+ ->getReturnType()
+ .getNonReferenceType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+ const SourceManager = *Match.SourceManager;
+ const LangOptions = Match.Context->getLangOpts();
+
+ for (const Expr *Arg : Result.Args) {
+const auto *InnerCall = dyn_cast(Arg->IgnoreParenImpCasts());
+
+// If the argument is not a nested call
+if (!InnerCall) {
+ // check if typecast is required
+ const QualType ArgType = Arg->IgnoreParenImpCasts()
+ ->getType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+
+ if (ArgType == ResultType)
+continue;
+
+ const StringRef ArgText = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Arg->getSourceRange()), SourceMngr,
+ LanguageOpts);
+
+ Twine Replacement = llvm::Twine("static_cast<")
+ .concat(ResultType.getAsString(LanguageOpts))
+ .concat(">(")
+ .concat(ArgText)
+ .concat(")");
+
+ FixItHints.push_back(FixItHint::CreateReplacement(Arg->getSourceRange(),
+Replacement.str()));
+
+ continue;
+}
+
+const FindArgsResult InnerResult = findArgs(InnerCall);
+
+// if the nested call doesn't have arguments skip it
+if (!InnerResult.First || !InnerResult.Last)
+ continue;
+
+// if the nested call is not the same as the top call
+if (InnerCall->getDirectCallee()->getQualifiedNameAsString() !=
+TopCall->getDirectCallee()->getQualifiedNameAsString())
+ continue;
+
+// if the nested call doesn't have the same compare function
+if ((Result.Compare || InnerResult.Compare) &&
+!utils::areStatementsIdentical(Result.Compare, InnerResult.Compare,
+ *Match.Context))
+ continue;
+
+// remove the function call
+FixItHints.push_back(
+FixItHint::CreateRemoval(InnerCall->getCallee()->getSourceRange()));
+
+// remove the parentheses
+const auto LParen = utils::lexer::findNextTokenSkippingComments(
+InnerCall->getCallee()->getEndLoc(), SourceMngr, LanguageOpts);
+FixItHints.push_back(
+
[clang-tools-extra] Add clang-tidy check readability-math-missing-parentheses (PR #84481)
@@ -0,0 +1,97 @@
+//===--- MathMissingParenthesesCheck.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 "MathMissingParenthesesCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+void MathMissingParenthesesCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(binaryOperator(unless(hasParent(binaryOperator())),
+unless(isAssignmentOperator()),
+unless(isComparisonOperator()),
+unless(hasAnyOperatorName("&&", "||")),
+hasDescendant(binaryOperator()))
+ .bind("binOp"),
+ this);
+}
+
+static int getPrecedence(const BinaryOperator *BinOp) {
+ if (!BinOp)
+return 0;
+ switch (BinOp->getOpcode()) {
+ case BO_Mul:
+ case BO_Div:
+ case BO_Rem:
+return 5;
+ case BO_Add:
+ case BO_Sub:
+return 4;
+ case BO_And:
+return 3;
+ case BO_Xor:
+return 2;
+ case BO_Or:
+return 1;
+ default:
+return 0;
+ }
+}
+static void addParantheses(const BinaryOperator *BinOp,
+ const BinaryOperator *ParentBinOp,
+ ClangTidyCheck *Check,
+ const clang::SourceManager ,
+ const clang::LangOptions ) {
+ if (!BinOp)
+return;
+
+ int Precedence1 = getPrecedence(BinOp);
+ int Precedence2 = getPrecedence(ParentBinOp);
+
+ if (ParentBinOp != nullptr && Precedence1 != Precedence2) {
+const clang::SourceLocation StartLoc = BinOp->getBeginLoc();
+const clang::SourceLocation EndLoc =
+clang::Lexer::getLocForEndOfToken(BinOp->getEndLoc(), 0, SM, LangOpts);
+
+auto Diag =
+Check->diag(StartLoc,
+"'%0' has higher precedence than '%1'; add parentheses to "
+"explicitly specify the order of operations")
+<< (Precedence1 > Precedence2 ? BinOp->getOpcodeStr()
+ : ParentBinOp->getOpcodeStr())
+<< (Precedence1 > Precedence2 ? ParentBinOp->getOpcodeStr()
+ : BinOp->getOpcodeStr());
+
+Diag << FixItHint::CreateInsertion(StartLoc, "(");
+Diag << FixItHint::CreateInsertion(EndLoc, ")");
+Diag << SourceRange(StartLoc, EndLoc);
5chmidti wrote:
Nit: You could stream these directly into the diagnostic, like the operator
strings, and remove the `Diag` variable`.
https://github.com/llvm/llvm-project/pull/84481
___
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 readability-math-missing-parentheses (PR #84481)
https://github.com/5chmidti approved this pull request. LGTM from my side, others are still reviewing. Your test file contains a few trailing whitespaces that you could remove. https://github.com/llvm/llvm-project/pull/84481 ___ 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 readability-math-missing-parentheses (PR #84481)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/84481 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,278 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang;
+
+namespace {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ SmallVector Args;
+};
+
+} // anonymous namespace
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+static FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ // check if the function has initializer list argument
+ if (Call->getNumArgs() < 3) {
+auto ArgIterator = Call->arguments().begin();
+
+const auto *InitListExpr =
+dyn_cast(*ArgIterator);
+const auto *InitList =
+InitListExpr != nullptr
+? dyn_cast(
+ InitListExpr->getSubExpr()->IgnoreImplicit())
+: nullptr;
+
+if (InitListExpr && InitList) {
+ Result.Args.insert(Result.Args.begin(), InitList->inits().begin(),
+ InitList->inits().end());
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ // check if there is a comparison argument
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+Result.Compare = *ArgIterator;
+ }
+
+ return Result;
+}
+// if it has 3 arguments then the last will be the comparison
+ } else {
+Result.Compare = *(std::next(Call->arguments().begin(), 2));
+ }
+
+ for (const Expr *Arg : Call->arguments()) {
+if (!Result.First)
+ Result.First = Arg;
+
+if (Arg == Result.Compare)
+ continue;
+
+Result.Args.push_back(Arg);
+Result.Last = Arg;
+ }
+
+ return Result;
+}
+
+static SmallVector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult ) {
+ SmallVector FixItHints;
+
+ const QualType ResultType = TopCall->getDirectCallee()
+ ->getReturnType()
+ .getNonReferenceType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+ const auto = *Match.SourceManager;
+ const auto LanguageOpts = Match.Context->getLangOpts();
+ const bool IsInitializerList = Result.First == Result.Last;
+
+ // add { and } if the top call doesn't have an initializer list arg
+ if (!IsInitializerList) {
+FixItHints.push_back(
+FixItHint::CreateInsertion(Result.First->getBeginLoc(), "{"));
+
+if (Result.Compare)
+ FixItHints.push_back(FixItHint::CreateInsertion(
+ Lexer::getLocForEndOfToken(Result.Last->getEndLoc(), 0, SourceMngr,
+ LanguageOpts),
+ "}"));
+else
+ FixItHints.push_back(
+ FixItHint::CreateInsertion(TopCall->getEndLoc(), "}"));
+ }
+
+ for (const Expr *Arg : Result.Args) {
+const auto *InnerCall = dyn_cast(Arg->IgnoreParenImpCasts());
+
+// If the argument is not a nested call
+if (!InnerCall) {
+ // check if typecast is required
+ const QualType ArgType = Arg->IgnoreParenImpCasts()
+ ->getType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+
+ if (ArgType == ResultType)
+continue;
+
+ const StringRef ArgText = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Arg->getSourceRange()), SourceMngr,
+ LanguageOpts);
+
+ Twine Replacement = llvm::Twine("static_cast<")
+ .concat(ResultType.getAsString(LanguageOpts))
+ .concat(">(")
+ .concat(ArgText)
+ .concat(")");
+
+ FixItHints.push_back(FixItHint::CreateReplacement(Arg->getSourceRange(),
+Replacement.str()));
+
+ continue;
+}
+
+const auto InnerResult = findArgs(InnerCall);
+const auto InnerReplacements =
+generateReplacement(Match, InnerCall, InnerResult);
+const bool IsInnerInitializerList = InnerResult.First == InnerResult.Last;
+
+// if the nested call doesn't have arguments skip it
+if (!InnerResult.First || !InnerResult.Last)
+ continue;
+
+// if the nested call is not the same
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,278 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang;
+
+namespace {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ SmallVector Args;
+};
+
+} // anonymous namespace
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+static FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ // check if the function has initializer list argument
+ if (Call->getNumArgs() < 3) {
+auto ArgIterator = Call->arguments().begin();
+
+const auto *InitListExpr =
+dyn_cast(*ArgIterator);
+const auto *InitList =
+InitListExpr != nullptr
+? dyn_cast(
+ InitListExpr->getSubExpr()->IgnoreImplicit())
+: nullptr;
+
+if (InitListExpr && InitList) {
+ Result.Args.insert(Result.Args.begin(), InitList->inits().begin(),
+ InitList->inits().end());
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ // check if there is a comparison argument
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+Result.Compare = *ArgIterator;
+ }
+
+ return Result;
+}
+// if it has 3 arguments then the last will be the comparison
+ } else {
+Result.Compare = *(std::next(Call->arguments().begin(), 2));
+ }
+
+ for (const Expr *Arg : Call->arguments()) {
+if (!Result.First)
+ Result.First = Arg;
+
+if (Arg == Result.Compare)
+ continue;
+
+Result.Args.push_back(Arg);
+Result.Last = Arg;
+ }
+
+ return Result;
+}
+
+static SmallVector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult ) {
+ SmallVector FixItHints;
+
+ const QualType ResultType = TopCall->getDirectCallee()
+ ->getReturnType()
+ .getNonReferenceType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+ const auto = *Match.SourceManager;
+ const auto LanguageOpts = Match.Context->getLangOpts();
+ const bool IsInitializerList = Result.First == Result.Last;
+
+ // add { and } if the top call doesn't have an initializer list arg
+ if (!IsInitializerList) {
+FixItHints.push_back(
+FixItHint::CreateInsertion(Result.First->getBeginLoc(), "{"));
+
+if (Result.Compare)
+ FixItHints.push_back(FixItHint::CreateInsertion(
+ Lexer::getLocForEndOfToken(Result.Last->getEndLoc(), 0, SourceMngr,
+ LanguageOpts),
+ "}"));
+else
+ FixItHints.push_back(
+ FixItHint::CreateInsertion(TopCall->getEndLoc(), "}"));
+ }
+
+ for (const Expr *Arg : Result.Args) {
+const auto *InnerCall = dyn_cast(Arg->IgnoreParenImpCasts());
+
+// If the argument is not a nested call
+if (!InnerCall) {
+ // check if typecast is required
+ const QualType ArgType = Arg->IgnoreParenImpCasts()
+ ->getType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+
+ if (ArgType == ResultType)
+continue;
+
+ const StringRef ArgText = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Arg->getSourceRange()), SourceMngr,
+ LanguageOpts);
+
+ Twine Replacement = llvm::Twine("static_cast<")
+ .concat(ResultType.getAsString(LanguageOpts))
+ .concat(">(")
+ .concat(ArgText)
+ .concat(")");
+
+ FixItHints.push_back(FixItHint::CreateReplacement(Arg->getSourceRange(),
+Replacement.str()));
+
+ continue;
+}
+
+const auto InnerResult = findArgs(InnerCall);
+const auto InnerReplacements =
+generateReplacement(Match, InnerCall, InnerResult);
+const bool IsInnerInitializerList = InnerResult.First == InnerResult.Last;
+
+// if the nested call doesn't have arguments skip it
+if (!InnerResult.First || !InnerResult.Last)
+ continue;
+
+// if the nested call is not the same
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,278 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang;
+
+namespace {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ SmallVector Args;
+};
+
+} // anonymous namespace
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+static FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ // check if the function has initializer list argument
+ if (Call->getNumArgs() < 3) {
+auto ArgIterator = Call->arguments().begin();
+
+const auto *InitListExpr =
+dyn_cast(*ArgIterator);
+const auto *InitList =
+InitListExpr != nullptr
+? dyn_cast(
+ InitListExpr->getSubExpr()->IgnoreImplicit())
+: nullptr;
+
+if (InitListExpr && InitList) {
+ Result.Args.insert(Result.Args.begin(), InitList->inits().begin(),
+ InitList->inits().end());
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ // check if there is a comparison argument
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+Result.Compare = *ArgIterator;
+ }
+
+ return Result;
+}
+// if it has 3 arguments then the last will be the comparison
+ } else {
+Result.Compare = *(std::next(Call->arguments().begin(), 2));
+ }
+
+ for (const Expr *Arg : Call->arguments()) {
+if (!Result.First)
+ Result.First = Arg;
+
+if (Arg == Result.Compare)
+ continue;
+
+Result.Args.push_back(Arg);
+Result.Last = Arg;
+ }
+
+ return Result;
+}
+
+static SmallVector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult ) {
+ SmallVector FixItHints;
+
+ const QualType ResultType = TopCall->getDirectCallee()
+ ->getReturnType()
+ .getNonReferenceType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+ const auto = *Match.SourceManager;
+ const auto LanguageOpts = Match.Context->getLangOpts();
5chmidti wrote:
`LanguageOpt` should be `const&`
https://github.com/llvm/llvm-project/pull/85572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,278 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang;
+
+namespace {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ SmallVector Args;
+};
+
+} // anonymous namespace
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+static FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ // check if the function has initializer list argument
+ if (Call->getNumArgs() < 3) {
+auto ArgIterator = Call->arguments().begin();
+
+const auto *InitListExpr =
+dyn_cast(*ArgIterator);
+const auto *InitList =
+InitListExpr != nullptr
+? dyn_cast(
+ InitListExpr->getSubExpr()->IgnoreImplicit())
+: nullptr;
+
+if (InitListExpr && InitList) {
+ Result.Args.insert(Result.Args.begin(), InitList->inits().begin(),
+ InitList->inits().end());
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ // check if there is a comparison argument
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+Result.Compare = *ArgIterator;
+ }
+
+ return Result;
+}
+// if it has 3 arguments then the last will be the comparison
+ } else {
+Result.Compare = *(std::next(Call->arguments().begin(), 2));
+ }
+
+ for (const Expr *Arg : Call->arguments()) {
+if (!Result.First)
+ Result.First = Arg;
+
+if (Arg == Result.Compare)
+ continue;
+
+Result.Args.push_back(Arg);
+Result.Last = Arg;
+ }
+
+ return Result;
+}
+
+static SmallVector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult ) {
+ SmallVector FixItHints;
+
+ const QualType ResultType = TopCall->getDirectCallee()
+ ->getReturnType()
+ .getNonReferenceType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+ const auto = *Match.SourceManager;
+ const auto LanguageOpts = Match.Context->getLangOpts();
+ const bool IsInitializerList = Result.First == Result.Last;
+
+ // add { and } if the top call doesn't have an initializer list arg
+ if (!IsInitializerList) {
+FixItHints.push_back(
+FixItHint::CreateInsertion(Result.First->getBeginLoc(), "{"));
+
+if (Result.Compare)
+ FixItHints.push_back(FixItHint::CreateInsertion(
+ Lexer::getLocForEndOfToken(Result.Last->getEndLoc(), 0, SourceMngr,
+ LanguageOpts),
+ "}"));
+else
+ FixItHints.push_back(
+ FixItHint::CreateInsertion(TopCall->getEndLoc(), "}"));
+ }
+
+ for (const Expr *Arg : Result.Args) {
+const auto *InnerCall = dyn_cast(Arg->IgnoreParenImpCasts());
+
+// If the argument is not a nested call
+if (!InnerCall) {
+ // check if typecast is required
+ const QualType ArgType = Arg->IgnoreParenImpCasts()
+ ->getType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+
+ if (ArgType == ResultType)
+continue;
+
+ const StringRef ArgText = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Arg->getSourceRange()), SourceMngr,
+ LanguageOpts);
+
+ Twine Replacement = llvm::Twine("static_cast<")
+ .concat(ResultType.getAsString(LanguageOpts))
+ .concat(">(")
+ .concat(ArgText)
+ .concat(")");
+
+ FixItHints.push_back(FixItHint::CreateReplacement(Arg->getSourceRange(),
+Replacement.str()));
+
+ continue;
+}
+
+const auto InnerResult = findArgs(InnerCall);
+const auto InnerReplacements =
+generateReplacement(Match, InnerCall, InnerResult);
+const bool IsInnerInitializerList = InnerResult.First == InnerResult.Last;
+
+// if the nested call doesn't have arguments skip it
+if (!InnerResult.First || !InnerResult.Last)
+ continue;
+
+// if the nested call is not the same
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,278 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang;
+
+namespace {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ SmallVector Args;
+};
+
+} // anonymous namespace
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+static FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ // check if the function has initializer list argument
+ if (Call->getNumArgs() < 3) {
+auto ArgIterator = Call->arguments().begin();
+
+const auto *InitListExpr =
+dyn_cast(*ArgIterator);
+const auto *InitList =
+InitListExpr != nullptr
+? dyn_cast(
+ InitListExpr->getSubExpr()->IgnoreImplicit())
+: nullptr;
+
+if (InitListExpr && InitList) {
+ Result.Args.insert(Result.Args.begin(), InitList->inits().begin(),
+ InitList->inits().end());
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ // check if there is a comparison argument
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+Result.Compare = *ArgIterator;
+ }
+
+ return Result;
+}
+// if it has 3 arguments then the last will be the comparison
+ } else {
+Result.Compare = *(std::next(Call->arguments().begin(), 2));
+ }
+
+ for (const Expr *Arg : Call->arguments()) {
+if (!Result.First)
+ Result.First = Arg;
+
+if (Arg == Result.Compare)
+ continue;
+
+Result.Args.push_back(Arg);
+Result.Last = Arg;
+ }
+
+ return Result;
+}
+
+static SmallVector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult ) {
+ SmallVector FixItHints;
+
+ const QualType ResultType = TopCall->getDirectCallee()
+ ->getReturnType()
+ .getNonReferenceType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+ const auto = *Match.SourceManager;
+ const auto LanguageOpts = Match.Context->getLangOpts();
+ const bool IsInitializerList = Result.First == Result.Last;
+
+ // add { and } if the top call doesn't have an initializer list arg
+ if (!IsInitializerList) {
+FixItHints.push_back(
+FixItHint::CreateInsertion(Result.First->getBeginLoc(), "{"));
+
+if (Result.Compare)
+ FixItHints.push_back(FixItHint::CreateInsertion(
+ Lexer::getLocForEndOfToken(Result.Last->getEndLoc(), 0, SourceMngr,
+ LanguageOpts),
+ "}"));
+else
+ FixItHints.push_back(
+ FixItHint::CreateInsertion(TopCall->getEndLoc(), "}"));
+ }
+
+ for (const Expr *Arg : Result.Args) {
+const auto *InnerCall = dyn_cast(Arg->IgnoreParenImpCasts());
+
+// If the argument is not a nested call
+if (!InnerCall) {
+ // check if typecast is required
+ const QualType ArgType = Arg->IgnoreParenImpCasts()
+ ->getType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+
+ if (ArgType == ResultType)
+continue;
+
+ const StringRef ArgText = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Arg->getSourceRange()), SourceMngr,
+ LanguageOpts);
+
+ Twine Replacement = llvm::Twine("static_cast<")
+ .concat(ResultType.getAsString(LanguageOpts))
+ .concat(">(")
+ .concat(ArgText)
+ .concat(")");
+
+ FixItHints.push_back(FixItHint::CreateReplacement(Arg->getSourceRange(),
+Replacement.str()));
+
+ continue;
+}
+
+const auto InnerResult = findArgs(InnerCall);
+const auto InnerReplacements =
+generateReplacement(Match, InnerCall, InnerResult);
+const bool IsInnerInitializerList = InnerResult.First == InnerResult.Last;
+
+// if the nested call doesn't have arguments skip it
+if (!InnerResult.First || !InnerResult.Last)
+ continue;
+
+// if the nested call is not the same
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,278 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang;
+
+namespace {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ SmallVector Args;
+};
+
+} // anonymous namespace
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+static FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ // check if the function has initializer list argument
+ if (Call->getNumArgs() < 3) {
+auto ArgIterator = Call->arguments().begin();
+
+const auto *InitListExpr =
+dyn_cast(*ArgIterator);
+const auto *InitList =
+InitListExpr != nullptr
+? dyn_cast(
+ InitListExpr->getSubExpr()->IgnoreImplicit())
+: nullptr;
+
+if (InitListExpr && InitList) {
+ Result.Args.insert(Result.Args.begin(), InitList->inits().begin(),
+ InitList->inits().end());
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ // check if there is a comparison argument
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+Result.Compare = *ArgIterator;
+ }
+
+ return Result;
+}
+// if it has 3 arguments then the last will be the comparison
+ } else {
+Result.Compare = *(std::next(Call->arguments().begin(), 2));
+ }
+
+ for (const Expr *Arg : Call->arguments()) {
+if (!Result.First)
+ Result.First = Arg;
+
+if (Arg == Result.Compare)
+ continue;
+
+Result.Args.push_back(Arg);
+Result.Last = Arg;
+ }
+
+ return Result;
+}
+
+static SmallVector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult ) {
+ SmallVector FixItHints;
+
+ const QualType ResultType = TopCall->getDirectCallee()
+ ->getReturnType()
+ .getNonReferenceType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+ const auto = *Match.SourceManager;
+ const auto LanguageOpts = Match.Context->getLangOpts();
+ const bool IsInitializerList = Result.First == Result.Last;
+
+ // add { and } if the top call doesn't have an initializer list arg
+ if (!IsInitializerList) {
+FixItHints.push_back(
+FixItHint::CreateInsertion(Result.First->getBeginLoc(), "{"));
+
+if (Result.Compare)
+ FixItHints.push_back(FixItHint::CreateInsertion(
+ Lexer::getLocForEndOfToken(Result.Last->getEndLoc(), 0, SourceMngr,
+ LanguageOpts),
+ "}"));
+else
+ FixItHints.push_back(
+ FixItHint::CreateInsertion(TopCall->getEndLoc(), "}"));
+ }
+
+ for (const Expr *Arg : Result.Args) {
+const auto *InnerCall = dyn_cast(Arg->IgnoreParenImpCasts());
+
+// If the argument is not a nested call
+if (!InnerCall) {
+ // check if typecast is required
+ const QualType ArgType = Arg->IgnoreParenImpCasts()
+ ->getType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+
+ if (ArgType == ResultType)
+continue;
+
+ const StringRef ArgText = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Arg->getSourceRange()), SourceMngr,
+ LanguageOpts);
+
+ Twine Replacement = llvm::Twine("static_cast<")
+ .concat(ResultType.getAsString(LanguageOpts))
+ .concat(">(")
+ .concat(ArgText)
+ .concat(")");
+
+ FixItHints.push_back(FixItHint::CreateReplacement(Arg->getSourceRange(),
+Replacement.str()));
+
+ continue;
+}
+
+const auto InnerResult = findArgs(InnerCall);
+const auto InnerReplacements =
5chmidti wrote:
Move `InnerReplacements` to the place when it is needed, it's not needed to
check the following conditions and can be created afterward, when it is added
to `FixItHints`.
https://github.com/llvm/llvm-project/pull/85572
___
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,278 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang;
+
+namespace {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ SmallVector Args;
+};
+
+} // anonymous namespace
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+static FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ // check if the function has initializer list argument
+ if (Call->getNumArgs() < 3) {
+auto ArgIterator = Call->arguments().begin();
+
+const auto *InitListExpr =
+dyn_cast(*ArgIterator);
+const auto *InitList =
+InitListExpr != nullptr
+? dyn_cast(
+ InitListExpr->getSubExpr()->IgnoreImplicit())
+: nullptr;
+
+if (InitListExpr && InitList) {
5chmidti wrote:
You only need `InitList` to not be a `nullptr`, `InitListExpr` will implicitly
be not a `nullptr` as well if `InitList` isn't one, and you don't even use
`InitListExpr` inside the `if`.
https://github.com/llvm/llvm-project/pull/85572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,278 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang;
+
+namespace {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ SmallVector Args;
+};
+
+} // anonymous namespace
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+static FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ // check if the function has initializer list argument
+ if (Call->getNumArgs() < 3) {
+auto ArgIterator = Call->arguments().begin();
+
+const auto *InitListExpr =
+dyn_cast(*ArgIterator);
+const auto *InitList =
+InitListExpr != nullptr
+? dyn_cast(
+ InitListExpr->getSubExpr()->IgnoreImplicit())
+: nullptr;
+
+if (InitListExpr && InitList) {
+ Result.Args.insert(Result.Args.begin(), InitList->inits().begin(),
+ InitList->inits().end());
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ // check if there is a comparison argument
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+Result.Compare = *ArgIterator;
+ }
+
+ return Result;
+}
+// if it has 3 arguments then the last will be the comparison
+ } else {
+Result.Compare = *(std::next(Call->arguments().begin(), 2));
+ }
+
+ for (const Expr *Arg : Call->arguments()) {
+if (!Result.First)
+ Result.First = Arg;
+
+if (Arg == Result.Compare)
+ continue;
+
+Result.Args.push_back(Arg);
+Result.Last = Arg;
+ }
+
+ return Result;
+}
+
+static SmallVector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult ) {
+ SmallVector FixItHints;
+
+ const QualType ResultType = TopCall->getDirectCallee()
+ ->getReturnType()
+ .getNonReferenceType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+ const auto = *Match.SourceManager;
+ const auto LanguageOpts = Match.Context->getLangOpts();
+ const bool IsInitializerList = Result.First == Result.Last;
+
+ // add { and } if the top call doesn't have an initializer list arg
+ if (!IsInitializerList) {
+FixItHints.push_back(
+FixItHint::CreateInsertion(Result.First->getBeginLoc(), "{"));
+
+if (Result.Compare)
+ FixItHints.push_back(FixItHint::CreateInsertion(
+ Lexer::getLocForEndOfToken(Result.Last->getEndLoc(), 0, SourceMngr,
+ LanguageOpts),
+ "}"));
+else
+ FixItHints.push_back(
+ FixItHint::CreateInsertion(TopCall->getEndLoc(), "}"));
+ }
+
+ for (const Expr *Arg : Result.Args) {
+const auto *InnerCall = dyn_cast(Arg->IgnoreParenImpCasts());
+
+// If the argument is not a nested call
+if (!InnerCall) {
+ // check if typecast is required
+ const QualType ArgType = Arg->IgnoreParenImpCasts()
+ ->getType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+
+ if (ArgType == ResultType)
+continue;
+
+ const StringRef ArgText = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Arg->getSourceRange()), SourceMngr,
+ LanguageOpts);
+
+ Twine Replacement = llvm::Twine("static_cast<")
+ .concat(ResultType.getAsString(LanguageOpts))
+ .concat(">(")
+ .concat(ArgText)
+ .concat(")");
+
+ FixItHints.push_back(FixItHint::CreateReplacement(Arg->getSourceRange(),
+Replacement.str()));
+
+ continue;
+}
+
+const auto InnerResult = findArgs(InnerCall);
+const auto InnerReplacements =
5chmidti wrote:
Please explicitly spell out the types of these variables, because they are not
mentioned on the right-hand-side.
https://github.com/llvm/llvm-project/pull/85572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,278 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang;
+
+namespace {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ SmallVector Args;
+};
+
+} // anonymous namespace
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+static FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ // check if the function has initializer list argument
+ if (Call->getNumArgs() < 3) {
+auto ArgIterator = Call->arguments().begin();
+
+const auto *InitListExpr =
+dyn_cast(*ArgIterator);
+const auto *InitList =
+InitListExpr != nullptr
+? dyn_cast(
+ InitListExpr->getSubExpr()->IgnoreImplicit())
+: nullptr;
+
+if (InitListExpr && InitList) {
+ Result.Args.insert(Result.Args.begin(), InitList->inits().begin(),
+ InitList->inits().end());
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ // check if there is a comparison argument
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+Result.Compare = *ArgIterator;
+ }
+
+ return Result;
+}
+// if it has 3 arguments then the last will be the comparison
+ } else {
+Result.Compare = *(std::next(Call->arguments().begin(), 2));
+ }
+
+ for (const Expr *Arg : Call->arguments()) {
+if (!Result.First)
+ Result.First = Arg;
+
+if (Arg == Result.Compare)
+ continue;
+
+Result.Args.push_back(Arg);
+Result.Last = Arg;
+ }
+
+ return Result;
+}
+
+static SmallVector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult ) {
+ SmallVector FixItHints;
+
+ const QualType ResultType = TopCall->getDirectCallee()
+ ->getReturnType()
+ .getNonReferenceType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+ const auto = *Match.SourceManager;
+ const auto LanguageOpts = Match.Context->getLangOpts();
+ const bool IsInitializerList = Result.First == Result.Last;
+
+ // add { and } if the top call doesn't have an initializer list arg
+ if (!IsInitializerList) {
+FixItHints.push_back(
+FixItHint::CreateInsertion(Result.First->getBeginLoc(), "{"));
+
+if (Result.Compare)
+ FixItHints.push_back(FixItHint::CreateInsertion(
+ Lexer::getLocForEndOfToken(Result.Last->getEndLoc(), 0, SourceMngr,
+ LanguageOpts),
+ "}"));
+else
+ FixItHints.push_back(
+ FixItHint::CreateInsertion(TopCall->getEndLoc(), "}"));
+ }
+
+ for (const Expr *Arg : Result.Args) {
+const auto *InnerCall = dyn_cast(Arg->IgnoreParenImpCasts());
+
+// If the argument is not a nested call
+if (!InnerCall) {
+ // check if typecast is required
+ const QualType ArgType = Arg->IgnoreParenImpCasts()
+ ->getType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+
+ if (ArgType == ResultType)
+continue;
+
+ const StringRef ArgText = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Arg->getSourceRange()), SourceMngr,
+ LanguageOpts);
+
+ Twine Replacement = llvm::Twine("static_cast<")
+ .concat(ResultType.getAsString(LanguageOpts))
+ .concat(">(")
+ .concat(ArgText)
+ .concat(")");
+
+ FixItHints.push_back(FixItHint::CreateReplacement(Arg->getSourceRange(),
+Replacement.str()));
+
+ continue;
+}
+
+const auto InnerResult = findArgs(InnerCall);
+const auto InnerReplacements =
+generateReplacement(Match, InnerCall, InnerResult);
+const bool IsInnerInitializerList = InnerResult.First == InnerResult.Last;
+
+// if the nested call doesn't have arguments skip it
+if (!InnerResult.First || !InnerResult.Last)
+ continue;
+
+// if the nested call is not the same
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,278 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang;
+
+namespace {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ SmallVector Args;
+};
+
+} // anonymous namespace
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+static FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ // check if the function has initializer list argument
+ if (Call->getNumArgs() < 3) {
+auto ArgIterator = Call->arguments().begin();
+
+const auto *InitListExpr =
+dyn_cast(*ArgIterator);
+const auto *InitList =
+InitListExpr != nullptr
+? dyn_cast(
+ InitListExpr->getSubExpr()->IgnoreImplicit())
+: nullptr;
+
+if (InitListExpr && InitList) {
+ Result.Args.insert(Result.Args.begin(), InitList->inits().begin(),
+ InitList->inits().end());
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ // check if there is a comparison argument
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+Result.Compare = *ArgIterator;
+ }
5chmidti wrote:
Single statement if-branch -> remove braces
https://github.com/llvm/llvm-project/pull/85572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/85572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,278 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang;
+
+namespace {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ SmallVector Args;
+};
+
+} // anonymous namespace
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+static FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ // check if the function has initializer list argument
+ if (Call->getNumArgs() < 3) {
+auto ArgIterator = Call->arguments().begin();
+
+const auto *InitListExpr =
+dyn_cast(*ArgIterator);
+const auto *InitList =
+InitListExpr != nullptr
+? dyn_cast(
+ InitListExpr->getSubExpr()->IgnoreImplicit())
+: nullptr;
+
+if (InitListExpr && InitList) {
+ Result.Args.insert(Result.Args.begin(), InitList->inits().begin(),
+ InitList->inits().end());
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ // check if there is a comparison argument
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+Result.Compare = *ArgIterator;
+ }
+
+ return Result;
+}
+// if it has 3 arguments then the last will be the comparison
+ } else {
+Result.Compare = *(std::next(Call->arguments().begin(), 2));
+ }
+
+ for (const Expr *Arg : Call->arguments()) {
+if (!Result.First)
+ Result.First = Arg;
+
+if (Arg == Result.Compare)
+ continue;
+
+Result.Args.push_back(Arg);
+Result.Last = Arg;
+ }
+
+ return Result;
+}
+
+static SmallVector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult ) {
+ SmallVector FixItHints;
+
+ const QualType ResultType = TopCall->getDirectCallee()
+ ->getReturnType()
+ .getNonReferenceType()
+ .getUnqualifiedType()
+ .getCanonicalType();
+ const auto = *Match.SourceManager;
+ const auto LanguageOpts = Match.Context->getLangOpts();
+ const bool IsInitializerList = Result.First == Result.Last;
+
+ // add { and } if the top call doesn't have an initializer list arg
+ if (!IsInitializerList) {
+FixItHints.push_back(
+FixItHint::CreateInsertion(Result.First->getBeginLoc(), "{"));
+
+if (Result.Compare)
+ FixItHints.push_back(FixItHint::CreateInsertion(
+ Lexer::getLocForEndOfToken(Result.Last->getEndLoc(), 0, SourceMngr,
+ LanguageOpts),
5chmidti wrote:
It looks like `Lexer::getLocForEndOfToken` is not necessary. When I remove it,
the tests still pass. I also tested: `std::min(static_cast(111),
std::min(static_cast(222), static_cast(333), less_than), less_than)`
and it works as expected. If there is a need for this function call, please add
a test case for it.
https://github.com/llvm/llvm-project/pull/85572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,278 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang;
+
+namespace {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ SmallVector Args;
+};
+
+} // anonymous namespace
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+static FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ // check if the function has initializer list argument
+ if (Call->getNumArgs() < 3) {
+auto ArgIterator = Call->arguments().begin();
+
+const auto *InitListExpr =
+dyn_cast(*ArgIterator);
+const auto *InitList =
+InitListExpr != nullptr
+? dyn_cast(
+ InitListExpr->getSubExpr()->IgnoreImplicit())
+: nullptr;
+
+if (InitListExpr && InitList) {
+ Result.Args.insert(Result.Args.begin(), InitList->inits().begin(),
+ InitList->inits().end());
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ // check if there is a comparison argument
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+Result.Compare = *ArgIterator;
+ }
+
+ return Result;
+}
+// if it has 3 arguments then the last will be the comparison
+ } else {
+Result.Compare = *(std::next(Call->arguments().begin(), 2));
+ }
+
+ for (const Expr *Arg : Call->arguments()) {
+if (!Result.First)
+ Result.First = Arg;
+
+if (Arg == Result.Compare)
+ continue;
+
+Result.Args.push_back(Arg);
+Result.Last = Arg;
+ }
5chmidti wrote:
This can be simplified to
```c++
if (Result.Compare)
Result.Args = SmallVector(llvm::drop_end(Call->arguments()));
else
Result.Args = SmallVector(Call->arguments());
Result.First = Result.Args.front();
Result.Last = Result.Args.back();
```
https://github.com/llvm/llvm-project/pull/85572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
https://github.com/5chmidti requested changes to this pull request.
Functionality-wise, this looks good to me, I only have some comments regarding
cleanup.
Please also add tests for min and max calls that are true-negatives, i.e.:
`std::max(1, 2);`, `std::max({1, 2, 3});` and `std::max({1, 2, 3}, less_than);`
are probably enough
https://github.com/llvm/llvm-project/pull/85572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,278 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang;
+
+namespace {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ SmallVector Args;
+};
+
+} // anonymous namespace
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+static FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ // check if the function has initializer list argument
+ if (Call->getNumArgs() < 3) {
+auto ArgIterator = Call->arguments().begin();
+
+const auto *InitListExpr =
+dyn_cast(*ArgIterator);
+const auto *InitList =
+InitListExpr != nullptr
+? dyn_cast(
+ InitListExpr->getSubExpr()->IgnoreImplicit())
+: nullptr;
+
+if (InitListExpr && InitList) {
+ Result.Args.insert(Result.Args.begin(), InitList->inits().begin(),
+ InitList->inits().end());
5chmidti wrote:
Nit: You could write `Result.Args.append(InitList->inits().begin(),
InitList->inits().end());` instead, but this isn't really important.
https://github.com/llvm/llvm-project/pull/85572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,278 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "../utils/LexerUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang;
+
+namespace {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ SmallVector Args;
+};
+
+} // anonymous namespace
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+static FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ // check if the function has initializer list argument
+ if (Call->getNumArgs() < 3) {
+auto ArgIterator = Call->arguments().begin();
+
+const auto *InitListExpr =
+dyn_cast(*ArgIterator);
+const auto *InitList =
+InitListExpr != nullptr
+? dyn_cast(
+ InitListExpr->getSubExpr()->IgnoreImplicit())
+: nullptr;
+
+if (InitListExpr && InitList) {
+ Result.Args.insert(Result.Args.begin(), InitList->inits().begin(),
+ InitList->inits().end());
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ // check if there is a comparison argument
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+Result.Compare = *ArgIterator;
+ }
+
+ return Result;
+}
+// if it has 3 arguments then the last will be the comparison
5chmidti wrote:
Nit: Move this comment either inside the `else` branch or in it's onw line
between `}`, `else` and/or `{`.
https://github.com/llvm/llvm-project/pull/85572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,199 @@
+//===--- EnumInitialValueCheck.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 "EnumInitialValueCheck.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+static bool isNoneEnumeratorsInitialized(const EnumDecl ) {
+ return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) {
+return ECD->getInitExpr() == nullptr;
+ });
+}
+
+static bool isOnlyFirstEnumeratorInitialized(const EnumDecl ) {
+ bool IsFirst = true;
+ for (const EnumConstantDecl *ECD : Node.enumerators()) {
+if ((IsFirst && ECD->getInitExpr() == nullptr) ||
+(!IsFirst && ECD->getInitExpr() != nullptr))
+ return false;
+IsFirst = false;
+ }
+ return !IsFirst;
+}
+
+static bool areAllEnumeratorsInitialized(const EnumDecl ) {
+ return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) {
+return ECD->getInitExpr() != nullptr;
+ });
+}
+
+/// Check if \p Enumerator is initialized with a (potentially negated) \c
+/// IntegerLiteral.
+static bool isInitializedByLiteral(const EnumConstantDecl *Enumerator) {
+ const Expr *const Init = Enumerator->getInitExpr();
+ if (!Init)
+return false;
+ return Init->isIntegerConstantExpr(Enumerator->getASTContext());
+}
+
+static void cleanInitialValue(DiagnosticBuilder ,
+ const EnumConstantDecl *ECD,
+ const SourceManager ,
+ const LangOptions ) {
+ std::optional EqualToken =
utils::lexer::findNextTokenSkippingComments(
+ ECD->getLocation(), SM, LangOpts);
+ if (!EqualToken.has_value())
+return;
+ SourceLocation EqualLoc{EqualToken->getLocation()};
+ if (EqualLoc.isInvalid() || EqualLoc.isMacroID())
+return;
+ SourceRange InitExprRange = ECD->getInitExpr()->getSourceRange();
+ if (InitExprRange.isInvalid() || InitExprRange.getBegin().isMacroID() ||
+ InitExprRange.getEnd().isMacroID())
+return;
+ Diag << FixItHint::CreateRemoval(EqualLoc)
+ << FixItHint::CreateRemoval(InitExprRange);
+ return;
+}
+
+namespace {
+
+AST_MATCHER(EnumDecl, isMacro) {
+ SourceLocation Loc = Node.getBeginLoc();
+ return Loc.isMacroID();
+}
+
+AST_MATCHER(EnumDecl, hasConsistentInitialValues) {
+ return isNoneEnumeratorsInitialized(Node) ||
+ isOnlyFirstEnumeratorInitialized(Node) ||
+ areAllEnumeratorsInitialized(Node);
+}
+
+AST_MATCHER(EnumDecl, hasZeroInitialValueForFirstEnumerator) {
+ EnumDecl::enumerator_range Enumerators = Node.enumerators();
+ if (Enumerators.empty())
+return false;
+ const EnumConstantDecl *ECD = *Enumerators.begin();
+ return isOnlyFirstEnumeratorInitialized(Node) &&
+ isInitializedByLiteral(ECD) && ECD->getInitVal().isZero();
+}
+
+/// Excludes bitfields because enumerators initialized with the result of a
+/// bitwise operator on enumeration values or any other expr that is not a
+/// potentially negative integer literal.
+/// Enumerations where it is not directly clear if they are used with
+/// bitmask, evident when enumerators are only initialized with (potentially
+/// negative) integer literals, are ignored. This is also the case when all
+/// enumerators are powers of two (e.g., 0, 1, 2).
+AST_MATCHER(EnumDecl, hasSequentialInitialValues) {
+ EnumDecl::enumerator_range Enumerators = Node.enumerators();
+ if (Enumerators.empty())
+return false;
+ const EnumConstantDecl *const FirstEnumerator = *Node.enumerator_begin();
+ llvm::APSInt PrevValue = FirstEnumerator->getInitVal();
+ if (!isInitializedByLiteral(FirstEnumerator))
+return false;
+ bool AllEnumeratorsArePowersOfTwo = true;
+ for (const EnumConstantDecl *Enumerator : llvm::drop_begin(Enumerators)) {
+const llvm::APSInt NewValue = Enumerator->getInitVal();
+if (NewValue != ++PrevValue)
+ return false;
+if (!isInitializedByLiteral(Enumerator))
+ return false;
+PrevValue = NewValue;
+AllEnumeratorsArePowersOfTwo &= NewValue.isPowerOf2();
+ }
+ return !AllEnumeratorsArePowersOfTwo;
+}
+
+} // namespace
+
+EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name,
+ ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ AllowExplicitZeroFirstInitialValue(
+ Options.get("AllowExplicitZeroFirstInitialValue", true)),
+
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,199 @@
+//===--- EnumInitialValueCheck.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 "EnumInitialValueCheck.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+static bool isNoneEnumeratorsInitialized(const EnumDecl ) {
+ return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) {
+return ECD->getInitExpr() == nullptr;
+ });
+}
+
+static bool isOnlyFirstEnumeratorInitialized(const EnumDecl ) {
+ bool IsFirst = true;
+ for (const EnumConstantDecl *ECD : Node.enumerators()) {
+if ((IsFirst && ECD->getInitExpr() == nullptr) ||
+(!IsFirst && ECD->getInitExpr() != nullptr))
+ return false;
+IsFirst = false;
+ }
+ return !IsFirst;
+}
+
+static bool areAllEnumeratorsInitialized(const EnumDecl ) {
+ return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) {
+return ECD->getInitExpr() != nullptr;
+ });
+}
+
+/// Check if \p Enumerator is initialized with a (potentially negated) \c
+/// IntegerLiteral.
+static bool isInitializedByLiteral(const EnumConstantDecl *Enumerator) {
+ const Expr *const Init = Enumerator->getInitExpr();
+ if (!Init)
+return false;
+ return Init->isIntegerConstantExpr(Enumerator->getASTContext());
+}
+
+static void cleanInitialValue(DiagnosticBuilder ,
+ const EnumConstantDecl *ECD,
+ const SourceManager ,
+ const LangOptions ) {
+ std::optional EqualToken =
utils::lexer::findNextTokenSkippingComments(
+ ECD->getLocation(), SM, LangOpts);
+ if (!EqualToken.has_value())
+return;
+ SourceLocation EqualLoc{EqualToken->getLocation()};
+ if (EqualLoc.isInvalid() || EqualLoc.isMacroID())
+return;
+ SourceRange InitExprRange = ECD->getInitExpr()->getSourceRange();
+ if (InitExprRange.isInvalid() || InitExprRange.getBegin().isMacroID() ||
+ InitExprRange.getEnd().isMacroID())
+return;
+ Diag << FixItHint::CreateRemoval(EqualLoc)
+ << FixItHint::CreateRemoval(InitExprRange);
+ return;
+}
+
+namespace {
+
+AST_MATCHER(EnumDecl, isMacro) {
+ SourceLocation Loc = Node.getBeginLoc();
+ return Loc.isMacroID();
+}
+
+AST_MATCHER(EnumDecl, hasConsistentInitialValues) {
+ return isNoneEnumeratorsInitialized(Node) ||
+ isOnlyFirstEnumeratorInitialized(Node) ||
+ areAllEnumeratorsInitialized(Node);
+}
+
+AST_MATCHER(EnumDecl, hasZeroInitialValueForFirstEnumerator) {
+ EnumDecl::enumerator_range Enumerators = Node.enumerators();
+ if (Enumerators.empty())
+return false;
+ const EnumConstantDecl *ECD = *Enumerators.begin();
+ return isOnlyFirstEnumeratorInitialized(Node) &&
+ isInitializedByLiteral(ECD) && ECD->getInitVal().isZero();
+}
+
+/// Excludes bitfields because enumerators initialized with the result of a
+/// bitwise operator on enumeration values or any other expr that is not a
+/// potentially negative integer literal.
+/// Enumerations where it is not directly clear if they are used with
+/// bitmask, evident when enumerators are only initialized with (potentially
+/// negative) integer literals, are ignored. This is also the case when all
+/// enumerators are powers of two (e.g., 0, 1, 2).
+AST_MATCHER(EnumDecl, hasSequentialInitialValues) {
+ EnumDecl::enumerator_range Enumerators = Node.enumerators();
+ if (Enumerators.empty())
+return false;
+ const EnumConstantDecl *const FirstEnumerator = *Node.enumerator_begin();
+ llvm::APSInt PrevValue = FirstEnumerator->getInitVal();
+ if (!isInitializedByLiteral(FirstEnumerator))
+return false;
+ bool AllEnumeratorsArePowersOfTwo = true;
+ for (const EnumConstantDecl *Enumerator : llvm::drop_begin(Enumerators)) {
+const llvm::APSInt NewValue = Enumerator->getInitVal();
+if (NewValue != ++PrevValue)
+ return false;
+if (!isInitializedByLiteral(Enumerator))
+ return false;
+PrevValue = NewValue;
+AllEnumeratorsArePowersOfTwo &= NewValue.isPowerOf2();
+ }
+ return !AllEnumeratorsArePowersOfTwo;
+}
+
+} // namespace
+
+EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name,
+ ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ AllowExplicitZeroFirstInitialValue(
+ Options.get("AllowExplicitZeroFirstInitialValue", true)),
+
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,199 @@
+//===--- EnumInitialValueCheck.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 "EnumInitialValueCheck.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+static bool isNoneEnumeratorsInitialized(const EnumDecl ) {
+ return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) {
+return ECD->getInitExpr() == nullptr;
+ });
+}
+
+static bool isOnlyFirstEnumeratorInitialized(const EnumDecl ) {
+ bool IsFirst = true;
+ for (const EnumConstantDecl *ECD : Node.enumerators()) {
+if ((IsFirst && ECD->getInitExpr() == nullptr) ||
+(!IsFirst && ECD->getInitExpr() != nullptr))
+ return false;
+IsFirst = false;
+ }
+ return !IsFirst;
+}
+
+static bool areAllEnumeratorsInitialized(const EnumDecl ) {
+ return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) {
+return ECD->getInitExpr() != nullptr;
+ });
+}
+
+/// Check if \p Enumerator is initialized with a (potentially negated) \c
+/// IntegerLiteral.
+static bool isInitializedByLiteral(const EnumConstantDecl *Enumerator) {
+ const Expr *const Init = Enumerator->getInitExpr();
+ if (!Init)
+return false;
+ return Init->isIntegerConstantExpr(Enumerator->getASTContext());
+}
+
+static void cleanInitialValue(DiagnosticBuilder ,
+ const EnumConstantDecl *ECD,
+ const SourceManager ,
+ const LangOptions ) {
+ std::optional EqualToken =
utils::lexer::findNextTokenSkippingComments(
+ ECD->getLocation(), SM, LangOpts);
+ if (!EqualToken.has_value())
+return;
+ SourceLocation EqualLoc{EqualToken->getLocation()};
+ if (EqualLoc.isInvalid() || EqualLoc.isMacroID())
+return;
+ SourceRange InitExprRange = ECD->getInitExpr()->getSourceRange();
+ if (InitExprRange.isInvalid() || InitExprRange.getBegin().isMacroID() ||
+ InitExprRange.getEnd().isMacroID())
+return;
+ Diag << FixItHint::CreateRemoval(EqualLoc)
+ << FixItHint::CreateRemoval(InitExprRange);
+ return;
+}
+
+namespace {
+
+AST_MATCHER(EnumDecl, isMacro) {
+ SourceLocation Loc = Node.getBeginLoc();
+ return Loc.isMacroID();
+}
+
+AST_MATCHER(EnumDecl, hasConsistentInitialValues) {
+ return isNoneEnumeratorsInitialized(Node) ||
+ isOnlyFirstEnumeratorInitialized(Node) ||
+ areAllEnumeratorsInitialized(Node);
+}
+
+AST_MATCHER(EnumDecl, hasZeroInitialValueForFirstEnumerator) {
+ EnumDecl::enumerator_range Enumerators = Node.enumerators();
+ if (Enumerators.empty())
+return false;
+ const EnumConstantDecl *ECD = *Enumerators.begin();
+ return isOnlyFirstEnumeratorInitialized(Node) &&
+ isInitializedByLiteral(ECD) && ECD->getInitVal().isZero();
+}
+
+/// Excludes bitfields because enumerators initialized with the result of a
+/// bitwise operator on enumeration values or any other expr that is not a
+/// potentially negative integer literal.
+/// Enumerations where it is not directly clear if they are used with
+/// bitmask, evident when enumerators are only initialized with (potentially
+/// negative) integer literals, are ignored. This is also the case when all
+/// enumerators are powers of two (e.g., 0, 1, 2).
+AST_MATCHER(EnumDecl, hasSequentialInitialValues) {
+ EnumDecl::enumerator_range Enumerators = Node.enumerators();
+ if (Enumerators.empty())
+return false;
+ const EnumConstantDecl *const FirstEnumerator = *Node.enumerator_begin();
+ llvm::APSInt PrevValue = FirstEnumerator->getInitVal();
+ if (!isInitializedByLiteral(FirstEnumerator))
+return false;
+ bool AllEnumeratorsArePowersOfTwo = true;
+ for (const EnumConstantDecl *Enumerator : llvm::drop_begin(Enumerators)) {
+const llvm::APSInt NewValue = Enumerator->getInitVal();
+if (NewValue != ++PrevValue)
+ return false;
+if (!isInitializedByLiteral(Enumerator))
+ return false;
+PrevValue = NewValue;
+AllEnumeratorsArePowersOfTwo &= NewValue.isPowerOf2();
+ }
+ return !AllEnumeratorsArePowersOfTwo;
+}
+
+} // namespace
+
+EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name,
+ ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ AllowExplicitZeroFirstInitialValue(
+ Options.get("AllowExplicitZeroFirstInitialValue", true)),
+
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,199 @@
+//===--- EnumInitialValueCheck.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 "EnumInitialValueCheck.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+static bool isNoneEnumeratorsInitialized(const EnumDecl ) {
+ return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) {
+return ECD->getInitExpr() == nullptr;
+ });
+}
+
+static bool isOnlyFirstEnumeratorInitialized(const EnumDecl ) {
+ bool IsFirst = true;
+ for (const EnumConstantDecl *ECD : Node.enumerators()) {
+if ((IsFirst && ECD->getInitExpr() == nullptr) ||
+(!IsFirst && ECD->getInitExpr() != nullptr))
+ return false;
+IsFirst = false;
+ }
+ return !IsFirst;
+}
+
+static bool areAllEnumeratorsInitialized(const EnumDecl ) {
+ return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) {
+return ECD->getInitExpr() != nullptr;
+ });
+}
+
+/// Check if \p Enumerator is initialized with a (potentially negated) \c
+/// IntegerLiteral.
+static bool isInitializedByLiteral(const EnumConstantDecl *Enumerator) {
+ const Expr *const Init = Enumerator->getInitExpr();
+ if (!Init)
+return false;
+ return Init->isIntegerConstantExpr(Enumerator->getASTContext());
+}
+
+static void cleanInitialValue(DiagnosticBuilder ,
+ const EnumConstantDecl *ECD,
+ const SourceManager ,
+ const LangOptions ) {
+ std::optional EqualToken =
utils::lexer::findNextTokenSkippingComments(
+ ECD->getLocation(), SM, LangOpts);
+ if (!EqualToken.has_value())
+return;
+ SourceLocation EqualLoc{EqualToken->getLocation()};
+ if (EqualLoc.isInvalid() || EqualLoc.isMacroID())
+return;
+ SourceRange InitExprRange = ECD->getInitExpr()->getSourceRange();
+ if (InitExprRange.isInvalid() || InitExprRange.getBegin().isMacroID() ||
+ InitExprRange.getEnd().isMacroID())
+return;
5chmidti wrote:
This check can be done before the call to `findNextTokenSkippingComments`,
which is cheaper than going through the lexer first.
https://github.com/llvm/llvm-project/pull/86129
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,199 @@
+//===--- EnumInitialValueCheck.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 "EnumInitialValueCheck.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+static bool isNoneEnumeratorsInitialized(const EnumDecl ) {
+ return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) {
+return ECD->getInitExpr() == nullptr;
+ });
+}
+
+static bool isOnlyFirstEnumeratorInitialized(const EnumDecl ) {
+ bool IsFirst = true;
+ for (const EnumConstantDecl *ECD : Node.enumerators()) {
+if ((IsFirst && ECD->getInitExpr() == nullptr) ||
+(!IsFirst && ECD->getInitExpr() != nullptr))
+ return false;
+IsFirst = false;
+ }
+ return !IsFirst;
+}
+
+static bool areAllEnumeratorsInitialized(const EnumDecl ) {
+ return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) {
+return ECD->getInitExpr() != nullptr;
+ });
+}
+
+/// Check if \p Enumerator is initialized with a (potentially negated) \c
+/// IntegerLiteral.
+static bool isInitializedByLiteral(const EnumConstantDecl *Enumerator) {
+ const Expr *const Init = Enumerator->getInitExpr();
+ if (!Init)
+return false;
+ return Init->isIntegerConstantExpr(Enumerator->getASTContext());
+}
+
+static void cleanInitialValue(DiagnosticBuilder ,
+ const EnumConstantDecl *ECD,
+ const SourceManager ,
+ const LangOptions ) {
+ std::optional EqualToken =
utils::lexer::findNextTokenSkippingComments(
+ ECD->getLocation(), SM, LangOpts);
+ if (!EqualToken.has_value())
+return;
+ SourceLocation EqualLoc{EqualToken->getLocation()};
+ if (EqualLoc.isInvalid() || EqualLoc.isMacroID())
+return;
+ SourceRange InitExprRange = ECD->getInitExpr()->getSourceRange();
5chmidti wrote:
2x `const SourceRange`
https://github.com/llvm/llvm-project/pull/86129
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,199 @@
+//===--- EnumInitialValueCheck.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 "EnumInitialValueCheck.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+static bool isNoneEnumeratorsInitialized(const EnumDecl ) {
+ return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) {
+return ECD->getInitExpr() == nullptr;
+ });
+}
+
+static bool isOnlyFirstEnumeratorInitialized(const EnumDecl ) {
+ bool IsFirst = true;
+ for (const EnumConstantDecl *ECD : Node.enumerators()) {
+if ((IsFirst && ECD->getInitExpr() == nullptr) ||
+(!IsFirst && ECD->getInitExpr() != nullptr))
+ return false;
+IsFirst = false;
+ }
+ return !IsFirst;
+}
+
+static bool areAllEnumeratorsInitialized(const EnumDecl ) {
+ return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) {
+return ECD->getInitExpr() != nullptr;
+ });
+}
+
+/// Check if \p Enumerator is initialized with a (potentially negated) \c
+/// IntegerLiteral.
+static bool isInitializedByLiteral(const EnumConstantDecl *Enumerator) {
+ const Expr *const Init = Enumerator->getInitExpr();
+ if (!Init)
+return false;
+ return Init->isIntegerConstantExpr(Enumerator->getASTContext());
+}
+
+static void cleanInitialValue(DiagnosticBuilder ,
+ const EnumConstantDecl *ECD,
+ const SourceManager ,
+ const LangOptions ) {
+ std::optional EqualToken =
utils::lexer::findNextTokenSkippingComments(
+ ECD->getLocation(), SM, LangOpts);
+ if (!EqualToken.has_value())
+return;
+ SourceLocation EqualLoc{EqualToken->getLocation()};
+ if (EqualLoc.isInvalid() || EqualLoc.isMacroID())
+return;
+ SourceRange InitExprRange = ECD->getInitExpr()->getSourceRange();
+ if (InitExprRange.isInvalid() || InitExprRange.getBegin().isMacroID() ||
+ InitExprRange.getEnd().isMacroID())
+return;
+ Diag << FixItHint::CreateRemoval(EqualLoc)
+ << FixItHint::CreateRemoval(InitExprRange);
+ return;
+}
+
+namespace {
+
+AST_MATCHER(EnumDecl, isMacro) {
+ SourceLocation Loc = Node.getBeginLoc();
+ return Loc.isMacroID();
+}
+
+AST_MATCHER(EnumDecl, hasConsistentInitialValues) {
+ return isNoneEnumeratorsInitialized(Node) ||
+ isOnlyFirstEnumeratorInitialized(Node) ||
+ areAllEnumeratorsInitialized(Node);
+}
+
+AST_MATCHER(EnumDecl, hasZeroInitialValueForFirstEnumerator) {
+ EnumDecl::enumerator_range Enumerators = Node.enumerators();
5chmidti wrote:
Please add `const`
https://github.com/llvm/llvm-project/pull/86129
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/5chmidti commented: I only have some minor comments, otherwise looks good to me https://github.com/llvm/llvm-project/pull/86129 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/86129 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add new check readability-enum-initial-value (PR #86129)
@@ -0,0 +1,199 @@
+//===--- EnumInitialValueCheck.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 "EnumInitialValueCheck.h"
+#include "../utils/LexerUtils.h"
+#include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+static bool isNoneEnumeratorsInitialized(const EnumDecl ) {
+ return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) {
+return ECD->getInitExpr() == nullptr;
+ });
+}
+
+static bool isOnlyFirstEnumeratorInitialized(const EnumDecl ) {
+ bool IsFirst = true;
+ for (const EnumConstantDecl *ECD : Node.enumerators()) {
+if ((IsFirst && ECD->getInitExpr() == nullptr) ||
+(!IsFirst && ECD->getInitExpr() != nullptr))
+ return false;
+IsFirst = false;
+ }
+ return !IsFirst;
+}
+
+static bool areAllEnumeratorsInitialized(const EnumDecl ) {
+ return llvm::all_of(Node.enumerators(), [](const EnumConstantDecl *ECD) {
+return ECD->getInitExpr() != nullptr;
+ });
+}
+
+/// Check if \p Enumerator is initialized with a (potentially negated) \c
+/// IntegerLiteral.
+static bool isInitializedByLiteral(const EnumConstantDecl *Enumerator) {
+ const Expr *const Init = Enumerator->getInitExpr();
+ if (!Init)
+return false;
+ return Init->isIntegerConstantExpr(Enumerator->getASTContext());
+}
+
+static void cleanInitialValue(DiagnosticBuilder ,
+ const EnumConstantDecl *ECD,
+ const SourceManager ,
+ const LangOptions ) {
+ std::optional EqualToken =
utils::lexer::findNextTokenSkippingComments(
+ ECD->getLocation(), SM, LangOpts);
+ if (!EqualToken.has_value())
+return;
+ SourceLocation EqualLoc{EqualToken->getLocation()};
+ if (EqualLoc.isInvalid() || EqualLoc.isMacroID())
+return;
+ SourceRange InitExprRange = ECD->getInitExpr()->getSourceRange();
+ if (InitExprRange.isInvalid() || InitExprRange.getBegin().isMacroID() ||
+ InitExprRange.getEnd().isMacroID())
+return;
+ Diag << FixItHint::CreateRemoval(EqualLoc)
+ << FixItHint::CreateRemoval(InitExprRange);
+ return;
+}
+
+namespace {
+
+AST_MATCHER(EnumDecl, isMacro) {
+ SourceLocation Loc = Node.getBeginLoc();
+ return Loc.isMacroID();
+}
+
+AST_MATCHER(EnumDecl, hasConsistentInitialValues) {
+ return isNoneEnumeratorsInitialized(Node) ||
+ isOnlyFirstEnumeratorInitialized(Node) ||
+ areAllEnumeratorsInitialized(Node);
+}
+
+AST_MATCHER(EnumDecl, hasZeroInitialValueForFirstEnumerator) {
+ EnumDecl::enumerator_range Enumerators = Node.enumerators();
+ if (Enumerators.empty())
+return false;
+ const EnumConstantDecl *ECD = *Enumerators.begin();
+ return isOnlyFirstEnumeratorInitialized(Node) &&
+ isInitializedByLiteral(ECD) && ECD->getInitVal().isZero();
+}
+
+/// Excludes bitfields because enumerators initialized with the result of a
+/// bitwise operator on enumeration values or any other expr that is not a
+/// potentially negative integer literal.
+/// Enumerations where it is not directly clear if they are used with
+/// bitmask, evident when enumerators are only initialized with (potentially
+/// negative) integer literals, are ignored. This is also the case when all
+/// enumerators are powers of two (e.g., 0, 1, 2).
+AST_MATCHER(EnumDecl, hasSequentialInitialValues) {
+ EnumDecl::enumerator_range Enumerators = Node.enumerators();
+ if (Enumerators.empty())
+return false;
+ const EnumConstantDecl *const FirstEnumerator = *Node.enumerator_begin();
+ llvm::APSInt PrevValue = FirstEnumerator->getInitVal();
+ if (!isInitializedByLiteral(FirstEnumerator))
+return false;
+ bool AllEnumeratorsArePowersOfTwo = true;
+ for (const EnumConstantDecl *Enumerator : llvm::drop_begin(Enumerators)) {
+const llvm::APSInt NewValue = Enumerator->getInitVal();
+if (NewValue != ++PrevValue)
+ return false;
+if (!isInitializedByLiteral(Enumerator))
+ return false;
+PrevValue = NewValue;
+AllEnumeratorsArePowersOfTwo &= NewValue.isPowerOf2();
+ }
+ return !AllEnumeratorsArePowersOfTwo;
+}
+
+} // namespace
+
+EnumInitialValueCheck::EnumInitialValueCheck(StringRef Name,
+ ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ AllowExplicitZeroFirstInitialValue(
+ Options.get("AllowExplicitZeroFirstInitialValue", true)),
+
[clang-tools-extra] Add clang-tidy check readability-math-missing-parentheses (PR #84481)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/84481 ___ 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 readability-math-missing-parentheses (PR #84481)
https://github.com/5chmidti commented: I had an idea to simplify `addParantheses` and `check` a bit, otherwise this looks good. https://github.com/llvm/llvm-project/pull/84481 ___ 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 readability-math-missing-parentheses (PR #84481)
@@ -0,0 +1,112 @@
+//===--- MathMissingParenthesesCheck.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 "MathMissingParenthesesCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+void MathMissingParenthesesCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(binaryOperator(unless(hasParent(binaryOperator())),
+unless(allOf(hasOperatorName("&&"),
+ hasOperatorName("||"))),
+hasDescendant(binaryOperator()))
+ .bind("binOp"),
+ this);
+}
+
+static int getPrecedence(const BinaryOperator *BinOp) {
+ if (!BinOp)
+return 0;
+ switch (BinOp->getOpcode()) {
+ case BO_Mul:
+ case BO_Div:
+ case BO_Rem:
+return 5;
+ case BO_Add:
+ case BO_Sub:
+return 4;
+ case BO_And:
+return 3;
+ case BO_Xor:
+return 2;
+ case BO_Or:
+return 1;
+ default:
+return 0;
+ }
+}
+static bool addParantheses(
+const BinaryOperator *BinOp, const BinaryOperator *ParentBinOp,
+std::vector<
+std::pair>>
+,
+const clang::SourceManager , const clang::LangOptions ) {
+ bool NeedToDiagnose = false;
+ if (!BinOp)
+return NeedToDiagnose;
+
+ if (ParentBinOp != nullptr &&
+ getPrecedence(BinOp) != getPrecedence(ParentBinOp)) {
+NeedToDiagnose = true;
+const clang::SourceLocation StartLoc = BinOp->getBeginLoc();
+clang::SourceLocation EndLoc =
+clang::Lexer::getLocForEndOfToken(BinOp->getEndLoc(), 0, SM, LangOpts);
+Insertions.push_back(
+{clang::SourceRange(StartLoc, EndLoc), {BinOp, ParentBinOp}});
5chmidti wrote:
Instead of filling this vector, you could move emitting the diagnostic itself
into this function by passing it the check itself like in
https://github.com/llvm/llvm-project/blob/6aa53888a8e8a6e3f0bd279539703f4d4701b4e7/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp#L377-L379
Because whatever you push into the vector will be diagnosed. This would remove
the need for the vector and the bool return type.
https://github.com/llvm/llvm-project/pull/84481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/85572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/85572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,342 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ std::vector Args;
+};
+
+static const FindArgsResult findArgs(const CallExpr *Call);
+static std::vector>
+getCommentRanges(const std::string );
+static bool
+isPositionInComment(int position,
+const std::vector> );
+static void
+removeCharacterFromSource(std::string ,
+ const std::vector>
,
+ char Character, const CallExpr *InnerCall,
+ std::vector , bool ReverseSearch);
+static SourceLocation
+getLocForEndOfToken(const Expr *expr, const MatchFinder::MatchResult );
+static const std::vector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult );
+
+MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ Inserter(Options.getLocalOrGlobal("IncludeStyle",
+utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+
+void MinMaxUseInitializerListCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+}
+
+void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) {
+ auto CreateMatcher = [](const std::string ) {
+auto FuncDecl = functionDecl(hasName(FunctionName));
+auto Expression = callExpr(callee(FuncDecl));
+
+return callExpr(callee(FuncDecl),
+anyOf(hasArgument(0, Expression),
+ hasArgument(1, Expression),
+ hasArgument(0, cxxStdInitializerListExpr())),
+unless(hasParent(Expression)))
+.bind("topCall");
+ };
+
+ Finder->addMatcher(CreateMatcher("::std::max"), this);
+ Finder->addMatcher(CreateMatcher("::std::min"), this);
+}
+
+void MinMaxUseInitializerListCheck::registerPPCallbacks(
+const SourceManager , Preprocessor *PP, Preprocessor *ModuleExpanderPP)
{
+ Inserter.registerPreprocessor(PP);
+}
+
+void MinMaxUseInitializerListCheck::check(
+const MatchFinder::MatchResult ) {
+
+ const auto *TopCall = Match.Nodes.getNodeAs("topCall");
+
+ // if topcall in macro ignore
+ if (TopCall->getBeginLoc().isMacroID()) {
+return;
+ }
+
+ FindArgsResult Result = findArgs(TopCall);
+ const std::vector Replacement =
+ generateReplacement(Match, TopCall, Result);
+
+ // if only changes are inserting '{' and '}' then ignore
+ if (Replacement.size() <= 2) {
+return;
+ }
+
+ const DiagnosticBuilder Diagnostic =
+ diag(TopCall->getBeginLoc(),
+ "do not use nested 'std::%0' calls, use initializer lists instead")
+ << TopCall->getDirectCallee()->getName()
+ << Inserter.createIncludeInsertion(
+ Match.SourceManager->getFileID(TopCall->getBeginLoc()),
+ "");
+
+ for (const auto : Replacement) {
+Diagnostic << FixIt;
+ }
+}
+
+static const FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ if (Call->getNumArgs() == 3) {
+auto ArgIterator = Call->arguments().begin();
+std::advance(ArgIterator, 2);
+Result.Compare = *ArgIterator;
+ } else {
+auto ArgIterator = Call->arguments().begin();
+
+if (const auto *InitListExpr =
+dyn_cast(*ArgIterator)) {
+ if (const auto *TempExpr =
+ dyn_cast(InitListExpr->getSubExpr())) {
+if (const auto *InitList =
+dyn_cast(TempExpr->getSubExpr())) {
+ for (const Expr *Init : InitList->inits()) {
+Result.Args.push_back(Init);
+ }
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+Result.Compare = *ArgIterator;
+ }
+ return Result;
+}
+ }
+}
+ }
+
+ for (const Expr *Arg : Call->arguments()) {
+if (!Result.First)
+ Result.First = Arg;
+
+if (Arg == Result.Compare)
+ continue;
+
+Result.Args.push_back(Arg);
+Result.Last =
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,342 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ std::vector Args;
+};
+
+static const FindArgsResult findArgs(const CallExpr *Call);
+static std::vector>
+getCommentRanges(const std::string );
+static bool
+isPositionInComment(int position,
+const std::vector> );
+static void
+removeCharacterFromSource(std::string ,
+ const std::vector>
,
+ char Character, const CallExpr *InnerCall,
+ std::vector , bool ReverseSearch);
+static SourceLocation
+getLocForEndOfToken(const Expr *expr, const MatchFinder::MatchResult );
+static const std::vector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult );
+
+MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ Inserter(Options.getLocalOrGlobal("IncludeStyle",
+utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+
+void MinMaxUseInitializerListCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+}
+
+void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) {
+ auto CreateMatcher = [](const std::string ) {
+auto FuncDecl = functionDecl(hasName(FunctionName));
+auto Expression = callExpr(callee(FuncDecl));
+
+return callExpr(callee(FuncDecl),
+anyOf(hasArgument(0, Expression),
+ hasArgument(1, Expression),
+ hasArgument(0, cxxStdInitializerListExpr())),
+unless(hasParent(Expression)))
+.bind("topCall");
+ };
+
+ Finder->addMatcher(CreateMatcher("::std::max"), this);
+ Finder->addMatcher(CreateMatcher("::std::min"), this);
+}
+
+void MinMaxUseInitializerListCheck::registerPPCallbacks(
+const SourceManager , Preprocessor *PP, Preprocessor *ModuleExpanderPP)
{
+ Inserter.registerPreprocessor(PP);
+}
+
+void MinMaxUseInitializerListCheck::check(
+const MatchFinder::MatchResult ) {
+
+ const auto *TopCall = Match.Nodes.getNodeAs("topCall");
+
+ // if topcall in macro ignore
+ if (TopCall->getBeginLoc().isMacroID()) {
+return;
+ }
+
+ FindArgsResult Result = findArgs(TopCall);
+ const std::vector Replacement =
+ generateReplacement(Match, TopCall, Result);
+
+ // if only changes are inserting '{' and '}' then ignore
+ if (Replacement.size() <= 2) {
+return;
+ }
+
+ const DiagnosticBuilder Diagnostic =
+ diag(TopCall->getBeginLoc(),
+ "do not use nested 'std::%0' calls, use initializer lists instead")
+ << TopCall->getDirectCallee()->getName()
+ << Inserter.createIncludeInsertion(
+ Match.SourceManager->getFileID(TopCall->getBeginLoc()),
+ "");
+
+ for (const auto : Replacement) {
+Diagnostic << FixIt;
+ }
+}
+
+static const FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ if (Call->getNumArgs() == 3) {
+auto ArgIterator = Call->arguments().begin();
+std::advance(ArgIterator, 2);
+Result.Compare = *ArgIterator;
+ } else {
+auto ArgIterator = Call->arguments().begin();
+
+if (const auto *InitListExpr =
+dyn_cast(*ArgIterator)) {
+ if (const auto *TempExpr =
+ dyn_cast(InitListExpr->getSubExpr())) {
+if (const auto *InitList =
+dyn_cast(TempExpr->getSubExpr())) {
+ for (const Expr *Init : InitList->inits()) {
+Result.Args.push_back(Init);
+ }
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+Result.Compare = *ArgIterator;
+ }
+ return Result;
+}
+ }
+}
+ }
+
+ for (const Expr *Arg : Call->arguments()) {
+if (!Result.First)
+ Result.First = Arg;
+
+if (Arg == Result.Compare)
+ continue;
+
+Result.Args.push_back(Arg);
+Result.Last =
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,337 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ std::vector Args;
+};
+
+static const FindArgsResult findArgs(const CallExpr *Call);
+static std::vector>
+getCommentRanges(const std::string );
+static bool
+isPositionInComment(int position,
+const std::vector> );
+static void
+removeCharacterFromSource(std::string ,
+ const std::vector>
,
+ char Character, const CallExpr *InnerCall,
+ std::vector , bool ReverseSearch);
+static SourceLocation
+getLocForEndOfToken(const Expr *expr, const MatchFinder::MatchResult );
+static const std::vector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult );
+
+MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ Inserter(Options.getLocalOrGlobal("IncludeStyle",
+utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+
+void MinMaxUseInitializerListCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+}
+
+void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) {
+ auto CreateMatcher = [](const std::string ) {
+auto FuncDecl = functionDecl(hasName(FunctionName));
+auto Expression = callExpr(callee(FuncDecl));
+
+return callExpr(callee(FuncDecl),
+anyOf(hasArgument(0, Expression),
+ hasArgument(1, Expression),
+ hasArgument(0, cxxStdInitializerListExpr())),
+unless(hasParent(Expression)))
+.bind("topCall");
+ };
+
+ Finder->addMatcher(CreateMatcher("::std::max"), this);
+ Finder->addMatcher(CreateMatcher("::std::min"), this);
+}
+
+void MinMaxUseInitializerListCheck::registerPPCallbacks(
+const SourceManager , Preprocessor *PP, Preprocessor *ModuleExpanderPP)
{
+ Inserter.registerPreprocessor(PP);
+}
+
+void MinMaxUseInitializerListCheck::check(
+const MatchFinder::MatchResult ) {
+
+ const auto *TopCall = Match.Nodes.getNodeAs("topCall");
+
+ FindArgsResult Result = findArgs(TopCall);
5chmidti wrote:
`const FindArgsResult`
https://github.com/llvm/llvm-project/pull/85572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,342 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ std::vector Args;
+};
+
+static const FindArgsResult findArgs(const CallExpr *Call);
+static std::vector>
+getCommentRanges(const std::string );
+static bool
+isPositionInComment(int position,
+const std::vector> );
+static void
+removeCharacterFromSource(std::string ,
+ const std::vector>
,
+ char Character, const CallExpr *InnerCall,
+ std::vector , bool ReverseSearch);
+static SourceLocation
+getLocForEndOfToken(const Expr *expr, const MatchFinder::MatchResult );
+static const std::vector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult );
5chmidti wrote:
Don't return values (not references) by const, same goes for the definition of
these functions.
You comment handling functions are also not needed, see comments on
`generateReplacement`
https://github.com/llvm/llvm-project/pull/85572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,268 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ std::vector Args;
+};
+
+static const FindArgsResult findArgs(const CallExpr *Call);
+static const std::vector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult );
+
+MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ Inserter(Options.getLocalOrGlobal("IncludeStyle",
+utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+
+void MinMaxUseInitializerListCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+}
+
+void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) {
+ auto CreateMatcher = [](const std::string ) {
+auto FuncDecl = functionDecl(hasName(FunctionName));
+auto Expression = callExpr(callee(FuncDecl));
+
+return callExpr(callee(FuncDecl),
+anyOf(hasArgument(0, Expression),
+ hasArgument(1, Expression),
+ hasArgument(0, cxxStdInitializerListExpr())),
+unless(hasParent(Expression)))
+.bind("topCall");
+ };
+
+ Finder->addMatcher(CreateMatcher("::std::max"), this);
+ Finder->addMatcher(CreateMatcher("::std::min"), this);
+}
+
+void MinMaxUseInitializerListCheck::registerPPCallbacks(
+const SourceManager , Preprocessor *PP, Preprocessor *ModuleExpanderPP)
{
+ Inserter.registerPreprocessor(PP);
+}
+
+void MinMaxUseInitializerListCheck::check(
+const MatchFinder::MatchResult ) {
+
+ const auto *TopCall = Match.Nodes.getNodeAs("topCall");
+
+ FindArgsResult Result = findArgs(TopCall);
+ const std::vector Replacement =
+ generateReplacement(Match, TopCall, Result);
+
+ if (Replacement.size() <= 2) {
+return;
+ }
+
+ const DiagnosticBuilder Diagnostic =
+ diag(TopCall->getBeginLoc(),
+ "do not use nested 'std::%0' calls, use initializer lists instead")
+ << TopCall->getDirectCallee()->getName()
+ << Inserter.createIncludeInsertion(
+ Match.SourceManager->getFileID(TopCall->getBeginLoc()),
+ "");
+
+ for (const auto : Replacement) {
+Diagnostic << FixIt;
+ }
+}
+
+static const FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ if (Call->getNumArgs() == 3) {
+auto ArgIterator = Call->arguments().begin();
+std::advance(ArgIterator, 2);
+Result.Compare = *ArgIterator;
+ } else {
+auto ArgIterator = Call->arguments().begin();
+
+if (const auto *InitListExpr =
+dyn_cast(*ArgIterator)) {
+ if (const auto *TempExpr =
+ dyn_cast(InitListExpr->getSubExpr())) {
+if (const auto *InitList =
+dyn_cast(TempExpr->getSubExpr())) {
+ for (const Expr *Init : InitList->inits()) {
+Result.Args.push_back(Init);
+ }
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+Result.Compare = *ArgIterator;
+ }
+ return Result;
+}
+ }
+}
+ }
5chmidti wrote:
You can skip the cast (and the associated if-stmt) to
`MaterializeTemporaryExpr` by writing
`dyn_cast(TempExpr->getSubExpr()->IgnoreImplicit())` in
the if statement of `InitList`.
https://github.com/llvm/llvm-project/pull/85572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
https://github.com/5chmidti requested changes to this pull request. I've added a comment about the string parsing and comments in `generateReplacement` on how to do this in a better way, which should help you. Also, if-statements with a single statement branches should not have braces. https://github.com/llvm/llvm-project/pull/85572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,342 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ std::vector Args;
+};
+
+static const FindArgsResult findArgs(const CallExpr *Call);
+static std::vector>
+getCommentRanges(const std::string );
+static bool
+isPositionInComment(int position,
+const std::vector> );
+static void
+removeCharacterFromSource(std::string ,
+ const std::vector>
,
+ char Character, const CallExpr *InnerCall,
+ std::vector , bool ReverseSearch);
+static SourceLocation
+getLocForEndOfToken(const Expr *expr, const MatchFinder::MatchResult );
+static const std::vector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult );
+
+MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ Inserter(Options.getLocalOrGlobal("IncludeStyle",
+utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+
+void MinMaxUseInitializerListCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+}
+
+void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) {
+ auto CreateMatcher = [](const std::string ) {
+auto FuncDecl = functionDecl(hasName(FunctionName));
+auto Expression = callExpr(callee(FuncDecl));
+
+return callExpr(callee(FuncDecl),
+anyOf(hasArgument(0, Expression),
+ hasArgument(1, Expression),
+ hasArgument(0, cxxStdInitializerListExpr())),
+unless(hasParent(Expression)))
+.bind("topCall");
+ };
+
+ Finder->addMatcher(CreateMatcher("::std::max"), this);
+ Finder->addMatcher(CreateMatcher("::std::min"), this);
+}
+
+void MinMaxUseInitializerListCheck::registerPPCallbacks(
+const SourceManager , Preprocessor *PP, Preprocessor *ModuleExpanderPP)
{
+ Inserter.registerPreprocessor(PP);
+}
+
+void MinMaxUseInitializerListCheck::check(
+const MatchFinder::MatchResult ) {
+
+ const auto *TopCall = Match.Nodes.getNodeAs("topCall");
+
+ // if topcall in macro ignore
+ if (TopCall->getBeginLoc().isMacroID()) {
+return;
+ }
+
+ FindArgsResult Result = findArgs(TopCall);
+ const std::vector Replacement =
+ generateReplacement(Match, TopCall, Result);
+
+ // if only changes are inserting '{' and '}' then ignore
+ if (Replacement.size() <= 2) {
+return;
+ }
+
+ const DiagnosticBuilder Diagnostic =
+ diag(TopCall->getBeginLoc(),
+ "do not use nested 'std::%0' calls, use initializer lists instead")
5chmidti wrote:
This should spell `"do not use nested 'std::%0' calls, use an initializer list
instead"` instead, because the replacement code only uses one initializer list,
not multiple.
https://github.com/llvm/llvm-project/pull/85572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,342 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ std::vector Args;
+};
+
+static const FindArgsResult findArgs(const CallExpr *Call);
+static std::vector>
+getCommentRanges(const std::string );
+static bool
+isPositionInComment(int position,
+const std::vector> );
+static void
+removeCharacterFromSource(std::string ,
+ const std::vector>
,
+ char Character, const CallExpr *InnerCall,
+ std::vector , bool ReverseSearch);
+static SourceLocation
+getLocForEndOfToken(const Expr *expr, const MatchFinder::MatchResult );
+static const std::vector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult );
+
+MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ Inserter(Options.getLocalOrGlobal("IncludeStyle",
+utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+
+void MinMaxUseInitializerListCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+}
+
+void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) {
+ auto CreateMatcher = [](const std::string ) {
+auto FuncDecl = functionDecl(hasName(FunctionName));
+auto Expression = callExpr(callee(FuncDecl));
+
+return callExpr(callee(FuncDecl),
+anyOf(hasArgument(0, Expression),
+ hasArgument(1, Expression),
+ hasArgument(0, cxxStdInitializerListExpr())),
+unless(hasParent(Expression)))
+.bind("topCall");
+ };
+
+ Finder->addMatcher(CreateMatcher("::std::max"), this);
+ Finder->addMatcher(CreateMatcher("::std::min"), this);
+}
+
+void MinMaxUseInitializerListCheck::registerPPCallbacks(
+const SourceManager , Preprocessor *PP, Preprocessor *ModuleExpanderPP)
{
+ Inserter.registerPreprocessor(PP);
+}
+
+void MinMaxUseInitializerListCheck::check(
+const MatchFinder::MatchResult ) {
+
+ const auto *TopCall = Match.Nodes.getNodeAs("topCall");
+
+ // if topcall in macro ignore
+ if (TopCall->getBeginLoc().isMacroID()) {
+return;
+ }
+
+ FindArgsResult Result = findArgs(TopCall);
+ const std::vector Replacement =
+ generateReplacement(Match, TopCall, Result);
+
+ // if only changes are inserting '{' and '}' then ignore
+ if (Replacement.size() <= 2) {
+return;
+ }
+
+ const DiagnosticBuilder Diagnostic =
+ diag(TopCall->getBeginLoc(),
+ "do not use nested 'std::%0' calls, use initializer lists instead")
+ << TopCall->getDirectCallee()->getName()
+ << Inserter.createIncludeInsertion(
+ Match.SourceManager->getFileID(TopCall->getBeginLoc()),
+ "");
+
+ for (const auto : Replacement) {
+Diagnostic << FixIt;
+ }
+}
+
+static const FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ if (Call->getNumArgs() == 3) {
+auto ArgIterator = Call->arguments().begin();
+std::advance(ArgIterator, 2);
+Result.Compare = *ArgIterator;
+ } else {
+auto ArgIterator = Call->arguments().begin();
+
+if (const auto *InitListExpr =
+dyn_cast(*ArgIterator)) {
+ if (const auto *TempExpr =
+ dyn_cast(InitListExpr->getSubExpr())) {
+if (const auto *InitList =
+dyn_cast(TempExpr->getSubExpr())) {
+ for (const Expr *Init : InitList->inits()) {
+Result.Args.push_back(Init);
+ }
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+Result.Compare = *ArgIterator;
+ }
+ return Result;
+}
+ }
+}
+ }
+
+ for (const Expr *Arg : Call->arguments()) {
+if (!Result.First)
+ Result.First = Arg;
+
+if (Arg == Result.Compare)
+ continue;
+
+Result.Args.push_back(Arg);
+Result.Last =
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,337 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "../utils/ASTUtils.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+struct FindArgsResult {
+ const Expr *First;
+ const Expr *Last;
+ const Expr *Compare;
+ std::vector Args;
+};
+
+static const FindArgsResult findArgs(const CallExpr *Call);
+static std::vector>
+getCommentRanges(const std::string );
+static bool
+isPositionInComment(int position,
+const std::vector> );
+static void
+removeCharacterFromSource(std::string ,
+ const std::vector>
,
+ char Character, const CallExpr *InnerCall,
+ std::vector , bool ReverseSearch);
+static SourceLocation
+getLocForEndOfToken(const Expr *expr, const MatchFinder::MatchResult );
+static const std::vector
+generateReplacement(const MatchFinder::MatchResult ,
+const CallExpr *TopCall, const FindArgsResult );
+
+MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ Inserter(Options.getLocalOrGlobal("IncludeStyle",
+utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+
+void MinMaxUseInitializerListCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+}
+
+void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) {
+ auto CreateMatcher = [](const std::string ) {
+auto FuncDecl = functionDecl(hasName(FunctionName));
+auto Expression = callExpr(callee(FuncDecl));
+
+return callExpr(callee(FuncDecl),
+anyOf(hasArgument(0, Expression),
+ hasArgument(1, Expression),
+ hasArgument(0, cxxStdInitializerListExpr())),
+unless(hasParent(Expression)))
+.bind("topCall");
+ };
+
+ Finder->addMatcher(CreateMatcher("::std::max"), this);
+ Finder->addMatcher(CreateMatcher("::std::min"), this);
+}
+
+void MinMaxUseInitializerListCheck::registerPPCallbacks(
+const SourceManager , Preprocessor *PP, Preprocessor *ModuleExpanderPP)
{
+ Inserter.registerPreprocessor(PP);
+}
+
+void MinMaxUseInitializerListCheck::check(
+const MatchFinder::MatchResult ) {
+
+ const auto *TopCall = Match.Nodes.getNodeAs("topCall");
+
+ FindArgsResult Result = findArgs(TopCall);
+ const std::vector Replacement =
+ generateReplacement(Match, TopCall, Result);
+
+ // if only changes are inserting '{' and '}' then ignore
+ if (Replacement.size() <= 2) {
+return;
+ }
+
+ const DiagnosticBuilder Diagnostic =
+ diag(TopCall->getBeginLoc(),
+ "do not use nested 'std::%0' calls, use initializer lists instead")
+ << TopCall->getDirectCallee()->getName()
+ << Inserter.createIncludeInsertion(
+ Match.SourceManager->getFileID(TopCall->getBeginLoc()),
+ "");
+
+ for (const auto : Replacement) {
+Diagnostic << FixIt;
+ }
+}
+
+static const FindArgsResult findArgs(const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ if (Call->getNumArgs() == 3) {
+auto ArgIterator = Call->arguments().begin();
+std::advance(ArgIterator, 2);
+Result.Compare = *ArgIterator;
+ } else {
+auto ArgIterator = Call->arguments().begin();
+
+if (const auto *InitListExpr =
+dyn_cast(*ArgIterator)) {
+ if (const auto *TempExpr =
+ dyn_cast(InitListExpr->getSubExpr())) {
+if (const auto *InitList =
+dyn_cast(TempExpr->getSubExpr())) {
+ for (const Expr *Init : InitList->inits()) {
+Result.Args.push_back(Init);
+ }
+ Result.First = *ArgIterator;
+ Result.Last = *ArgIterator;
+
+ std::advance(ArgIterator, 1);
+ if (ArgIterator != Call->arguments().end()) {
+Result.Compare = *ArgIterator;
+ }
+ return Result;
+}
+ }
+}
+ }
+
+ for (const Expr *Arg : Call->arguments()) {
+if (!Result.First)
+ Result.First = Arg;
+
+if (Arg == Result.Compare)
+ continue;
+
+Result.Args.push_back(Arg);
+Result.Last = Arg;
+ }
+
+ return Result;
+}
+
+static std::vector>
+getCommentRanges(const std::string ) {
+
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/85572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,26 @@ +.. title:: clang-tidy - modernize-min-max-use-initializer-list + +modernize-min-max-use-initializer-list +== + +Replaces nested ``std::min`` and ``std::max`` calls with an initializer list where applicable. + +For instance, consider the following code: + +.. code-block:: cpp + + int a = std::max(std::max(i, j), k); + +`modernize-min-max-use-initializer-list` check will transform the above code to: 5chmidti wrote: Add `The` to the start of this sentence, maybe drop the check name. https://github.com/llvm/llvm-project/pull/85572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Improve performance of google-runtime-int (PR #86596)
https://github.com/5chmidti approved this pull request. LGTM. That's a very nice speedup :) https://github.com/llvm/llvm-project/pull/86596 ___ 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 fix-it overlaps in readability-static-definition-in-anonymous-namespace (PR #86599)
https://github.com/5chmidti approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/86599 ___ 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 readability-math-missing-parentheses (PR #84481)
@@ -0,0 +1,25 @@ +.. title:: clang-tidy - readability-math-missing-parentheses + +readability-math-missing-parentheses + + +Check for missing parentheses in mathematical expressions that involve operators +of different priorities. Parentheses in mathematical expressions clarify the order +of operations, especially with different-priority operators. Lengthy or multiline +expressions can obscure this order, leading to coding errors. IDEs can aid clarity +by highlighting parentheses. Explicitly using parentheses also clarify what the 5chmidti wrote: Maybe I didn't select the correct lines, given that GitHub shows 4 lines here, my bad. I only meant the second `clarify`, not both. - `Parentheses in mathematical expressions clarify` - `Explicitly using parentheses also clarifies` https://github.com/llvm/llvm-project/pull/84481 ___ 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 expresions in unevaluated context in bugprone-inc-dec-in-conditions (PR #85849)
https://github.com/5chmidti approved this pull request. https://github.com/llvm/llvm-project/pull/85849 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
https://github.com/5chmidti edited https://github.com/llvm/llvm-project/pull/85572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,199 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ Inserter(Options.getLocalOrGlobal("IncludeStyle",
+utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+
+void MinMaxUseInitializerListCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+}
+
+void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(
+ callee(functionDecl(hasName("::std::max"))),
+ anyOf(hasArgument(
+0, callExpr(callee(functionDecl(hasName("::std::max"),
+hasArgument(
+1, callExpr(callee(functionDecl(hasName("::std::max")),
+ unless(
+
hasParent(callExpr(callee(functionDecl(hasName("::std::max")))
+ .bind("topCall"),
+ this);
+
+ Finder->addMatcher(
+ callExpr(
+ callee(functionDecl(hasName("::std::min"))),
+ anyOf(hasArgument(
+0, callExpr(callee(functionDecl(hasName("::std::min"),
+hasArgument(
+1, callExpr(callee(functionDecl(hasName("::std::min")),
+ unless(
+
hasParent(callExpr(callee(functionDecl(hasName("::std::min")))
+ .bind("topCall"),
+ this);
+}
+
+void MinMaxUseInitializerListCheck::registerPPCallbacks(
+const SourceManager , Preprocessor *PP, Preprocessor *ModuleExpanderPP)
{
+ Inserter.registerPreprocessor(PP);
+}
+
+void MinMaxUseInitializerListCheck::check(
+const MatchFinder::MatchResult ) {
+ const CallExpr *TopCall = Match.Nodes.getNodeAs("topCall");
+ MinMaxUseInitializerListCheck::FindArgsResult Result =
+ findArgs(Match, TopCall);
+
+ if (!Result.First || !Result.Last || Result.Args.size() <= 2) {
+return;
+ }
+
+ std::string ReplacementText = generateReplacement(Match, TopCall, Result);
+
+ diag(TopCall->getBeginLoc(),
+ "do not use nested std::%0 calls, use %1 instead")
+ << TopCall->getDirectCallee()->getName() << ReplacementText
+ << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(TopCall->getBeginLoc(),
+TopCall->getEndLoc()),
+ ReplacementText)
+ << Inserter.createMainFileIncludeInsertion("");
5chmidti wrote:
The location of `TopCall` may be inside a header, so this include would be in
the wrong file. Please use
`createIncludeInsertion(Match.SourceManager->getFileID(TopCall->getBeginLoc()),
"")`
https://github.com/llvm/llvm-project/pull/85572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,199 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ Inserter(Options.getLocalOrGlobal("IncludeStyle",
+utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+
+void MinMaxUseInitializerListCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+}
+
+void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(
+ callee(functionDecl(hasName("::std::max"))),
+ anyOf(hasArgument(
+0, callExpr(callee(functionDecl(hasName("::std::max"),
+hasArgument(
+1, callExpr(callee(functionDecl(hasName("::std::max")),
+ unless(
+
hasParent(callExpr(callee(functionDecl(hasName("::std::max")))
+ .bind("topCall"),
+ this);
+
+ Finder->addMatcher(
+ callExpr(
+ callee(functionDecl(hasName("::std::min"))),
+ anyOf(hasArgument(
+0, callExpr(callee(functionDecl(hasName("::std::min"),
+hasArgument(
+1, callExpr(callee(functionDecl(hasName("::std::min")),
+ unless(
+
hasParent(callExpr(callee(functionDecl(hasName("::std::min")))
+ .bind("topCall"),
+ this);
+}
+
+void MinMaxUseInitializerListCheck::registerPPCallbacks(
+const SourceManager , Preprocessor *PP, Preprocessor *ModuleExpanderPP)
{
+ Inserter.registerPreprocessor(PP);
+}
+
+void MinMaxUseInitializerListCheck::check(
+const MatchFinder::MatchResult ) {
+ const CallExpr *TopCall = Match.Nodes.getNodeAs("topCall");
+ MinMaxUseInitializerListCheck::FindArgsResult Result =
+ findArgs(Match, TopCall);
+
+ if (!Result.First || !Result.Last || Result.Args.size() <= 2) {
+return;
+ }
+
+ std::string ReplacementText = generateReplacement(Match, TopCall, Result);
+
+ diag(TopCall->getBeginLoc(),
+ "do not use nested std::%0 calls, use %1 instead")
+ << TopCall->getDirectCallee()->getName() << ReplacementText
+ << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(TopCall->getBeginLoc(),
+TopCall->getEndLoc()),
+ ReplacementText)
+ << Inserter.createMainFileIncludeInsertion("");
+}
+
+MinMaxUseInitializerListCheck::FindArgsResult
+MinMaxUseInitializerListCheck::findArgs(const MatchFinder::MatchResult ,
+const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ if (Call->getNumArgs() > 2) {
+auto ArgIterator = Call->arguments().begin();
+std::advance(ArgIterator, 2);
+Result.Compare = *ArgIterator;
+ }
+
+ for (const Expr *Arg : Call->arguments()) {
+if (!Result.First)
+ Result.First = Arg;
+
+const CallExpr *InnerCall = dyn_cast(Arg);
+if (InnerCall && InnerCall->getDirectCallee() &&
+InnerCall->getDirectCallee()->getNameAsString() ==
+Call->getDirectCallee()->getNameAsString()) {
+ FindArgsResult InnerResult = findArgs(Match, InnerCall);
+
+ bool processInnerResult = false;
+
+ if (!Result.Compare && !InnerResult.Compare)
+processInnerResult = true;
+ else if (Result.Compare && InnerResult.Compare &&
+ Lexer::getSourceText(CharSourceRange::getTokenRange(
+Result.Compare->getSourceRange()),
+*Match.SourceManager,
+Match.Context->getLangOpts()) ==
+ Lexer::getSourceText(
+ CharSourceRange::getTokenRange(
+ InnerResult.Compare->getSourceRange()),
+ *Match.SourceManager, Match.Context->getLangOpts()))
+processInnerResult = true;
+
+ if (processInnerResult) {
+Result.Args.insert(Result.Args.end(), InnerResult.Args.begin(),
+ InnerResult.Args.end());
+continue;
+ }
+}
+
+if (Arg == Result.Compare)
+ continue;
+
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,199 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ Inserter(Options.getLocalOrGlobal("IncludeStyle",
+utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+
+void MinMaxUseInitializerListCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+}
+
+void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(
+ callee(functionDecl(hasName("::std::max"))),
+ anyOf(hasArgument(
+0, callExpr(callee(functionDecl(hasName("::std::max"),
+hasArgument(
+1, callExpr(callee(functionDecl(hasName("::std::max")),
+ unless(
+
hasParent(callExpr(callee(functionDecl(hasName("::std::max")))
+ .bind("topCall"),
+ this);
+
+ Finder->addMatcher(
+ callExpr(
+ callee(functionDecl(hasName("::std::min"))),
+ anyOf(hasArgument(
+0, callExpr(callee(functionDecl(hasName("::std::min"),
+hasArgument(
+1, callExpr(callee(functionDecl(hasName("::std::min")),
+ unless(
+
hasParent(callExpr(callee(functionDecl(hasName("::std::min")))
+ .bind("topCall"),
+ this);
+}
+
+void MinMaxUseInitializerListCheck::registerPPCallbacks(
+const SourceManager , Preprocessor *PP, Preprocessor *ModuleExpanderPP)
{
+ Inserter.registerPreprocessor(PP);
+}
+
+void MinMaxUseInitializerListCheck::check(
+const MatchFinder::MatchResult ) {
+ const CallExpr *TopCall = Match.Nodes.getNodeAs("topCall");
+ MinMaxUseInitializerListCheck::FindArgsResult Result =
+ findArgs(Match, TopCall);
+
+ if (!Result.First || !Result.Last || Result.Args.size() <= 2) {
+return;
+ }
+
+ std::string ReplacementText = generateReplacement(Match, TopCall, Result);
+
+ diag(TopCall->getBeginLoc(),
+ "do not use nested std::%0 calls, use %1 instead")
+ << TopCall->getDirectCallee()->getName() << ReplacementText
+ << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(TopCall->getBeginLoc(),
+TopCall->getEndLoc()),
+ ReplacementText)
+ << Inserter.createMainFileIncludeInsertion("");
+}
+
+MinMaxUseInitializerListCheck::FindArgsResult
+MinMaxUseInitializerListCheck::findArgs(const MatchFinder::MatchResult ,
+const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ if (Call->getNumArgs() > 2) {
+auto ArgIterator = Call->arguments().begin();
+std::advance(ArgIterator, 2);
+Result.Compare = *ArgIterator;
+ }
+
+ for (const Expr *Arg : Call->arguments()) {
+if (!Result.First)
+ Result.First = Arg;
+
+const CallExpr *InnerCall = dyn_cast(Arg);
+if (InnerCall && InnerCall->getDirectCallee() &&
+InnerCall->getDirectCallee()->getNameAsString() ==
+Call->getDirectCallee()->getNameAsString()) {
+ FindArgsResult InnerResult = findArgs(Match, InnerCall);
+
+ bool processInnerResult = false;
+
+ if (!Result.Compare && !InnerResult.Compare)
+processInnerResult = true;
+ else if (Result.Compare && InnerResult.Compare &&
+ Lexer::getSourceText(CharSourceRange::getTokenRange(
+Result.Compare->getSourceRange()),
+*Match.SourceManager,
+Match.Context->getLangOpts()) ==
+ Lexer::getSourceText(
+ CharSourceRange::getTokenRange(
+ InnerResult.Compare->getSourceRange()),
+ *Match.SourceManager, Match.Context->getLangOpts()))
+processInnerResult = true;
+
+ if (processInnerResult) {
+Result.Args.insert(Result.Args.end(), InnerResult.Args.begin(),
+ InnerResult.Args.end());
+continue;
+ }
+}
+
+if (Arg == Result.Compare)
+ continue;
+
[clang-tools-extra] [clang-tidy] add check to suggest replacement of nested std::min or std::max with initializer lists (PR #85572)
@@ -0,0 +1,199 @@
+//===--- MinMaxUseInitializerListCheck.cpp - clang-tidy
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "MinMaxUseInitializerListCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+MinMaxUseInitializerListCheck::MinMaxUseInitializerListCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ Inserter(Options.getLocalOrGlobal("IncludeStyle",
+utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+
+void MinMaxUseInitializerListCheck::storeOptions(
+ClangTidyOptions::OptionMap ) {
+ Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+}
+
+void MinMaxUseInitializerListCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ callExpr(
+ callee(functionDecl(hasName("::std::max"))),
+ anyOf(hasArgument(
+0, callExpr(callee(functionDecl(hasName("::std::max"),
+hasArgument(
+1, callExpr(callee(functionDecl(hasName("::std::max")),
+ unless(
+
hasParent(callExpr(callee(functionDecl(hasName("::std::max")))
+ .bind("topCall"),
+ this);
+
+ Finder->addMatcher(
+ callExpr(
+ callee(functionDecl(hasName("::std::min"))),
+ anyOf(hasArgument(
+0, callExpr(callee(functionDecl(hasName("::std::min"),
+hasArgument(
+1, callExpr(callee(functionDecl(hasName("::std::min")),
+ unless(
+
hasParent(callExpr(callee(functionDecl(hasName("::std::min")))
+ .bind("topCall"),
+ this);
+}
+
+void MinMaxUseInitializerListCheck::registerPPCallbacks(
+const SourceManager , Preprocessor *PP, Preprocessor *ModuleExpanderPP)
{
+ Inserter.registerPreprocessor(PP);
+}
+
+void MinMaxUseInitializerListCheck::check(
+const MatchFinder::MatchResult ) {
+ const CallExpr *TopCall = Match.Nodes.getNodeAs("topCall");
+ MinMaxUseInitializerListCheck::FindArgsResult Result =
+ findArgs(Match, TopCall);
+
+ if (!Result.First || !Result.Last || Result.Args.size() <= 2) {
+return;
+ }
+
+ std::string ReplacementText = generateReplacement(Match, TopCall, Result);
+
+ diag(TopCall->getBeginLoc(),
+ "do not use nested std::%0 calls, use %1 instead")
+ << TopCall->getDirectCallee()->getName() << ReplacementText
+ << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(TopCall->getBeginLoc(),
+TopCall->getEndLoc()),
+ ReplacementText)
+ << Inserter.createMainFileIncludeInsertion("");
+}
+
+MinMaxUseInitializerListCheck::FindArgsResult
+MinMaxUseInitializerListCheck::findArgs(const MatchFinder::MatchResult ,
+const CallExpr *Call) {
+ FindArgsResult Result;
+ Result.First = nullptr;
+ Result.Last = nullptr;
+ Result.Compare = nullptr;
+
+ if (Call->getNumArgs() > 2) {
+auto ArgIterator = Call->arguments().begin();
+std::advance(ArgIterator, 2);
+Result.Compare = *ArgIterator;
+ }
+
+ for (const Expr *Arg : Call->arguments()) {
+if (!Result.First)
+ Result.First = Arg;
+
+const CallExpr *InnerCall = dyn_cast(Arg);
+if (InnerCall && InnerCall->getDirectCallee() &&
+InnerCall->getDirectCallee()->getNameAsString() ==
+Call->getDirectCallee()->getNameAsString()) {
+ FindArgsResult InnerResult = findArgs(Match, InnerCall);
+
+ bool processInnerResult = false;
+
+ if (!Result.Compare && !InnerResult.Compare)
+processInnerResult = true;
+ else if (Result.Compare && InnerResult.Compare &&
+ Lexer::getSourceText(CharSourceRange::getTokenRange(
+Result.Compare->getSourceRange()),
+*Match.SourceManager,
+Match.Context->getLangOpts()) ==
+ Lexer::getSourceText(
+ CharSourceRange::getTokenRange(
+ InnerResult.Compare->getSourceRange()),
+ *Match.SourceManager, Match.Context->getLangOpts()))
+processInnerResult = true;
+
+ if (processInnerResult) {
+Result.Args.insert(Result.Args.end(), InnerResult.Args.begin(),
+ InnerResult.Args.end());
+continue;
+ }
+}
+
+if (Arg == Result.Compare)
+ continue;
+
