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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits