[clang-tools-extra] r347315 - [clang-tidy] Don't generate incorrect fixes for class constructed from list-initialized arguments
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 20 07:45:15 PST 2018
Author: hokein
Date: Tue Nov 20 07:45:15 2018
New Revision: 347315
URL: http://llvm.org/viewvc/llvm-project?rev=347315&view=rev
Log:
[clang-tidy] Don't generate incorrect fixes for class constructed from list-initialized arguments
Summary:
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`.
```
struct Bar { int a, b; };
struct Foo { Foo(Bar); };
auto foo = std::unique_ptr<Foo>(new Foo({1, 2}));
```
Reviewers: aaron.ballman
Subscribers: xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D54704
Modified:
clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp
Modified: clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp?rev=347315&r1=347314&r2=347315&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp Tue Nov 20 07:45:15 2018
@@ -280,7 +280,27 @@ bool MakeSmartPtrCheck::replaceNew(Diagn
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
+ // 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.
+ if (const auto *ImplicitCE =
+ dyn_cast<CXXConstructExpr>(Arg->IgnoreImplicit())) {
+ if (ImplicitCE->isStdInitListInitialization())
+ return true;
+ }
+ return false;
+ }
+ return false;
+ };
switch (New->getInitializationStyle()) {
case CXXNewExpr::NoInit: {
if (ArraySizeExpr.empty()) {
@@ -300,32 +320,20 @@ bool MakeSmartPtrCheck::replaceNew(Diagn
// 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 +354,20 @@ bool MakeSmartPtrCheck::replaceNew(Diagn
// 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.
Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp?rev=347315&r1=347314&r2=347315&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp Tue Nov 20 07:45:15 2018
@@ -58,6 +58,10 @@ struct I {
I(G);
};
+struct J {
+ J(E e, int);
+};
+
namespace {
class Foo {};
} // namespace
@@ -372,6 +376,34 @@ void initialization(int T, Base b) {
// 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>();
More information about the cfe-commits
mailing list