[llvm-branch-commits] [clang-tools-extra-branch] r303524 - [clang-tidy] Fix PR32896, PR33058: detect initializer lists in modernize-use-empalce

[Jakub Kuderski via llvm-branch-commits]

Author: kuhar
Date: Sun May 21 20:56:33 2017
New Revision: 303524

URL: http://llvm.org/viewvc/llvm-project?rev=303524&view=rev
Log:
[clang-tidy] Fix PR32896, PR33058: detect initializer lists in modernize-use-empalce

Summary:
The patch is backported from: r302281 to fix the 4.0.1 release blocker PR33058.
This patch fixes [[ https://bugs.llvm.org/show_bug.cgi?id=32896 | PR32896 ]].

The problem was that modernize-use-emplace incorrectly removed changed push_back into emplace_back, removing explicit constructor call with initializer list parameter, resulting in compiler error after applying fixits.
modernize-use-emplace used to check if matched constructor had InitListExpr, but didn't check against CXXStdInitializerListExpr.

Eg.

```
std::vector<std::vector<int>> v;
  v.push_back(std::vector<int>({1})); // --> v.emplace_back({1});
```

Reviewers: Prazek, alexfh, aaron.ballman

Reviewed By: Prazek, alexfh, aaron.ballman

Subscribers: xazax.hun, cfe-commits

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D32767


Modified:
    clang-tools-extra/branches/release_40/clang-tidy/modernize/UseEmplaceCheck.cpp
    clang-tools-extra/branches/release_40/test/clang-tidy/modernize-use-emplace.cpp

Modified: clang-tools-extra/branches/release_40/clang-tidy/modernize/UseEmplaceCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/branches/release_40/clang-tidy/modernize/UseEmplaceCheck.cpp?rev=303524&r1=303523&r2=303524&view=diff
==============================================================================
--- clang-tools-extra/branches/release_40/clang-tidy/modernize/UseEmplaceCheck.cpp (original)
+++ clang-tools-extra/branches/release_40/clang-tidy/modernize/UseEmplaceCheck.cpp Sun May 21 20:56:33 2017
@@ -20,6 +20,14 @@ static const auto DefaultContainersWithP
 static const auto DefaultSmartPointers =
     "::std::shared_ptr; ::std::unique_ptr; ::std::auto_ptr; ::std::weak_ptr";
 
+namespace {
+namespace impl {
+// FIXME: This matcher should be replaced by a matcher from ASTMatcher.h
+const ast_matchers::internal::VariadicDynCastAllOfMatcher<Stmt,
+    CXXStdInitializerListExpr> cxxStdInitializerListExpr;
+} // namespace impl
+} // namespace
+
 UseEmplaceCheck::UseEmplaceCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       ContainersWithPushBack(utils::options::parseStringList(Options.get(
@@ -69,7 +77,11 @@ void UseEmplaceCheck::registerMatchers(M
   // emplace_back can't access private constructor.
   auto isPrivateCtor = hasDeclaration(cxxConstructorDecl(isPrivate()));
 
-  auto hasInitList = has(ignoringImplicit(initListExpr()));
+  auto hasInitList = anyOf(has(ignoringImplicit(initListExpr())),
+                           has(impl::cxxStdInitializerListExpr()));
+  // FIXME: Replace internal C++ initializer list matcher with one from
+  // ASTMatchers.h
+
   // FIXME: Discard 0/NULL (as nullptr), static inline const data members,
   // overloaded functions and template names.
   auto soughtConstructExpr =

Modified: clang-tools-extra/branches/release_40/test/clang-tidy/modernize-use-emplace.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/branches/release_40/test/clang-tidy/modernize-use-emplace.cpp?rev=303524&r1=303523&r2=303524&view=diff
==============================================================================
--- clang-tools-extra/branches/release_40/test/clang-tidy/modernize-use-emplace.cpp (original)
+++ clang-tools-extra/branches/release_40/test/clang-tidy/modernize-use-emplace.cpp Sun May 21 20:56:33 2017
@@ -4,9 +4,19 @@
 // RUN:               value: '::std::vector; ::std::list; ::std::deque; llvm::LikeASmallVector'}]}" -- -std=c++11
 
 namespace std {
+template <typename>
+class initializer_list
+{
+public:
+  initializer_list() noexcept {}
+};
+
 template <typename T>
 class vector {
 public:
+  vector() = default;
+  vector(initializer_list<T>) {}
+
   void push_back(const T &) {}
   void push_back(T &&) {}
 
@@ -422,3 +432,16 @@ void testWithDtor() {
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
   // CHECK-FIXES: v.emplace_back(42);
 }
+
+void testInitializerList() {
+  std::vector<std::vector<int>> v;
+  v.push_back(std::vector<int>({1}));
+  // Test against the bug reported in PR32896.
+
+  v.push_back({{2}});
+
+  using PairIntVector = std::pair<int, std::vector<int>>;
+  std::vector<PairIntVector> x;
+  x.push_back(PairIntVector(3, {4}));
+  x.push_back({5, {6}});
+}