hokein created this revision.
hokein added a reviewer: aaron.ballman.
Herald added a subscriber: xazax.hun.
Currently the smart_ptr check (modernize-make-unique) generates the
fixes that cannot compile for cases like below -- because brace list can
not be deduced in `make_unique`.
class Bar { int a, b; };
class Foo { Foo(Bar); };
auto foo = std::unique_ptr<Foo>(new Foo({1, 2}));
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54704
Files:
clang-tidy/modernize/MakeSmartPtrCheck.cpp
test/clang-tidy/modernize-make-unique.cpp
Index: test/clang-tidy/modernize-make-unique.cpp
===================================================================
--- test/clang-tidy/modernize-make-unique.cpp
+++ test/clang-tidy/modernize-make-unique.cpp
@@ -58,6 +58,10 @@
I(G);
};
+struct J {
+ J(E e, int);
+};
+
namespace {
class Foo {};
} // namespace
@@ -372,6 +376,34 @@
// 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<J> PJ1 = std::unique_ptr<J>(new J({1, 2}, 1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
+ // CHECK-FIXES: std::unique_ptr<J> PJ1 = std::unique_ptr<J>(new J({1, 2}, 1));
+ PJ1.reset(new J({1, 2}, 1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
+ // CHECK-FIXES: PJ1.reset(new J({1, 2}, 1));
+
+ std::unique_ptr<J> PJ2 = std::unique_ptr<J>(new J(E{1, 2}, 1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
+ // CHECK-FIXES: std::unique_ptr<J> PJ2 = std::make_unique<J>(E{1, 2}, 1);
+ PJ2.reset(new J(E{1, 2}, 1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
+ // CHECK-FIXES: PJ2 = std::make_unique<J>(E{1, 2}, 1);
+
+ std::unique_ptr<J> PJ3 = std::unique_ptr<J>(new J{ {1, 2}, 1 });
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
+ // CHECK-FIXES: std::unique_ptr<J> PJ3 = std::unique_ptr<J>(new J{ {1, 2}, 1 });
+ PJ3.reset(new J{ {1, 2}, 1 });
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
+ // CHECK-FIXES: PJ3.reset(new J{ {1, 2}, 1 });
+
+ std::unique_ptr<J> PJ4 = std::unique_ptr<J>(new J{E{1, 2}, 1});
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
+ // CHECK-FIXES: std::unique_ptr<J> PJ4 = std::make_unique<J>(E{1, 2}, 1);
+ PJ4.reset(new J{E{1, 2}, 1});
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
+ // CHECK-FIXES: PJ4 = std::make_unique<J>(E{1, 2}, 1);
+
std::unique_ptr<Foo> FF = std::unique_ptr<Foo>(new Foo());
// CHECK-MESSAGES: :[[@LINE-1]]:29: warning:
// CHECK-FIXES: std::unique_ptr<Foo> FF = std::make_unique<Foo>();
Index: clang-tidy/modernize/MakeSmartPtrCheck.cpp
===================================================================
--- clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -280,7 +280,29 @@
SM, getLangOpts())
.str();
}
-
+ // Returns true if the given constructor expression has any braced-init-list
+ // argument, e.g.
+ // Foo({1, 2}, 1) => true
+ // Foo(Bar{1, 2}) => true
+ // Foo(1) => false
+ auto HasListIntializedArgument = [](const CXXConstructExpr *CE) {
+ for (const auto *Arg : CE->arguments()) {
+ if (isa<CXXStdInitializerListExpr>(Arg) || isa<InitListExpr>(Arg))
+ return true;
+ // Check whether we implicitly construct a class from a
+ // std::initializer_list.
+ auto IsStdInitListInitConstructExpr = [](const Expr *E) {
+ assert(E);
+ if (const auto *ImplicitCE = dyn_cast<CXXConstructExpr>(E)) {
+ if (ImplicitCE->isStdInitListInitialization())
+ return true;
+ }
+ return false;
+ };
+ return IsStdInitListInitConstructExpr(Arg->IgnoreImplicit());
+ }
+ return false;
+ };
switch (New->getInitializationStyle()) {
case CXXNewExpr::NoInit: {
if (ArraySizeExpr.empty()) {
@@ -300,32 +322,20 @@
// std::make_smart_ptr, we need to specify the type explicitly in the fixes:
// struct S { S(std::initializer_list<int>, int); };
// struct S2 { S2(std::vector<int>); };
+ // struct S3 { S3(S2, int); };
// smart_ptr<S>(new S({1, 2, 3}, 1)); // C++98 call-style initialization
// smart_ptr<S>(new S({}, 1));
// smart_ptr<S2>(new S2({1})); // implicit conversion:
// // std::initializer_list => std::vector
+ // smart_ptr<S3>(new S3({1, 2}, 3));
// The above samples have to be replaced with:
// 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}));
+ // std::make_smart_ptr<S3>(S2{1, 2}, 3);
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;
- };
- if (IsStdInitListInitConstructExpr(Arg->IgnoreImplicit()))
- return false;
- }
+ if (HasListIntializedArgument(CE))
+ return false;
}
if (ArraySizeExpr.empty()) {
SourceRange InitRange = New->getDirectInitRange();
@@ -346,16 +356,20 @@
// Range of the substring that we do not want to remove.
SourceRange InitRange;
if (const auto *NewConstruct = New->getConstructExpr()) {
- if (NewConstruct->isStdInitListInitialization()) {
+ if (NewConstruct->isStdInitListInitialization() ||
+ HasListIntializedArgument(NewConstruct)) {
// FIXME: Add fixes for direct initialization with the initializer-list
// constructor. Similar to the above CallInit case, the type has to be
// specified explicitly in the fixes.
// struct S { S(std::initializer_list<int>); };
+ // struct S2 { S2(S, int); };
// smart_ptr<S>(new S{1, 2, 3}); // C++11 direct list-initialization
// smart_ptr<S>(new S{}); // use initializer-list consturctor
+ // smart_ptr<S2>()new S2{ {1,2}, 3 }; // have a list-initialized arg
// The above cases have to be replaced with:
// std::make_smart_ptr<S>(std::initializer_list<int>({1, 2, 3}));
// std::make_smart_ptr<S>(std::initializer_list<int>({}));
+ // std::make_smart_ptr<S2>(S{1, 2}, 3);
return false;
} else {
// Direct initialization with ordinary constructors.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
