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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f76df7d
add ignoringParenImpCasts in hasAnyArgument
komalverma04 Apr 20, 2024
186e351
Merge branch 'main' of https://github.com/llvm/llvm-project into Add-…
komalverma04 Apr 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions clang-tools-extra/clang-tidy/abseil/StrCatAppendCheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ void MisplacedOperatorInStrlenInAllocCheck::registerMatchers(

const auto BadUse =
callExpr(callee(StrLenFunc),
hasAnyArgument(ignoringImpCasts(
hasAnyArgument(ignoringParenImpCasts(
binaryOperator(
hasOperatorName("+"),
hasRHS(ignoringParenImpCasts(integerLiteral(equals(1)))))
Expand Down Expand Up @@ -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");
Expand Down
3 changes: 2 additions & 1 deletion clang-tools-extra/clang-tidy/bugprone/MisplacedWideningCastCheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
14 changes: 6 additions & 8 deletions clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
9 changes: 5 additions & 4 deletions clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
9 changes: 6 additions & 3 deletions clang-tools-extra/clang-tidy/bugprone/StringLiteralWithEmbeddedNulCheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
12 changes: 7 additions & 5 deletions clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))),
Comment on lines +77 to +79
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.

unless(cxxTemporaryObjectExpr(/* filters out type spellings */)),
has(expr().bind("null_arg_expr")))
.bind("construct_expr");
Expand All @@ -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))),
Comment on lines +94 to +96
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

has(expr().bind("null_arg_expr")))),
remove(node("null_arg_expr")), construction_warning);

Expand Down Expand Up @@ -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("\"\"")),
Expand Down
3 changes: 2 additions & 1 deletion clang-tools-extra/clang-tidy/bugprone/SuspiciousStringviewDataUsageCheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
14 changes: 7 additions & 7 deletions clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))));
Expand Down Expand Up @@ -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)));
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down