[PATCH] D32313: [clang-tidy] Don't turn .push_back({1, 2}) into .emplace_back(1, 2) for pairs

[Jakub Kuderski via Phabricator via cfe-commits]

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

This patch prevents modernize-use-emplace from changing push_backs of brace initialized pairs (`.push_back({1, 2})`) to `.emplace_back(1, 2)`.
Pair's constructor doesn't have any interesting logic and basically performs aggregate initialization. There also doesn't seem to be much win
in terms of making code more concise, thus is makes little sense to turn such push_back calls into emplace_backs.

The change is inspired by the recent discussion on changing and enforcing coding guidelines:
[llvm-dev] [cfe-dev] Modernizing LLVM Coding Style Guide and	enforcing Clang-tidy
http://lists.llvm.org/pipermail/llvm-dev/2016-December/108559.html


https://reviews.llvm.org/D32313

Files:
  clang-tidy/modernize/UseEmplaceCheck.cpp
  clang-tidy/modernize/UseEmplaceCheck.h
  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
@@ -186,6 +186,11 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
   // CHECK-FIXES: v.emplace_back(1, 2);
 
+  v.push_back({1, 2});
+  v.push_back(std::pair<int, int>{1, 2});
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(1, 2);
+
   GetPair g;
   v.push_back(g.getPair());
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
Index: clang-tidy/modernize/UseEmplaceCheck.h
===================================================================
--- clang-tidy/modernize/UseEmplaceCheck.h
+++ clang-tidy/modernize/UseEmplaceCheck.h
@@ -35,6 +35,7 @@
 private:
   std::vector<std::string> ContainersWithPushBack;
   std::vector<std::string> SmartPointers;
+  std::vector<std::string> PairTypes;
 };
 
 } // namespace modernize
Index: clang-tidy/modernize/UseEmplaceCheck.cpp
===================================================================
--- clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -19,13 +19,16 @@
     "::std::vector; ::std::list; ::std::deque";
 static const auto DefaultSmartPointers =
     "::std::shared_ptr; ::std::unique_ptr; ::std::auto_ptr; ::std::weak_ptr";
+static const auto DefaultPairTypes = "::std::pair";
 
 UseEmplaceCheck::UseEmplaceCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       ContainersWithPushBack(utils::options::parseStringList(Options.get(
           "ContainersWithPushBack", DefaultContainersWithPushBack))),
       SmartPointers(utils::options::parseStringList(
-          Options.get("SmartPointers", DefaultSmartPointers))) {}
+          Options.get("SmartPointers", DefaultSmartPointers))),
+      PairTypes(utils::options::parseStringList(
+          Options.get("PairTypes", DefaultPairTypes))) {}
 
 void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus11)
@@ -72,11 +75,19 @@
   auto hasInitList = has(ignoringImplicit(initListExpr()));
   // FIXME: Discard 0/NULL (as nullptr), static inline const data members,
   // overloaded functions and template names.
+
+  // Ignore push_back({first, second}) for pair types (eg. std::pair).
+  auto isPairBraceInit = expr(allOf(cxxConstructExpr(hasDeclaration(
+      cxxConstructorDecl(ofClass(hasAnyName(SmallVector<StringRef, 2>(
+          PairTypes.begin(), PairTypes.end())))))),
+                                    unless(cxxTemporaryObjectExpr())),
+                              unless(hasParent(implicitCastExpr())));
+
   auto soughtConstructExpr =
       cxxConstructExpr(
           unless(anyOf(isCtorOfSmartPtr, hasInitList, bitFieldAsArgument,
                        initializerListAsArgument, newExprAsArgument,
-                       constructingDerived, isPrivateCtor)))
+                       constructingDerived, isPrivateCtor, isPairBraceInit)))
           .bind("ctor");
   auto hasConstructExpr = has(ignoringImplicit(soughtConstructExpr));
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D32313.96020.patch
Type: text/x-patch
Size: 3177 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170420/d6b1f725/attachment.bin>