Skip to content

Add ignoring paren imp casts in has any argument #89509

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add ignoring paren imp casts in has any argument #89509

wants to merge 2 commits into from

Conversation

komalverma04
Copy link
Contributor

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:

- 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

@llvmbot
Copy link
Member

llvmbot commented Apr 20, 2024 •
edited
Loading

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

@llvm/pr-subscribers-clang-tidy

Author: None (komalverma04)

Changes

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:

- 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 --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");

@komalverma04
Copy link
Contributor Author

@PiotrZSL Please guide me tackling the failing tests

@EugeneZelenko EugeneZelenko requested a review from PiotrZSL April 21, 2024 14:18
5chmidti
5chmidti requested changes Apr 21, 2024
View reviewed changes
Copy link
Contributor

@5chmidti 5chmidti left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (possibly in a different pr).

See

llvm-project/clang/include/clang/ASTMatchers/ASTMatchers.h

Lines 4891 to 4907 in d674f45

AST_POLYMORPHIC_MATCHER_P(hasAnyArgument,
AST_POLYMORPHIC_SUPPORTED_TYPES(
CallExpr, CXXConstructExpr,
CXXUnresolvedConstructExpr, ObjCMessageExpr),
internal::Matcher<Expr>, InnerMatcher) {
for (const Expr *Arg : Node.arguments()) {
if (Finder->isTraversalIgnoringImplicitNodes() &&
isa<CXXDefaultArgExpr>(Arg))
break;
BoundNodesTreeBuilder Result(*Builder);
if (InnerMatcher.matches(*Arg, Finder, &Result)) {
*Builder = std::move(Result);
return true;
}
}
return false;
}

vs

llvm-project/clang/include/clang/ASTMatchers/ASTMatchers.h

Lines 4553 to 4564 in d674f45

AST_POLYMORPHIC_MATCHER_P2(hasArgument,
AST_POLYMORPHIC_SUPPORTED_TYPES(
CallExpr, CXXConstructExpr,
CXXUnresolvedConstructExpr, ObjCMessageExpr),
unsigned, N, internal::Matcher<Expr>, InnerMatcher) {
if (N >= Node.getNumArgs())
return false;
const Expr *Arg = Node.getArg(N);
if (Finder->isTraversalIgnoringImplicitNodes() && isa<CXXDefaultArgExpr>(Arg))
return false;
return InnerMatcher.matches(*Arg->IgnoreParenImpCasts(), Finder, Builder);
}
. 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: #75754 (comment)

This is also the reason why the tests are failing, you are actually changing the behavior of these checks.

clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
Comment on lines +77 to +79
hasAnyArgument(
/* `hasArgument` would skip over parens */ ignoringParenImpCasts(
anyOf(NullLiteral, NullInitList, EmptyInitList))),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
Comment on lines +94 to +96
hasAnyArgument(
/* `hasArgument` would skip over parens */ ignoringParenImpCasts(
anyOf(NullLiteral, NullInitList, EmptyInitList))),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@5chmidti 5chmidti 5chmidti requested changes

@PiotrZSL PiotrZSL Awaiting requested review from PiotrZSL

Assignees
No one assigned
Labels
clang-tidy clang-tools-extra
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

hasAnyArgument() spuriously ignores match
3 participants
@komalverma04 @llvmbot @5chmidti