[clang-tidy] Don't generate fix for argument constructed from std::initializer_list.

[hokein]

Summary

A follow-up fix of rL311652.

The previous vector in our test is different with std::vector, so
The check still generates fixes for std::vector (`auto p =
std::unique_ptr<Foo>(new Foo({1,2,3}))`) in real world, the patch makes the
vector behavior in test align with std::vector (both AST nodes are the same now).

[ilya-biryukov]

LGTM modulo a change to the 'initializer_list.h'. Do we really need it for this patch?

test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h

30 ↗

(On Diff #129043)

Why do we need to add this destructor in this patch?

[hokein]

test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h

30 ↗

(On Diff #129043)

Yeah, we do need it to reproduce the issue in real world. The AST is different with/without the destructor.

[ilya-biryukov]

test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h

30 ↗

(On Diff #129043)

Because without destructor CXXBindTemporaryExpr does not show up?

[hokein]

test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h

30 ↗

(On Diff #129043)

Yes.

[ilya-biryukov]

test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h

30 ↗

(On Diff #129043)

Thanks for explaining clang's AST to me :-)

clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp

case CXXNewExpr::CallInit: {
     //   std::make_smart_ptr<S>(std::initializer_list<int>({1, 2, 3}), 1);
     //   std::make_smart_ptr<S>(std::initializer_list<int>({}), 1);
     //   std::make_smart_ptr<S2>(std::vector<int>({1}));
     if (const auto *CE = New->getConstructExpr()) {
       for (const auto *Arg : CE->arguments()) {
         if (isa<CXXStdInitializerListExpr>(Arg)) {
           return false;
         }
+        // Check whether we construct a class from a std::initializer_list.
+        // If so, we won't generate the fixes.
+        auto IsStdInitListInitConstructExpr = [](const Expr* E) {
+          assert(E);
+          if (const auto *ImplicitCE = dyn_cast<CXXConstructExpr>(E)) {
+            if (ImplicitCE->isStdInitListInitialization())
+              return true;
+          }
+          return false;
+        };
         // Check the implicit conversion from the std::initializer_list type to
         // a class type.
-        if (const auto *ImplicitCE = dyn_cast<CXXConstructExpr>(Arg)) {
-          if (ImplicitCE->isStdInitListInitialization()) {
+        if (IsStdInitListInitConstructExpr(Arg))
+          return false;
+        // The Arg can be a CXXBindTemporaryExpr, checking its underlying
+        // construct expr.
+        if (const auto * CTE = dyn_cast<CXXBindTemporaryExpr>(Arg)) {
+          if (IsStdInitListInitConstructExpr(CTE->getSubExpr()))
             return false;
-          }
         }
       }
     }
     if (ArraySizeExpr.empty()) {
       SourceRange InitRange = New->getDirectInitRange();
       Diag << FixItHint::CreateRemoval(
           SourceRange(NewStart, InitRange.getBegin()));
       Diag << FixItHint::CreateRemoval(SourceRange(InitRange.getEnd(), NewEnd));

clang-tools-extra/trunk/test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h

public:
   const _E *begin() const { return __begin_; }
   const _E *end() const { return __begin_ + __size_; }
 };
 
 template <class _E>
 class vector {
  public:
   vector(initializer_list<_E> init);
+  ~vector();
 };
 } // namespace std

clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp

 };
 
 struct DPair {
   DPair() : a(0), b(0) {}
   DPair(int x, int y) : a(y), b(x) {}
   int a, b;
 };
 
+template<typename T>
+struct MyVector {
+  MyVector(std::initializer_list<T>);
+};
+
 struct Empty {};
 
 struct E {
   E(std::initializer_list<int>);
   E();
 };
 
 struct F {
   F(std::initializer_list<int>);
   F();
   int a;
 };
 
 struct G {
   G(std::initializer_list<int>);
   G(int);
 };
 
 struct H {
   H(std::vector<int>);
+  H(MyVector<int>, int);
 };
 
 struct I {
   I(G);
 };
 
 namespace {
 class Foo {};
void initialization(int T, Base b) {
 
   std::unique_ptr<H> PH1 = std::unique_ptr<H>(new H({1, 2, 3}));
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
   // CHECK-FIXES: std::unique_ptr<H> PH1 = std::unique_ptr<H>(new H({1, 2, 3}));
   PH1.reset(new H({1, 2, 3}));
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
   // CHECK-FIXES: PH1.reset(new H({1, 2, 3}));
 
+  std::unique_ptr<H> PH2 = std::unique_ptr<H>(new H({1, 2, 3}, 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr<H> PH2 = std::unique_ptr<H>(new H({1, 2, 3}, 1));
+  PH2.reset(new H({1, 2, 3}, 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
+  // CHECK-FIXES: PH2.reset(new H({1, 2, 3}, 1));
+
   std::unique_ptr<I> PI1 = std::unique_ptr<I>(new I(G({1, 2, 3})));
   // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
   // CHECK-FIXES: std::unique_ptr<I> PI1 = std::make_unique<I>(G({1, 2, 3}));
   PI1.reset(new I(G({1, 2, 3})));
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
   // CHECK-FIXES: PI1 = std::make_unique<I>(G({1, 2, 3}));
 
   std::unique_ptr<Foo> FF = std::unique_ptr<Foo>(new Foo());