[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Jakub Kuderski via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL301651: [clang-tidy] modernize-use-emplace: remove 
unnecessary make_pair calls (authored by kuhar).

Changed prior to commit:
  https://reviews.llvm.org/D32395?vs=96634=97113#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D32395

Files:
  clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.cpp
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-emplace.rst
  clang-tools-extra/trunk/test/clang-tidy/modernize-use-emplace.cpp

Index: clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -15,10 +15,16 @@
 namespace tidy {
 namespace modernize {
 
-static const auto DefaultContainersWithPushBack =
+namespace {
+AST_MATCHER(DeclRefExpr, hasExplicitTemplateArgs) {
+  return Node.hasExplicitTemplateArgs();
+}
+
+const auto DefaultContainersWithPushBack =
 "::std::vector; ::std::list; ::std::deque";
-static const auto DefaultSmartPointers =
+const auto DefaultSmartPointers =
 "::std::shared_ptr; ::std::unique_ptr; ::std::auto_ptr; ::std::weak_ptr";
+} // namespace
 
 UseEmplaceCheck::UseEmplaceCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
@@ -39,7 +45,6 @@
   // because this requires special treatment (it could cause performance
   // regression)
   // + match for emplace calls that should be replaced with insertion
-  // + match for make_pair calls.
   auto callPushBack = cxxMemberCallExpr(
   hasDeclaration(functionDecl(hasName("push_back"))),
   on(hasType(cxxRecordDecl(hasAnyName(SmallVector(
@@ -80,43 +85,64 @@
   .bind("ctor");
   auto hasConstructExpr = has(ignoringImplicit(soughtConstructExpr));
 
-  auto ctorAsArgument = materializeTemporaryExpr(
-  anyOf(hasConstructExpr, has(cxxFunctionalCastExpr(hasConstructExpr;
+  auto makePair = ignoringImplicit(
+  callExpr(callee(expr(ignoringImplicit(
+  declRefExpr(unless(hasExplicitTemplateArgs()),
+  to(functionDecl(hasName("::std::make_pair"
+  .bind("make_pair"));
+
+  // make_pair can return type convertible to container's element type.
+  // Allow the conversion only on containers of pairs.
+  auto makePairCtor = ignoringImplicit(cxxConstructExpr(
+  has(materializeTemporaryExpr(makePair)),
+  hasDeclaration(cxxConstructorDecl(ofClass(hasName("::std::pair"));
+
+  auto soughtParam = materializeTemporaryExpr(
+  anyOf(has(makePair), has(makePairCtor),
+hasConstructExpr, has(cxxFunctionalCastExpr(hasConstructExpr;
 
-  Finder->addMatcher(cxxMemberCallExpr(callPushBack, has(ctorAsArgument),
+  Finder->addMatcher(cxxMemberCallExpr(callPushBack, has(soughtParam),
unless(isInTemplateInstantiation()))
  .bind("call"),
  this);
 }
 
 void UseEmplaceCheck::check(const MatchFinder::MatchResult ) {
   const auto *Call = Result.Nodes.getNodeAs("call");
   const auto *InnerCtorCall = Result.Nodes.getNodeAs("ctor");
+  const auto *MakePairCall = Result.Nodes.getNodeAs("make_pair");
+  assert((InnerCtorCall || MakePairCall) && "No push_back parameter matched");
 
-  auto FunctionNameSourceRange = CharSourceRange::getCharRange(
+  const auto FunctionNameSourceRange = CharSourceRange::getCharRange(
   Call->getExprLoc(), Call->getArg(0)->getExprLoc());
 
   auto Diag = diag(Call->getExprLoc(), "use emplace_back instead of push_back");
 
   if (FunctionNameSourceRange.getBegin().isMacroID())
 return;
 
-  Diag << FixItHint::CreateReplacement(FunctionNameSourceRange,
-   "emplace_back(");
+  const auto *EmplacePrefix = MakePairCall ? "emplace_back" : "emplace_back(";
+  Diag << FixItHint::CreateReplacement(FunctionNameSourceRange, EmplacePrefix);
 
-  auto CallParensRange = InnerCtorCall->getParenOrBraceRange();
+  const SourceRange CallParensRange =
+  MakePairCall ? SourceRange(MakePairCall->getCallee()->getLocEnd(),
+ MakePairCall->getRParenLoc())
+   : InnerCtorCall->getParenOrBraceRange();
 
   // Finish if there is no explicit constructor call.
   if (CallParensRange.getBegin().isInvalid())
 return;
 
+  const SourceLocation ExprBegin =
+  MakePairCall ? MakePairCall->getExprLoc() : InnerCtorCall->getExprLoc();
+
   // Range for constructor name and opening brace.
-  auto CtorCallSourceRange = CharSourceRange::getTokenRange(
-  InnerCtorCall->getExprLoc(), CallParensRange.getBegin());
+  const auto ParamCallSourceRange =
+  CharSourceRange::getTokenRange(ExprBegin, CallParensRange.getBegin());
 
-  Diag << 

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh accepted this revision.
alexfh added a comment.

LG


https://reviews.llvm.org/D32395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek accepted this revision.
Prazek added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D32395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar added a comment.

I run clang-tidy with this version of modernize-use-emplace on the whole llvm + 
clang + clang tools extra tree and it emitted ~500 fixits, ~100 of them were 
make_pair ones.
The whole codebase compiled fine and there were no new test failures with 
check-all.


https://reviews.llvm.org/D32395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek added inline comments.



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115
+  const auto *MakePairCall = Result.Nodes.getNodeAs("make_pair");
+  assert(InnerCtorCall || MakePairCall);
 

kuhar wrote:
> Prazek wrote:
> > JDevlieghere wrote:
> > > It's highly recommended to put some kind of error message in the 
> > > assertion statement, as per the LLVM coding standards.
> > would it be better to change it to
> > !innerCtorCall && !MakePairCall && "No .."
> I don't think that this logic would work here. `!first && !second` ensures 
> both are null.
Yea, I forgot how to De Morgan


https://reviews.llvm.org/D32395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar marked 2 inline comments as done.
kuhar added inline comments.



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115
+  const auto *MakePairCall = Result.Nodes.getNodeAs("make_pair");
+  assert(InnerCtorCall || MakePairCall);
 

Prazek wrote:
> JDevlieghere wrote:
> > It's highly recommended to put some kind of error message in the assertion 
> > statement, as per the LLVM coding standards.
> would it be better to change it to
> !innerCtorCall && !MakePairCall && "No .."
I don't think that this logic would work here. `!first && !second` ensures both 
are null.


https://reviews.llvm.org/D32395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar updated this revision to Diff 96634.
kuhar added a comment.

Use igoringImplicit instead of ignoringParenImpCasts. Options moved to the 
anonymous namespace.


https://reviews.llvm.org/D32395

Files:
  clang-tidy/modernize/UseEmplaceCheck.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/modernize-use-emplace.rst
  test/clang-tidy/modernize-use-emplace.cpp

Index: test/clang-tidy/modernize-use-emplace.cpp
===
--- test/clang-tidy/modernize-use-emplace.cpp
+++ test/clang-tidy/modernize-use-emplace.cpp
@@ -53,8 +53,8 @@
 };
 
 template 
-pair make_pair(T1, T2) {
-  return pair();
+pair make_pair(T1&&, T2&&) {
+  return {};
 };
 
 template 
@@ -274,18 +274,51 @@
 
 void testMakePair() {
   std::vector> v;
-  // FIXME: add functionality to change calls of std::make_pair
   v.push_back(std::make_pair(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(1, 2);
 
-  // FIXME: This is not a bug, but call of make_pair should be removed in the
-  // future. This one matches because the return type of make_pair is different
-  // than the pair itself.
   v.push_back(std::make_pair(42LL, 13));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
-  // CHECK-FIXES: v.emplace_back(std::make_pair(42LL, 13));
+  // CHECK-FIXES: v.emplace_back(42LL, 13);
+
+  v.push_back(std::make_pair(0, 3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(std::make_pair(0, 3));
+  //
+  // Even though the call above could be turned into v.emplace_back(0, 3),
+  // we don't eliminate the make_pair call here, because of the explicit
+  // template parameters provided. make_pair's arguments can be convertible
+  // to its explicitly provided template parameter, but not to the pair's
+  // element type. The examples below illustrate the problem.
+  struct D {
+D(...) {}
+operator char() const { return 0; }
+  };
+  v.push_back(std::make_pair(Something(), 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(std::make_pair(Something(), 2));
+
+  struct X {
+X(std::pair) {}
+  };
+  std::vector x;
+  x.push_back(std::make_pair(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: x.emplace_back(std::make_pair(1, 2));
+  // make_pair cannot be removed here, as X is not constructible with two ints.
+
+  struct Y {
+Y(std::pair&&) {}
+  };
+  std::vector y;
+  y.push_back(std::make_pair(2, 3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: y.emplace_back(std::make_pair(2, 3));
+  // make_pair cannot be removed here, as Y is not constructible with two ints.
 }
 
-void testOtherCointainers() {
+void testOtherContainers() {
   std::list l;
   l.push_back(Something(42, 41));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
Index: docs/clang-tidy/checks/modernize-use-emplace.rst
===
--- docs/clang-tidy/checks/modernize-use-emplace.rst
+++ docs/clang-tidy/checks/modernize-use-emplace.rst
@@ -36,8 +36,7 @@
 
 std::vector> w;
 w.emplace_back(21, 37);
-// This will be fixed to w.emplace_back(21L, 37L); in next version
-w.emplace_back(std::make_pair(21L, 37L);
+w.emplace_back(21L, 37L);
 
 The other situation is when we pass arguments that will be converted to a type
 inside a container.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -101,6 +101,12 @@
   Finds possible inefficient vector operations in for loops that may cause
   unnecessary memory reallocations.
 
+- Improved `modernize-use-emplace
+  `_ check
+
+  Removes unnecessary std::make_pair calls in push_back(std::make_pair(a, b)) calls and turns them
+  into emplace_back(a, b).
+
 Improvements to include-fixer
 -
 
Index: clang-tidy/modernize/UseEmplaceCheck.cpp
===
--- clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -15,10 +15,16 @@
 namespace tidy {
 namespace modernize {
 
-static const auto DefaultContainersWithPushBack =
+namespace {
+AST_MATCHER(DeclRefExpr, hasExplicitTemplateArgs) {
+  return Node.hasExplicitTemplateArgs();
+}
+
+const auto DefaultContainersWithPushBack =
 "::std::vector; ::std::list; ::std::deque";
-static const auto DefaultSmartPointers =
+const auto DefaultSmartPointers =
 "::std::shared_ptr; ::std::unique_ptr; ::std::auto_ptr; ::std::weak_ptr";
+} // namespace
 
 UseEmplaceCheck::UseEmplaceCheck(StringRef Name, 

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek added inline comments.



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115
+  const auto *MakePairCall = Result.Nodes.getNodeAs("make_pair");
+  assert(InnerCtorCall || MakePairCall);
 

JDevlieghere wrote:
> It's highly recommended to put some kind of error message in the assertion 
> statement, as per the LLVM coding standards.
would it be better to change it to
!innerCtorCall && !MakePairCall && "No .."



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:22
+}
+} // namespace
+

extend the anonymous namespace to have static strings (and remove static)



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:90
+  auto makePair = ignoringImplicit(
+  callExpr(callee(expr(ignoringParenImpCasts(
+  declRefExpr(unless(hasExplicitTemplateArgs()),

Why ignoringParenImpCasts instead of ingnoringImplicit?


https://reviews.llvm.org/D32395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar updated this revision to Diff 96622.
kuhar marked 4 inline comments as done.
kuhar added a comment.

Added const where possible, moved from if-else to ternary.


https://reviews.llvm.org/D32395

Files:
  clang-tidy/modernize/UseEmplaceCheck.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/modernize-use-emplace.rst
  test/clang-tidy/modernize-use-emplace.cpp

Index: test/clang-tidy/modernize-use-emplace.cpp
===
--- test/clang-tidy/modernize-use-emplace.cpp
+++ test/clang-tidy/modernize-use-emplace.cpp
@@ -53,8 +53,8 @@
 };
 
 template 
-pair make_pair(T1, T2) {
-  return pair();
+pair make_pair(T1&&, T2&&) {
+  return {};
 };
 
 template 
@@ -274,18 +274,51 @@
 
 void testMakePair() {
   std::vector> v;
-  // FIXME: add functionality to change calls of std::make_pair
   v.push_back(std::make_pair(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(1, 2);
 
-  // FIXME: This is not a bug, but call of make_pair should be removed in the
-  // future. This one matches because the return type of make_pair is different
-  // than the pair itself.
   v.push_back(std::make_pair(42LL, 13));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
-  // CHECK-FIXES: v.emplace_back(std::make_pair(42LL, 13));
+  // CHECK-FIXES: v.emplace_back(42LL, 13);
+
+  v.push_back(std::make_pair(0, 3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(std::make_pair(0, 3));
+  //
+  // Even though the call above could be turned into v.emplace_back(0, 3),
+  // we don't eliminate the make_pair call here, because of the explicit
+  // template parameters provided. make_pair's arguments can be convertible
+  // to its explicitly provided template parameter, but not to the pair's
+  // element type. The examples below illustrate the problem.
+  struct D {
+D(...) {}
+operator char() const { return 0; }
+  };
+  v.push_back(std::make_pair(Something(), 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(std::make_pair(Something(), 2));
+
+  struct X {
+X(std::pair) {}
+  };
+  std::vector x;
+  x.push_back(std::make_pair(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: x.emplace_back(std::make_pair(1, 2));
+  // make_pair cannot be removed here, as X is not constructible with two ints.
+
+  struct Y {
+Y(std::pair&&) {}
+  };
+  std::vector y;
+  y.push_back(std::make_pair(2, 3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: y.emplace_back(std::make_pair(2, 3));
+  // make_pair cannot be removed here, as Y is not constructible with two ints.
 }
 
-void testOtherCointainers() {
+void testOtherContainers() {
   std::list l;
   l.push_back(Something(42, 41));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
Index: docs/clang-tidy/checks/modernize-use-emplace.rst
===
--- docs/clang-tidy/checks/modernize-use-emplace.rst
+++ docs/clang-tidy/checks/modernize-use-emplace.rst
@@ -36,8 +36,7 @@
 
 std::vector> w;
 w.emplace_back(21, 37);
-// This will be fixed to w.emplace_back(21L, 37L); in next version
-w.emplace_back(std::make_pair(21L, 37L);
+w.emplace_back(21L, 37L);
 
 The other situation is when we pass arguments that will be converted to a type
 inside a container.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -101,6 +101,12 @@
   Finds possible inefficient vector operations in for loops that may cause
   unnecessary memory reallocations.
 
+- Improved `modernize-use-emplace
+  `_ check
+
+  Removes unnecessary std::make_pair calls in push_back(std::make_pair(a, b)) calls and turns them
+  into emplace_back(a, b).
+
 Improvements to include-fixer
 -
 
Index: clang-tidy/modernize/UseEmplaceCheck.cpp
===
--- clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -15,6 +15,12 @@
 namespace tidy {
 namespace modernize {
 
+namespace {
+AST_MATCHER(DeclRefExpr, hasExplicitTemplateArgs) {
+  return Node.hasExplicitTemplateArgs();
+}
+} // namespace
+
 static const auto DefaultContainersWithPushBack =
 "::std::vector; ::std::list; ::std::deque";
 static const auto DefaultSmartPointers =
@@ -80,43 +86,64 @@
   .bind("ctor");
   auto hasConstructExpr = has(ignoringImplicit(soughtConstructExpr));
 
-  auto ctorAsArgument = materializeTemporaryExpr(
-  anyOf(hasConstructExpr, 

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar added a comment.

Thanks for the suggestions, Jonas. Fixed.


https://reviews.llvm.org/D32395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added inline comments.



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:115
+  const auto *MakePairCall = Result.Nodes.getNodeAs("make_pair");
+  assert(InnerCtorCall || MakePairCall);
 

It's highly recommended to put some kind of error message in the assertion 
statement, as per the LLVM coding standards.



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:130
+  SourceRange CallParensRange;
+  if (MakePairCall)
+CallParensRange = SourceRange(MakePairCall->getCallee()->getLocEnd(),

Can't we use the ternary operator here?



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:140
 
+  SourceLocation ExprBegin;
+  if (MakePairCall)

Same here?



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:147
   // Range for constructor name and opening brace.
-  auto CtorCallSourceRange = CharSourceRange::getTokenRange(
-  InnerCtorCall->getExprLoc(), CallParensRange.getBegin());
+  auto ParamCallSourceRange = CharSourceRange::getTokenRange(
+  ExprBegin, CallParensRange.getBegin());

I think this can be const.


https://reviews.llvm.org/D32395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar updated this revision to Diff 96384.
kuhar added a comment.

Don't remove make_pair calls when inserted type is different than std::pair.


https://reviews.llvm.org/D32395

Files:
  clang-tidy/modernize/UseEmplaceCheck.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/modernize-use-emplace.rst
  test/clang-tidy/modernize-use-emplace.cpp

Index: test/clang-tidy/modernize-use-emplace.cpp
===
--- test/clang-tidy/modernize-use-emplace.cpp
+++ test/clang-tidy/modernize-use-emplace.cpp
@@ -53,8 +53,8 @@
 };
 
 template 
-pair make_pair(T1, T2) {
-  return pair();
+pair make_pair(T1&&, T2&&) {
+  return {};
 };
 
 template 
@@ -274,18 +274,51 @@
 
 void testMakePair() {
   std::vector> v;
-  // FIXME: add functionality to change calls of std::make_pair
   v.push_back(std::make_pair(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(1, 2);
 
-  // FIXME: This is not a bug, but call of make_pair should be removed in the
-  // future. This one matches because the return type of make_pair is different
-  // than the pair itself.
   v.push_back(std::make_pair(42LL, 13));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
-  // CHECK-FIXES: v.emplace_back(std::make_pair(42LL, 13));
+  // CHECK-FIXES: v.emplace_back(42LL, 13);
+
+  v.push_back(std::make_pair(0, 3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(std::make_pair(0, 3));
+  //
+  // Even though the call above could be turned into v.emplace_back(0, 3),
+  // we don't eliminate the make_pair call here, because of the explicit
+  // template parameters provided. make_pair's arguments can be convertible
+  // to its explicitly provided template parameter, but not to the pair's
+  // element type. The examples below illustrate the problem.
+  struct D {
+D(...) {}
+operator char() const { return 0; }
+  };
+  v.push_back(std::make_pair(Something(), 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(std::make_pair(Something(), 2));
+
+  struct X {
+X(std::pair) {}
+  };
+  std::vector x;
+  x.push_back(std::make_pair(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: x.emplace_back(std::make_pair(1, 2));
+  // make_pair cannot be removed here, as X is not constructible with two ints.
+
+  struct Y {
+Y(std::pair&&) {}
+  };
+  std::vector y;
+  y.push_back(std::make_pair(2, 3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: y.emplace_back(std::make_pair(2, 3));
+  // make_pair cannot be removed here, as Y is not constructible with two ints.
 }
 
-void testOtherCointainers() {
+void testOtherContainers() {
   std::list l;
   l.push_back(Something(42, 41));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
Index: docs/clang-tidy/checks/modernize-use-emplace.rst
===
--- docs/clang-tidy/checks/modernize-use-emplace.rst
+++ docs/clang-tidy/checks/modernize-use-emplace.rst
@@ -36,8 +36,7 @@
 
 std::vector> w;
 w.emplace_back(21, 37);
-// This will be fixed to w.emplace_back(21L, 37L); in next version
-w.emplace_back(std::make_pair(21L, 37L);
+w.emplace_back(21L, 37L);
 
 The other situation is when we pass arguments that will be converted to a type
 inside a container.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -101,6 +101,12 @@
   Finds possible inefficient vector operations in for loops that may cause
   unnecessary memory reallocations.
 
+- Improved `modernize-use-emplace
+  `_ check
+
+  Removes unnecessary std::make_pair calls in push_back(std::make_pair(a, b)) calls and turns them
+  into emplace_back(a, b).
+
 Improvements to include-fixer
 -
 
Index: clang-tidy/modernize/UseEmplaceCheck.cpp
===
--- clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -15,6 +15,12 @@
 namespace tidy {
 namespace modernize {
 
+namespace {
+AST_MATCHER(DeclRefExpr, hasExplicitTemplateArgs) {
+  return Node.hasExplicitTemplateArgs();
+}
+} // namespace
+
 static const auto DefaultContainersWithPushBack =
 "::std::vector; ::std::list; ::std::deque";
 static const auto DefaultSmartPointers =
@@ -80,18 +86,33 @@
   .bind("ctor");
   auto hasConstructExpr = has(ignoringImplicit(soughtConstructExpr));
 
-  auto ctorAsArgument = materializeTemporaryExpr(
-  anyOf(hasConstructExpr, 

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar added inline comments.



Comment at: test/clang-tidy/modernize-use-emplace.cpp:284
+  // CHECK-FIXES: v.emplace_back(42LL, 13);
+
+  v.push_back(std::make_pair(0, 3));

kuhar wrote:
> Prazek wrote:
> > I would add here test like:
> > 
> >   class X {
> > X(std:;pair a) {}
> >   };
> >   
> >   std::vector v;
> >   v.push_back(make_pair(42, 42));
> >   
> > I guess as long as X ctor is not explicit this can happen, and we can't 
> > transform it to
> > emplace.back(42, 42)
> Nice idea for a test case, added.
A better test case would be to make X's ctor take a pair by const reference or 
rvalue reference. This way it wouldn't produce an extra CxxConstructExpr and 
will be currently (incorrectly) matched. This needs to be fixed.


https://reviews.llvm.org/D32395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar updated this revision to Diff 96332.
kuhar marked an inline comment as done.
kuhar added a comment.

Update modernize-use-emplace rst docs.


https://reviews.llvm.org/D32395

Files:
  clang-tidy/modernize/UseEmplaceCheck.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/modernize-use-emplace.rst
  test/clang-tidy/modernize-use-emplace.cpp

Index: test/clang-tidy/modernize-use-emplace.cpp
===
--- test/clang-tidy/modernize-use-emplace.cpp
+++ test/clang-tidy/modernize-use-emplace.cpp
@@ -53,8 +53,8 @@
 };
 
 template 
-pair make_pair(T1, T2) {
-  return pair();
+pair make_pair(T1&&, T2&&) {
+  return {};
 };
 
 template 
@@ -274,18 +274,42 @@
 
 void testMakePair() {
   std::vector> v;
-  // FIXME: add functionality to change calls of std::make_pair
   v.push_back(std::make_pair(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(1, 2);
 
-  // FIXME: This is not a bug, but call of make_pair should be removed in the
-  // future. This one matches because the return type of make_pair is different
-  // than the pair itself.
   v.push_back(std::make_pair(42LL, 13));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
-  // CHECK-FIXES: v.emplace_back(std::make_pair(42LL, 13));
+  // CHECK-FIXES: v.emplace_back(42LL, 13);
+
+  v.push_back(std::make_pair(0, 3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(std::make_pair(0, 3));
+  //
+  // Even though the call above could be turned into v.emplace_back(0, 3),
+  // we don't eliminate the make_pair call here, because of the explicit
+  // template parameters provided. make_pair's arguments can be convertible
+  // to its explicitly provided template parameter, but not to the pair's
+  // element type. The example below illustrates the problem.
+  struct D {
+D(...) {}
+operator char() const { return 0; }
+  };
+  v.push_back(std::make_pair(Something(), 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(std::make_pair(Something(), 2));
+
+  struct X {
+X(std::pair) {}
+  };
+  std::vector x;
+  x.push_back(std::make_pair(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: x.emplace_back(std::make_pair(1, 2));
+  // make_pair cannot be removed here, as X is not constructible with two ints.
 }
 
-void testOtherCointainers() {
+void testOtherContainers() {
   std::list l;
   l.push_back(Something(42, 41));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
Index: docs/clang-tidy/checks/modernize-use-emplace.rst
===
--- docs/clang-tidy/checks/modernize-use-emplace.rst
+++ docs/clang-tidy/checks/modernize-use-emplace.rst
@@ -36,8 +36,7 @@
 
 std::vector> w;
 w.emplace_back(21, 37);
-// This will be fixed to w.emplace_back(21L, 37L); in next version
-w.emplace_back(std::make_pair(21L, 37L);
+w.emplace_back(21L, 37L);
 
 The other situation is when we pass arguments that will be converted to a type
 inside a container.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -101,6 +101,12 @@
   Finds possible inefficient vector operations in for loops that may cause
   unnecessary memory reallocations.
 
+- Improved `modernize-use-emplace
+  `_ check
+
+  Removes unnecessary std::make_pair calls in push_back(std::make_pair(a, b)) calls and turns them
+  into emplace_back(a, b).
+
 Improvements to include-fixer
 -
 
Index: clang-tidy/modernize/UseEmplaceCheck.cpp
===
--- clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -15,6 +15,12 @@
 namespace tidy {
 namespace modernize {
 
+namespace {
+AST_MATCHER(DeclRefExpr, hasExplicitTemplateArgs) {
+  return Node.hasExplicitTemplateArgs();
+}
+} // namespace
+
 static const auto DefaultContainersWithPushBack =
 "::std::vector; ::std::list; ::std::deque";
 static const auto DefaultSmartPointers =
@@ -80,18 +86,31 @@
   .bind("ctor");
   auto hasConstructExpr = has(ignoringImplicit(soughtConstructExpr));
 
-  auto ctorAsArgument = materializeTemporaryExpr(
-  anyOf(hasConstructExpr, has(cxxFunctionalCastExpr(hasConstructExpr;
+  auto makePair = ignoringImplicit(
+  callExpr(callee(expr(ignoringParenImpCasts(
+  declRefExpr(unless(hasExplicitTemplateArgs()),
+  to(functionDecl(hasName("::std::make_pair"
+  .bind("make_pair"));
+
+  // make_pair can return type convertible to 

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar marked 2 inline comments as done.
kuhar added inline comments.



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:93
+  to(functionDecl(hasName("::std::make_pair"
+  
+  .bind("make_pair"));

Prazek wrote:
> JonasToth wrote:
> > is the new line here necessary? i think it looks better if the `.bind` is 
> > on this line.
> Better question is "is it clang formated?"
It was clang-formatted, but I do agree that it looked a bit weird. I removed 
the extra newline.



Comment at: test/clang-tidy/modernize-use-emplace.cpp:284
+  // CHECK-FIXES: v.emplace_back(42LL, 13);
+
+  v.push_back(std::make_pair(0, 3));

Prazek wrote:
> I would add here test like:
> 
>   class X {
> X(std:;pair a) {}
>   };
>   
>   std::vector v;
>   v.push_back(make_pair(42, 42));
>   
> I guess as long as X ctor is not explicit this can happen, and we can't 
> transform it to
> emplace.back(42, 42)
Nice idea for a test case, added.


https://reviews.llvm.org/D32395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar updated this revision to Diff 96329.
kuhar added a comment.

Add test for pair constructor argument, mention changes in release notes.


https://reviews.llvm.org/D32395

Files:
  clang-tidy/modernize/UseEmplaceCheck.cpp
  docs/ReleaseNotes.rst
  test/clang-tidy/modernize-use-emplace.cpp

Index: test/clang-tidy/modernize-use-emplace.cpp
===
--- test/clang-tidy/modernize-use-emplace.cpp
+++ test/clang-tidy/modernize-use-emplace.cpp
@@ -53,8 +53,8 @@
 };
 
 template 
-pair make_pair(T1, T2) {
-  return pair();
+pair make_pair(T1&&, T2&&) {
+  return {};
 };
 
 template 
@@ -274,18 +274,42 @@
 
 void testMakePair() {
   std::vector> v;
-  // FIXME: add functionality to change calls of std::make_pair
   v.push_back(std::make_pair(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(1, 2);
 
-  // FIXME: This is not a bug, but call of make_pair should be removed in the
-  // future. This one matches because the return type of make_pair is different
-  // than the pair itself.
   v.push_back(std::make_pair(42LL, 13));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
-  // CHECK-FIXES: v.emplace_back(std::make_pair(42LL, 13));
+  // CHECK-FIXES: v.emplace_back(42LL, 13);
+
+  v.push_back(std::make_pair(0, 3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(std::make_pair(0, 3));
+  //
+  // Even though the call above could be turned into v.emplace_back(0, 3),
+  // we don't eliminate the make_pair call here, because of the explicit
+  // template parameters provided. make_pair's arguments can be convertible
+  // to its explicitly provided template parameter, but not to the pair's
+  // element type. The example below illustrates the problem.
+  struct D {
+D(...) {}
+operator char() const { return 0; }
+  };
+  v.push_back(std::make_pair(Something(), 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(std::make_pair(Something(), 2));
+
+  struct X {
+X(std::pair) {}
+  };
+  std::vector x;
+  x.push_back(std::make_pair(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: x.emplace_back(std::make_pair(1, 2));
+  // make_pair cannot be removed here, as X is not constructible with two ints.
 }
 
-void testOtherCointainers() {
+void testOtherContainers() {
   std::list l;
   l.push_back(Something(42, 41));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -101,6 +101,12 @@
   Finds possible inefficient vector operations in for loops that may cause
   unnecessary memory reallocations.
 
+- Improved `modernize-use-emplace
+  `_ check
+
+  Removes unnecessary std::make_pair calls in push_back(std::make_pair(a, b)) calls and turns them
+  into emplace_back(a, b).
+
 Improvements to include-fixer
 -
 
Index: clang-tidy/modernize/UseEmplaceCheck.cpp
===
--- clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -15,6 +15,12 @@
 namespace tidy {
 namespace modernize {
 
+namespace {
+AST_MATCHER(DeclRefExpr, hasExplicitTemplateArgs) {
+  return Node.hasExplicitTemplateArgs();
+}
+} // namespace
+
 static const auto DefaultContainersWithPushBack =
 "::std::vector; ::std::list; ::std::deque";
 static const auto DefaultSmartPointers =
@@ -80,18 +86,31 @@
   .bind("ctor");
   auto hasConstructExpr = has(ignoringImplicit(soughtConstructExpr));
 
-  auto ctorAsArgument = materializeTemporaryExpr(
-  anyOf(hasConstructExpr, has(cxxFunctionalCastExpr(hasConstructExpr;
+  auto makePair = ignoringImplicit(
+  callExpr(callee(expr(ignoringParenImpCasts(
+  declRefExpr(unless(hasExplicitTemplateArgs()),
+  to(functionDecl(hasName("::std::make_pair"
+  .bind("make_pair"));
+
+  // make_pair can return type convertible to container's element type.
+  auto makePairCtor = ignoringImplicit(cxxConstructExpr(
+  has(materializeTemporaryExpr(makePair;
 
-  Finder->addMatcher(cxxMemberCallExpr(callPushBack, has(ctorAsArgument),
+  auto soughtParam = materializeTemporaryExpr(
+  anyOf(has(makePair), has(makePairCtor),
+hasConstructExpr, has(cxxFunctionalCastExpr(hasConstructExpr;
+
+  Finder->addMatcher(cxxMemberCallExpr(callPushBack, has(soughtParam),
unless(isInTemplateInstantiation()))
  .bind("call"),
  this);
 }
 
 void 

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek added inline comments.



Comment at: test/clang-tidy/modernize-use-emplace.cpp:284
+  // CHECK-FIXES: v.emplace_back(42LL, 13);
+
+  v.push_back(std::make_pair(0, 3));

I would add here test like:

  class X {
X(std:;pair a) {}
  };
  
  std::vector v;
  v.push_back(make_pair(42, 42));
  
I guess as long as X ctor is not explicit this can happen, and we can't 
transform it to
emplace.back(42, 42)


https://reviews.llvm.org/D32395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek added inline comments.



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:93
+  to(functionDecl(hasName("::std::make_pair"
+  
+  .bind("make_pair"));

JonasToth wrote:
> is the new line here necessary? i think it looks better if the `.bind` is on 
> this line.
Better question is "is it clang formated?"



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:97
+  // make_pair can return type convertible to container's element type.
+  auto makePairCtor = ignoringImplicit(cxxConstructExpr(
+  has(materializeTemporaryExpr(makePair;

JonasToth wrote:
> here, on line 100 and 89: shouldnt the matchers be upper case since they are 
> variables? Iam unsure about that.
True, all the matchers should have upper case, but it would be better to send 
it in separate patch


https://reviews.llvm.org/D32395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

please note this enhancement in the ReleaseNotes.




Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:93
+  to(functionDecl(hasName("::std::make_pair"
+  
+  .bind("make_pair"));

is the new line here necessary? i think it looks better if the `.bind` is on 
this line.



Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:97
+  // make_pair can return type convertible to container's element type.
+  auto makePairCtor = ignoringImplicit(cxxConstructExpr(
+  has(materializeTemporaryExpr(makePair;

here, on line 100 and 89: shouldnt the matchers be upper case since they are 
variables? Iam unsure about that.


https://reviews.llvm.org/D32395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32395: [clang-tidy] modernize-use-emplace: remove unnecessary make_pair calls

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar created this revision.
kuhar added a project: clang-tools-extra.

When there is a push_back with a call to make_pair, turn it into emplace_back 
and remove the unnecessary make_pair call.

Eg.

  std::vector> v;
  v.push_back(std::make_pair(1, 2)); // --> v.emplace_back(1, 2);

make_pair doesn't get removed when explicit template parameters are provided, 
because of potential problems with type conversions.


https://reviews.llvm.org/D32395

Files:
  clang-tidy/modernize/UseEmplaceCheck.cpp
  test/clang-tidy/modernize-use-emplace.cpp

Index: test/clang-tidy/modernize-use-emplace.cpp
===
--- test/clang-tidy/modernize-use-emplace.cpp
+++ test/clang-tidy/modernize-use-emplace.cpp
@@ -53,8 +53,8 @@
 };
 
 template 
-pair make_pair(T1, T2) {
-  return pair();
+pair make_pair(T1&&, T2&&) {
+  return {};
 };
 
 template 
@@ -274,18 +274,33 @@
 
 void testMakePair() {
   std::vector> v;
-  // FIXME: add functionality to change calls of std::make_pair
   v.push_back(std::make_pair(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(1, 2);
 
-  // FIXME: This is not a bug, but call of make_pair should be removed in the
-  // future. This one matches because the return type of make_pair is different
-  // than the pair itself.
   v.push_back(std::make_pair(42LL, 13));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
-  // CHECK-FIXES: v.emplace_back(std::make_pair(42LL, 13));
+  // CHECK-FIXES: v.emplace_back(42LL, 13);
+
+  v.push_back(std::make_pair(0, 3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(std::make_pair(0, 3));
+  //
+  // Even though the call above could be turned into v.emplace_back(0, 3),
+  // we don't eliminate the make_pair call here, because of the explicit
+  // template parameters provided. make_pair's arguments can be convertible
+  // to its explicitly provided template parameter, but not to the pair's
+  // element type. The example below illustrates the problem.
+  struct D {
+D(...) {}
+operator char() const { return 0; }
+  };
+  v.push_back(std::make_pair(Something(), 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(std::make_pair(Something(), 2));
 }
 
-void testOtherCointainers() {
+void testOtherContainers() {
   std::list l;
   l.push_back(Something(42, 41));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
Index: clang-tidy/modernize/UseEmplaceCheck.cpp
===
--- clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -15,6 +15,12 @@
 namespace tidy {
 namespace modernize {
 
+namespace {
+AST_MATCHER(DeclRefExpr, hasExplicitTemplateArgs) {
+  return Node.hasExplicitTemplateArgs();
+}
+} // namespace
+
 static const auto DefaultContainersWithPushBack =
 "::std::vector; ::std::list; ::std::deque";
 static const auto DefaultSmartPointers =
@@ -80,18 +86,32 @@
   .bind("ctor");
   auto hasConstructExpr = has(ignoringImplicit(soughtConstructExpr));
 
-  auto ctorAsArgument = materializeTemporaryExpr(
-  anyOf(hasConstructExpr, has(cxxFunctionalCastExpr(hasConstructExpr;
+  auto makePair = ignoringImplicit(
+  callExpr(callee(expr(ignoringParenImpCasts(
+  declRefExpr(unless(hasExplicitTemplateArgs()),
+  to(functionDecl(hasName("::std::make_pair"
+  
+  .bind("make_pair"));
+
+  // make_pair can return type convertible to container's element type.
+  auto makePairCtor = ignoringImplicit(cxxConstructExpr(
+  has(materializeTemporaryExpr(makePair;
 
-  Finder->addMatcher(cxxMemberCallExpr(callPushBack, has(ctorAsArgument),
+  auto soughtParam = materializeTemporaryExpr(
+  anyOf(has(makePair), has(makePairCtor),
+hasConstructExpr, has(cxxFunctionalCastExpr(hasConstructExpr;
+
+  Finder->addMatcher(cxxMemberCallExpr(callPushBack, has(soughtParam),
unless(isInTemplateInstantiation()))
  .bind("call"),
  this);
 }
 
 void UseEmplaceCheck::check(const MatchFinder::MatchResult ) {
   const auto *Call = Result.Nodes.getNodeAs("call");
   const auto *InnerCtorCall = Result.Nodes.getNodeAs("ctor");
+  const auto *MakePairCall = Result.Nodes.getNodeAs("make_pair");
+  assert(InnerCtorCall || MakePairCall);
 
   auto FunctionNameSourceRange = CharSourceRange::getCharRange(
   Call->getExprLoc(), Call->getArg(0)->getExprLoc());
@@ -101,22 +121,34 @@
   if (FunctionNameSourceRange.getBegin().isMacroID())
 return;
 
+  const auto *EmplacePrefix = MakePairCall ? "emplace_back" : "emplace_back(";
   Diag <<