diff --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp --- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp @@ -80,6 +80,29 @@ return Node.hasExplicitTemplateArgs(); } +// Helper Matcher which applies the given QualType Matcher either directly or by +// resolving a pointer type to its pointee. Used to match v.push_back() as well +// as p->push_back(). +auto hasTypeOrPointeeType( + const ast_matchers::internal::Matcher &TypeMatcher) { + return anyOf(hasType(TypeMatcher), + hasType(pointerType(pointee(TypeMatcher)))); +} + +// Matches if the node has canonical type matching any of the given names. +auto hasWantedType(llvm::ArrayRef TypeNames) { + return hasCanonicalType(hasDeclaration(cxxRecordDecl(hasAnyName(TypeNames)))); +} + +// Matches member call expressions of the named method on the listed container +// types. +auto cxxMemberCallExprOnContainer( + StringRef MethodName, llvm::ArrayRef ContainerNames) { + return cxxMemberCallExpr( + hasDeclaration(functionDecl(hasName(MethodName))), + on(hasTypeOrPointeeType(hasWantedType(ContainerNames)))); +} + const auto DefaultContainersWithPushBack = "::std::vector; ::std::list; ::std::deque"; const auto DefaultContainersWithPush = @@ -130,27 +153,19 @@ // because this requires special treatment (it could cause performance // regression) // + match for emplace calls that should be replaced with insertion - auto CallPushBack = cxxMemberCallExpr( - hasDeclaration(functionDecl(hasName("push_back"))), - on(hasType(hasCanonicalType( - hasDeclaration(cxxRecordDecl(hasAnyName(ContainersWithPushBack))))))); - - auto CallPush = - cxxMemberCallExpr(hasDeclaration(functionDecl(hasName("push"))), - on(hasType(hasCanonicalType(hasDeclaration( - cxxRecordDecl(hasAnyName(ContainersWithPush))))))); - - auto CallPushFront = cxxMemberCallExpr( - hasDeclaration(functionDecl(hasName("push_front"))), - on(hasType(hasCanonicalType(hasDeclaration( - cxxRecordDecl(hasAnyName(ContainersWithPushFront))))))); + auto CallPushBack = + cxxMemberCallExprOnContainer("push_back", ContainersWithPushBack); + auto CallPush = cxxMemberCallExprOnContainer("push", ContainersWithPush); + auto CallPushFront = + cxxMemberCallExprOnContainer("push_front", ContainersWithPushFront); auto CallEmplacy = cxxMemberCallExpr( hasDeclaration( functionDecl(hasAnyNameIgnoringTemplates(EmplacyFunctions))), - on(hasType(hasCanonicalType(hasDeclaration(has(typedefNameDecl( - hasName("value_type"), hasType(type(hasUnqualifiedDesugaredType( - recordType().bind("value_type"))))))))))); + on(hasTypeOrPointeeType(hasCanonicalType(hasDeclaration( + has(typedefNameDecl(hasName("value_type"), + hasType(type(hasUnqualifiedDesugaredType( + recordType().bind("value_type"))))))))))); // We can't replace push_backs of smart pointer because // if emplacement fails (f.e. bad_alloc in vector) we will have leak of diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp @@ -1334,3 +1334,90 @@ v3.push_back({{0}}); v3.push_back({{}}); } + +void testWithPointerTypes() { + std::list l; + std::list* lp = &l; + std::stack s; + std::stack* sp; + + lp->push_back(Something(1, 2)); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use emplace_back instead of push_back [modernize-use-emplace] + // CHECK-FIXES: lp->emplace_back(1, 2); + lp->push_front(Something(1, 2)); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use emplace_front instead of push_front [modernize-use-emplace] + // CHECK-FIXES: lp->emplace_front(1, 2); + sp->push(Something(1, 2)); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use emplace instead of push [modernize-use-emplace] + // CHECK-FIXES: sp->emplace(1, 2); + + lp->push_back(Something{1, 2}); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use emplace_back instead of push_back [modernize-use-emplace] + // CHECK-FIXES: lp->emplace_back(1, 2); + lp->push_front(Something{1, 2}); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use emplace_front instead of push_front [modernize-use-emplace] + // CHECK-FIXES: lp->emplace_front(1, 2); + sp->push(Something{1, 2}); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use emplace instead of push [modernize-use-emplace] + // CHECK-FIXES: sp->emplace(1, 2); + + lp->push_back(Something()); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use emplace_back instead of push_back [modernize-use-emplace] + // CHECK-FIXES: lp->emplace_back(); + lp->push_front(Something()); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use emplace_front instead of push_front [modernize-use-emplace] + // CHECK-FIXES: lp->emplace_front(); + sp->push(Something()); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use emplace instead of push [modernize-use-emplace] + // CHECK-FIXES: sp->emplace(); + + lp->push_back(Something{}); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use emplace_back instead of push_back [modernize-use-emplace] + // CHECK-FIXES: lp->emplace_back(); + lp->push_front(Something{}); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use emplace_front instead of push_front [modernize-use-emplace] + // CHECK-FIXES: lp->emplace_front(); + sp->push(Something{}); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use emplace instead of push [modernize-use-emplace] + // CHECK-FIXES: sp->emplace(); + + lp->emplace_back(Something(1, 2)); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: unnecessary temporary object created while calling emplace_back + // CHECK-FIXES: lp->emplace_back(1, 2); + lp->emplace_front(Something(1, 2)); + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: unnecessary temporary object created while calling emplace_front + // CHECK-FIXES: lp->emplace_front(1, 2); + sp->emplace(Something(1, 2)); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: unnecessary temporary object created while calling emplace + // CHECK-FIXES: sp->emplace(1, 2); + + lp->emplace_back(Something{1, 2}); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: unnecessary temporary object created while calling emplace_back + // CHECK-FIXES: lp->emplace_back(1, 2); + lp->emplace_front(Something{1, 2}); + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: unnecessary temporary object created while calling emplace_front + // CHECK-FIXES: lp->emplace_front(1, 2); + sp->emplace(Something{1, 2}); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: unnecessary temporary object created while calling emplace + // CHECK-FIXES: sp->emplace(1, 2); + + lp->emplace_back(Something()); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: unnecessary temporary object created while calling emplace_back + // CHECK-FIXES: lp->emplace_back(); + lp->emplace_front(Something()); + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: unnecessary temporary object created while calling emplace_front + // CHECK-FIXES: lp->emplace_front(); + sp->emplace(Something()); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: unnecessary temporary object created while calling emplace + // CHECK-FIXES: sp->emplace(); + + lp->emplace_back(Something{}); + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: unnecessary temporary object created while calling emplace_back + // CHECK-FIXES: lp->emplace_back(); + lp->emplace_front(Something{}); + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: unnecessary temporary object created while calling emplace_front + // CHECK-FIXES: lp->emplace_front(); + sp->emplace(Something{}); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: unnecessary temporary object created while calling emplace + // CHECK-FIXES: sp->emplace(); +}