[clang-tidy] Fix modernize-use-emplace to support alias cases

[corona10]

Summary

Fix modernize-use-emplace to support alias cases

[corona10]

I am new to this project. I know that I need to write a test also likewise: https://github.com/llvm/llvm-project/blob/b8655f7ff286b9ebcd97cdd24b9c8eb5b89b9651/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp#L872-L884
Please let me know how to generate CHECK-MESSAGES and CHECK-FIXES comments to this.

[corona10]

Add unittest

[njames93]

Can you mention this change in the Release Notes (clang-tools-extra/docs/ReleaseNotes.rst).

clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp

1059

Can you please add a test like this, where emplace is called with a temporary object when the container type is an alias.

[corona10]

• Update modernize-use-emplace to support emplacy cases for alias types.
• Add new test
• Update release note.

[corona10]

clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp

1059

Done!

[corona10]

Rebase

[njames93]

LGTM, just with a small nit.

[corona10]

In D132640#3753744, @njames93 wrote:

LGTM, just with a small nit.

@njames93

Thank you but I can not find your comment about nit.
Would you like to check the comment one more time?

[njames93]

clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp

138

Sorry about that, not sure what happened.
Anyway same goes for below, use a cxxRecordDecl matcher instead of namedDecl.

[corona10]

Address code review.

[corona10]

Thanks! I updated the change.

[corona10]

• clang-format

[corona10]

@njames93 Can we land this patch before it occurs conflicting with other changes?

[njames93]

In D132640#3757277, @corona10 wrote:

@njames93 Can we land this patch before it occurs conflicting with other changes?

Yes, sorry do you have commit access. If you don't what is your GitHub name and email that I should use when committing.

[corona10]

In D132640#3757395, @njames93 wrote:

In D132640#3757277, @corona10 wrote:

@njames93 Can we land this patch before it occurs conflicting with other changes?

Yes, sorry do you have commit access. If you don't what is your GitHub name and email that I should use when committing.

Thanks, I don't have a commit bit :)

Github name: corona10
email: [e-mail]

Thank you!

clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp

: ClangTidyCheck(Name, Context), IgnoreImplicitConstructors(Options.get(
           Options.get("EmplacyFunctions", DefaultEmplacyFunctions))) {}
 
 void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
   // FIXME: Bunch of functionality that could be easily added:
   // + add handling of `insert` for stl associative container, but be careful
   // 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(cxxRecordDecl(hasAnyName(ContainersWithPushBack)))));
-
-  auto CallPush = cxxMemberCallExpr(
-      hasDeclaration(functionDecl(hasName("push"))),
-      on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPush)))));
-
-  auto CallPushFront = cxxMemberCallExpr(
-      hasDeclaration(functionDecl(hasName("push_front"))),
-      on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPushFront)))));
+  auto CallPushBack =
+      cxxMemberCallExpr(hasDeclaration(functionDecl(hasName("push_back"))),
+                        on(hasType(hasCanonicalType(hasDeclaration(
+                            namedDecl(hasAnyName(ContainersWithPushBack)))))));
+
+  auto CallPush =
+      cxxMemberCallExpr(hasDeclaration(functionDecl(hasName("push"))),
+                        on(hasType(hasCanonicalType(hasDeclaration(
+                            namedDecl(hasAnyName(ContainersWithPush)))))));
+
+  auto CallPushFront =
+      cxxMemberCallExpr(hasDeclaration(functionDecl(hasName("push_front"))),
+                        on(hasType(hasCanonicalType(hasDeclaration(
+                            namedDecl(hasAnyName(ContainersWithPushFront)))))));
 
   auto CallEmplacy = cxxMemberCallExpr(
       hasDeclaration(
           functionDecl(hasAnyNameIgnoringTemplates(EmplacyFunctions))),
-      on(hasType(cxxRecordDecl(has(typedefNameDecl(
+      on(hasType(hasCanonicalType(hasDeclaration(has(typedefNameDecl(
           hasName("value_type"), hasType(type(hasUnqualifiedDesugaredType(
-                                     recordType().bind("value_type"))))))))));
+                                     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
   // passed pointer because smart pointer won't be constructed
   // (and destructed) as in push_back case.
   auto IsCtorOfSmartPtr =
       hasDeclaration(cxxConstructorDecl(ofClass(hasAnyName(SmartPointers))));
 

clang-tools-extra/docs/ReleaseNotes.rst

 
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 - Improved :doc:`bugprone-signal-handler
   <clang-tidy/checks/bugprone/signal-handler>` check. Partial
   support for C++14 signal handler rules was added. Bug report generation was
   improved.
-  
 - Improved `modernize-use-emplace <clang-tidy/checks/modernize/use-emplace.html>`_ check.
 
   The check now supports detecting inefficient invocations of ``push`` and
   ``push_front`` on STL-style containers and replacing them with ``emplace``
   or ``emplace_front``.
 
+  The check now supports detecting alias cases of ``push_back`` ``push`` and
+  ``push_front`` on STL-style containers and replacing them with ``emplace_back``,
+  ``emplace`` or ``emplace_front``.
+
 - Improved `modernize-use-equals-default <clang-tidy/checks/modernize/use-equals-default.html>`_ check.
 
   The check now skips unions since in this case a default constructor with empty body
   is not equivalent to the explicitly defaulted one.
 
 Removed checks
 ^^^^^^^^^^^^^^
 

clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp

void testAllSTLEmplacyFunctions() {
   stack.emplace(Foo(13));
   // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: unnecessary temporary object created while calling emplace
   // CHECK-FIXES: stack.emplace(13);
 
   queue.emplace(Foo(13));
   // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: unnecessary temporary object created while calling emplace
   // CHECK-FIXES: queue.emplace(13);
 
   priority_queue.emplace(Foo(13));
   // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: unnecessary temporary object created while calling emplace
   // CHECK-FIXES: priority_queue.emplace(13);
 }
 
+void test_AliasEmplacyFunctions() {
+  typedef std::list<Foo> L;
+  using DQ = std::deque<Foo>;
+  L l;
+  l.emplace_back(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: unnecessary temporary object created while calling emplace_back
+  // CHECK-FIXES: l.emplace_back(3);
+
+  DQ dq;
+  dq.emplace_back(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back
+  // CHECK-FIXES: dq.emplace_back(3);
+
+  typedef std::stack<Foo> STACK;
+  using PQ = std::priority_queue<Foo>;
+  STACK stack;
+  stack.emplace(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: stack.emplace(3);
+
+  PQ pq;
+  pq.emplace(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: pq.emplace(3);
+
+  typedef std::forward_list<Foo> FL;
+  using DQ2 = std::deque<Foo>;
+  FL fl;
+  fl.emplace_front(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: unnecessary temporary object created while calling emplace_front
+  // CHECK-FIXES: fl.emplace_front(3);
+
+  DQ2 dq2;
+  dq2.emplace_front(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: unnecessary temporary object created while calling emplace_front
+  // CHECK-FIXES: dq2.emplace_front(3);
+}
+
+void test_Alias() {
+  typedef std::list<Foo> L;
+  using DQ = std::deque<Foo>;
+  L l;
+  l.push_back(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back instead of push_back [modernize-use-emplace]
+  // CHECK-FIXES: l.emplace_back(3);
+
+  DQ dq;
+  dq.push_back(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back instead of push_back [modernize-use-emplace]
+  // CHECK-FIXES: dq.emplace_back(3);
+
+  typedef std::stack<Foo> STACK;
+  using PQ = std::priority_queue<Foo>;
+  STACK stack;
+  stack.push(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use emplace instead of push [modernize-use-emplace]
+  // CHECK-FIXES: stack.emplace(3);
+
+  PQ pq;
+  pq.push(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace instead of push [modernize-use-emplace]
+  // CHECK-FIXES: pq.emplace(3);
+
+  typedef std::forward_list<Foo> FL;
+  using DQ2 = std::deque<Foo>;
+  FL fl;
+  fl.push_front(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_front instead of push_front [modernize-use-emplace]
+  // CHECK-FIXES: fl.emplace_front(3);
+
+  DQ2 dq2;
+  dq2.push_front(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use emplace_front instead of push_front [modernize-use-emplace]
+  // CHECK-FIXES: dq2.emplace_front(3);
+}
+
 struct Bar {
 public:
   Bar(){};
   void testWithPrivateAndProtectedCtor() {
     std::vector<Bar> vec;
 
     vec.emplace_back(Bar(13));
     vec.emplace_back(Bar(13, 13));