[clang-tools-extra] Add ignoring paren imp casts in has any argument (PR #89509)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Apr 20 10:32:08 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy
Author: None (komalverma04)
<details>
<summary>Changes</summary>
# Maintaining Consistency in `hasAnyArgument()` and `hasArgument()` Matchers in Clang AST Matchers
The `hasArgument()` matcher already ignores implicit casts and parentheses, but the `hasAnyArgument()` matcher does not. To ensure consistency, we need to explicitly use `ignoringParenImpCasts()` to handle cases where there might be implicit casts or parentheses around the argument in the Clang AST match.
The code changes made are as follows:
```diff
- hasAnyArgument(hasType(asString("S *")))
+ hasAnyArgument(ignoringParenImpCasts(hasType(asString("S *"))))
```
This change ensures that any implicit casts and parentheses around the argument type S * are ignored.
Fixes #<!-- -->75754
---
Full diff: https://github.com/llvm/llvm-project/pull/89509.diff
11 Files Affected:
- (modified) clang-tools-extra/clang-tidy/abseil/StrCatAppendCheck.cpp (+8-7)
- (modified) clang-tools-extra/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp (+2-2)
- (modified) clang-tools-extra/clang-tidy/bugprone/MisplacedWideningCastCheck.cpp (+2-1)
- (modified) clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp (+6-8)
- (modified) clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp (+5-4)
- (modified) clang-tools-extra/clang-tidy/bugprone/StringLiteralWithEmbeddedNulCheck.cpp (+6-3)
- (modified) clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp (+7-5)
- (modified) clang-tools-extra/clang-tidy/bugprone/SuspiciousStringviewDataUsageCheck.cpp (+2-1)
- (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp (+2-2)
- (modified) clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp (+7-7)
- (modified) clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp (+2-2)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/abseil/StrCatAppendCheck.cpp b/clang-tools-extra/clang-tidy/abseil/StrCatAppendCheck.cpp
index ab6ed701e59fee..f6b64a2dc15ede 100644
--- a/clang-tools-extra/clang-tidy/abseil/StrCatAppendCheck.cpp
+++ b/clang-tools-extra/clang-tidy/abseil/StrCatAppendCheck.cpp
@@ -34,23 +34,23 @@ AST_MATCHER_P(Stmt, IgnoringTemporaries, ast_matchers::internal::Matcher<Stmt>,
return InnerMatcher.matches(*E, Finder, Builder);
}
-} // namespace
+} // namespace
// TODO: str += StrCat(...)
// str.append(StrCat(...))
void StrCatAppendCheck::registerMatchers(MatchFinder *Finder) {
const auto StrCat = functionDecl(hasName("::absl::StrCat"));
- // The arguments of absl::StrCat are implicitly converted to AlphaNum. This
- // matches to the arguments because of that behavior.
+ // The arguments of absl::StrCat are implicitly converted to AlphaNum. This
+ // matches to the arguments because of that behavior.
const auto AlphaNum = IgnoringTemporaries(cxxConstructExpr(
argumentCountIs(1), hasType(cxxRecordDecl(hasName("::absl::AlphaNum"))),
hasArgument(0, ignoringImpCasts(declRefExpr(to(equalsBoundNode("LHS")),
expr().bind("Arg0"))))));
- const auto HasAnotherReferenceToLhs =
- callExpr(hasAnyArgument(expr(hasDescendant(declRefExpr(
- to(equalsBoundNode("LHS")), unless(equalsBoundNode("Arg0")))))));
+ const auto HasAnotherReferenceToLhs = callExpr(
+ hasAnyArgument(ignoringParenImpCasts(expr(hasDescendant(declRefExpr(
+ to(equalsBoundNode("LHS")), unless(equalsBoundNode("Arg0"))))))));
// Now look for calls to operator= with an object on the LHS and a call to
// StrCat on the RHS. The first argument of the StrCat call should be the same
@@ -73,7 +73,8 @@ void StrCatAppendCheck::registerMatchers(MatchFinder *Finder) {
void StrCatAppendCheck::check(const MatchFinder::MatchResult &Result) {
const auto *Op = Result.Nodes.getNodeAs<CXXOperatorCallExpr>("Op");
const auto *Call = Result.Nodes.getNodeAs<CallExpr>("Call");
- assert(Op != nullptr && Call != nullptr && "Matcher does not work as expected");
+ assert(Op != nullptr && Call != nullptr &&
+ "Matcher does not work as expected");
// Handles the case 'x = absl::StrCat(x)', which has no effect.
if (Call->getNumArgs() == 1) {
diff --git a/clang-tools-extra/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
index 40e4ab6c8b12af..cf7f06e06b26a0 100644
--- a/clang-tools-extra/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
@@ -24,7 +24,7 @@ void MisplacedOperatorInStrlenInAllocCheck::registerMatchers(
const auto BadUse =
callExpr(callee(StrLenFunc),
- hasAnyArgument(ignoringImpCasts(
+ hasAnyArgument(ignoringParenImpCasts(
binaryOperator(
hasOperatorName("+"),
hasRHS(ignoringParenImpCasts(integerLiteral(equals(1)))))
@@ -74,7 +74,7 @@ void MisplacedOperatorInStrlenInAllocCheck::check(
if (!Alloc)
Alloc = Result.Nodes.getNodeAs<CXXNewExpr>("Alloc");
assert(Alloc && "Matched node bound by 'Alloc' should be either 'CallExpr'"
- " or 'CXXNewExpr'");
+ " or 'CXXNewExpr'");
const auto *StrLen = Result.Nodes.getNodeAs<CallExpr>("StrLen");
const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("BinOp");
diff --git a/clang-tools-extra/clang-tidy/bugprone/MisplacedWideningCastCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MisplacedWideningCastCheck.cpp
index a1f92aae55448c..f989e927871ace 100644
--- a/clang-tools-extra/clang-tidy/bugprone/MisplacedWideningCastCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/MisplacedWideningCastCheck.cpp
@@ -42,7 +42,8 @@ void MisplacedWideningCastCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(varDecl(hasInitializer(Cast)), this);
Finder->addMatcher(returnStmt(hasReturnValue(Cast)), this);
- Finder->addMatcher(callExpr(hasAnyArgument(Cast)), this);
+ Finder->addMatcher(callExpr(hasAnyArgument(ignoringParenImpCasts(Cast))),
+ this);
Finder->addMatcher(binaryOperator(hasOperatorName("="), hasRHS(Cast)), this);
Finder->addMatcher(
binaryOperator(isComparisonOperator(), hasEitherOperand(Cast)), this);
diff --git a/clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp
index 41191a3cfed23a..d304ac0bd681a7 100644
--- a/clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp
@@ -95,18 +95,16 @@ void MultipleNewInOneExpressionCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
callExpr(
- hasAnyArgument(
- expr(HasNewExpr1).bind("arg1")),
- hasAnyArgument(
- expr(HasNewExpr2, unless(equalsBoundNode("arg1"))).bind("arg2")),
+ hasAnyArgument(ignoringParenImpCasts(expr(HasNewExpr1).bind("arg1"))),
+ hasAnyArgument(ignoringParenImpCasts(
+ expr(HasNewExpr2, unless(equalsBoundNode("arg1"))).bind("arg2"))),
hasAncestor(BadAllocCatchingTryBlock)),
this);
Finder->addMatcher(
cxxConstructExpr(
- hasAnyArgument(
- expr(HasNewExpr1).bind("arg1")),
- hasAnyArgument(
- expr(HasNewExpr2, unless(equalsBoundNode("arg1"))).bind("arg2")),
+ hasAnyArgument(ignoringParenImpCasts(expr(HasNewExpr1).bind("arg1"))),
+ hasAnyArgument(ignoringParenImpCasts(
+ expr(HasNewExpr2, unless(equalsBoundNode("arg1"))).bind("arg2"))),
unless(isListInitialization()),
hasAncestor(BadAllocCatchingTryBlock)),
this);
diff --git a/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
index 977241e91b9a93..264926de70353e 100644
--- a/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
@@ -618,10 +618,11 @@ void NotNullTerminatedResultCheck::registerMatchers(MatchFinder *Finder) {
// Note: Sometimes the size of char is explicitly written out.
auto SizeExpr = anyOf(SizeOfCharExpr, integerLiteral(equals(1)));
- auto MallocLengthExpr = allOf(
- callee(functionDecl(
- hasAnyName("::alloca", "::calloc", "malloc", "realloc"))),
- hasAnyArgument(allOf(unless(SizeExpr), expr().bind(DestMallocExprName))));
+ auto MallocLengthExpr =
+ allOf(callee(functionDecl(
+ hasAnyName("::alloca", "::calloc", "malloc", "realloc"))),
+ hasAnyArgument(ignoringParenImpCasts(
+ allOf(unless(SizeExpr), expr().bind(DestMallocExprName)))));
// - Example: (char *)malloc(length);
auto DestMalloc = anyOf(callExpr(MallocLengthExpr),
diff --git a/clang-tools-extra/clang-tidy/bugprone/StringLiteralWithEmbeddedNulCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/StringLiteralWithEmbeddedNulCheck.cpp
index 72e680d25cb846..8e5a7a4462179d 100644
--- a/clang-tools-extra/clang-tidy/bugprone/StringLiteralWithEmbeddedNulCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/StringLiteralWithEmbeddedNulCheck.cpp
@@ -47,12 +47,15 @@ void StringLiteralWithEmbeddedNulCheck::registerMatchers(MatchFinder *Finder) {
// 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, StrLitWithNul))),
this);
// Detect passing a suspicious string literal through an overloaded operator.
- Finder->addMatcher(cxxOperatorCallExpr(hasAnyArgument(StrLitWithNul)), this);
+ Finder->addMatcher(
+ cxxOperatorCallExpr(hasAnyArgument(ignoringParenImpCasts(StrLitWithNul))),
+ this);
}
void StringLiteralWithEmbeddedNulCheck::check(
diff --git a/clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
index ea50250f829f02..fc7123a2caf881 100644
--- a/clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
@@ -74,8 +74,9 @@ RewriteRuleWith<std::string> 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))),
unless(cxxTemporaryObjectExpr(/* filters out type spellings */)),
has(expr().bind("null_arg_expr")))
.bind("construct_expr");
@@ -90,8 +91,9 @@ RewriteRuleWith<std::string> 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))),
has(expr().bind("null_arg_expr")))),
remove(node("null_arg_expr")), construction_warning);
@@ -263,7 +265,7 @@ RewriteRuleWith<std::string> StringviewNullptrCheckImpl() {
auto HandleConstructorInvocation =
makeRule(cxxConstructExpr(
hasAnyArgument(/* `hasArgument` would skip over parens */
- ignoringImpCasts(
+ ignoringParenImpCasts(
BasicStringViewConstructingFromNullExpr)),
unless(HasBasicStringViewType)),
changeTo(node("construct_expr"), cat("\"\"")),
diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousStringviewDataUsageCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SuspiciousStringviewDataUsageCheck.cpp
index 8f4b0c5e0dceda..f71bc6df200caf 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SuspiciousStringviewDataUsageCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousStringviewDataUsageCheck.cpp
@@ -73,7 +73,8 @@ void SuspiciousStringviewDataUsageCheck::registerMatchers(MatchFinder *Finder) {
hasAnyArgument(
ignoringParenImpCasts(equalsBoundNode("data-call"))),
unless(hasAnyArgument(ignoringParenImpCasts(SizeCall))),
- unless(hasAnyArgument(DescendantSizeCall)),
+ unless(hasAnyArgument(
+ ignoringParenImpCasts(DescendantSizeCall))),
hasDeclaration(namedDecl(
unless(matchers::matchesAnyListedName(AllowedCallees))))),
initListExpr(expr().bind("parent"),
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp
index 9b4d2ef99e5bf1..4b26949d2ca899 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp
@@ -86,9 +86,9 @@ void OwningMemoryCheck::registerMatchers(MatchFinder *Finder) {
// functions shall be 'gsl::owner<>'.
Finder->addMatcher(
traverse(TK_AsIs, callExpr(callee(LegacyOwnerConsumers),
- hasAnyArgument(expr(
+ hasAnyArgument(ignoringParenImpCasts(expr(
unless(ignoringImpCasts(ConsideredOwner)),
- hasType(pointerType()))))
+ hasType(pointerType())))))
.bind("legacy_consumer")),
this);
diff --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
index 430455a38f395e..0c061fc0eac6f7 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -98,8 +98,8 @@ auto hasWantedType(llvm::ArrayRef<StringRef> TypeNames) {
// Matches member call expressions of the named method on the listed container
// types.
-auto cxxMemberCallExprOnContainer(
- StringRef MethodName, llvm::ArrayRef<StringRef> ContainerNames) {
+auto cxxMemberCallExprOnContainer(StringRef MethodName,
+ llvm::ArrayRef<StringRef> ContainerNames) {
return cxxMemberCallExpr(
hasDeclaration(functionDecl(hasName(MethodName))),
on(hasTypeOrPointeeType(hasWantedType(ContainerNames))));
@@ -177,16 +177,16 @@ void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
hasDeclaration(cxxConstructorDecl(ofClass(hasAnyName(SmartPointers))));
// Bitfields binds only to consts and emplace_back take it by universal ref.
- auto BitFieldAsArgument = hasAnyArgument(
- ignoringImplicit(memberExpr(hasDeclaration(fieldDecl(isBitField())))));
+ auto BitFieldAsArgument = hasAnyArgument(ignoringParenImpCasts(
+ memberExpr(hasDeclaration(fieldDecl(isBitField())))));
// Initializer list can't be passed to universal reference.
auto InitializerListAsArgument = hasAnyArgument(
- ignoringImplicit(allOf(cxxConstructExpr(isListInitialization()),
- unless(cxxTemporaryObjectExpr()))));
+ ignoringParenImpCasts(allOf(cxxConstructExpr(isListInitialization()),
+ unless(cxxTemporaryObjectExpr()))));
// We could have leak of resource.
- auto NewExprAsArgument = hasAnyArgument(ignoringImplicit(cxxNewExpr()));
+ auto NewExprAsArgument = hasAnyArgument(ignoringParenImpCasts(cxxNewExpr()));
// We would call another constructor.
auto ConstructingDerived =
hasParent(implicitCastExpr(hasCastKind(CastKind::CK_DerivedToBase)));
diff --git a/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp b/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
index 9e4e3f63e19cfe..498fbecd2baa59 100644
--- a/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
@@ -32,12 +32,12 @@ void InefficientStringConcatenationCheck::registerMatchers(
const auto BasicStringPlusOperator = cxxOperatorCallExpr(
hasOverloadedOperatorName("+"),
- hasAnyArgument(ignoringImpCasts(declRefExpr(BasicStringType))));
+ hasAnyArgument(ignoringParenImpCasts(declRefExpr(BasicStringType))));
const auto PlusOperator =
cxxOperatorCallExpr(
hasOverloadedOperatorName("+"),
- hasAnyArgument(ignoringImpCasts(declRefExpr(BasicStringType))),
+ hasAnyArgument(ignoringParenImpCasts(declRefExpr(BasicStringType))),
hasDescendant(BasicStringPlusOperator))
.bind("plusOperator");
``````````
</details>
https://github.com/llvm/llvm-project/pull/89509
More information about the cfe-commits
mailing list