[clang-tools-extra] r311078 - [clang-tidy] Don't generate fixes for initializer_list constructor in make_unique check.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 17 03:14:52 PDT 2017
Author: hokein
Date: Thu Aug 17 03:14:52 2017
New Revision: 311078
URL: http://llvm.org/viewvc/llvm-project?rev=311078&view=rev
Log:
[clang-tidy] Don't generate fixes for initializer_list constructor in make_unique check.
Summary:
The current fix will break the compilation -- because braced list is not
deducible in std::make_unique (with the use of forwarding) without
specifying the type explicitly.
We could support it in the future.
Reviewers: alexfh
Reviewed By: alexfh
Subscribers: JDevlieghere, xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D36786
Modified:
clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h
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=311078&r1=311077&r2=311078&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp Thu Aug 17 03:14:52 2017
@@ -147,6 +147,10 @@ void MakeSmartPtrCheck::checkConstruct(S
return;
}
+ if (!replaceNew(Diag, New, SM)) {
+ return;
+ }
+
// Find the location of the template's left angle.
size_t LAngle = ExprStr.find("<");
SourceLocation ConstructCallEnd;
@@ -179,7 +183,6 @@ void MakeSmartPtrCheck::checkConstruct(S
")");
}
- replaceNew(Diag, New, SM);
insertHeader(Diag, SM.getFileID(ConstructCallStart));
}
@@ -214,6 +217,10 @@ void MakeSmartPtrCheck::checkReset(Sourc
return;
}
+ if (!replaceNew(Diag, New, SM)) {
+ return;
+ }
+
Diag << FixItHint::CreateReplacement(
CharSourceRange::getCharRange(OperatorLoc, ExprEnd),
(llvm::Twine(" = ") + MakeSmartPtrFunctionName + "<" +
@@ -223,11 +230,10 @@ void MakeSmartPtrCheck::checkReset(Sourc
if (Expr->isArrow())
Diag << FixItHint::CreateInsertion(ExprStart, "*");
- replaceNew(Diag, New, SM);
insertHeader(Diag, SM.getFileID(OperatorLoc));
}
-void MakeSmartPtrCheck::replaceNew(DiagnosticBuilder &Diag,
+bool MakeSmartPtrCheck::replaceNew(DiagnosticBuilder &Diag,
const CXXNewExpr *New,
SourceManager& SM) {
SourceLocation NewStart = New->getSourceRange().getBegin();
@@ -254,6 +260,22 @@ void MakeSmartPtrCheck::replaceNew(Diagn
break;
}
case CXXNewExpr::CallInit: {
+ // FIXME: Add fixes for constructors with initializer-list parameters.
+ // Unlike ordinal cases, braced list can not be deduced in
+ // std::make_smart_ptr, we need to specify the type explicitly in the fixes:
+ // struct S { S(std::initializer_list<int>, int); };
+ // smart_ptr<S>(new S({1, 2, 3}, 1)); // C++98 call-style initialization
+ // smart_ptr<S>(new S({}, 1));
+ // 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);
+ if (const auto *CE = New->getConstructExpr()) {
+ for (const auto *Arg : CE->arguments()) {
+ if (llvm::isa<CXXStdInitializerListExpr>(Arg)) {
+ return false;
+ }
+ }
+ }
if (ArraySizeExpr.empty()) {
SourceRange InitRange = New->getDirectInitRange();
Diag << FixItHint::CreateRemoval(
@@ -274,19 +296,16 @@ void MakeSmartPtrCheck::replaceNew(Diagn
SourceRange InitRange;
if (const auto *NewConstruct = New->getConstructExpr()) {
if (NewConstruct->isStdInitListInitialization()) {
- // Direct Initialization with the initializer-list constructor.
- // struct S { S(std::initializer_list<T>); };
- // smart_ptr<S>(new S{1, 2, 3});
- // smart_ptr<S>(new S{}); // use initializer-list consturctor
- // The brace has to be kept, so this has to be replaced with:
- // std::make_smart_ptr<S>({1, 2, 3});
- // std::make_smart_ptr<S>({});
- unsigned NumArgs = NewConstruct->getNumArgs();
- if (NumArgs == 0) {
- return;
- }
- InitRange = SourceRange(NewConstruct->getArg(0)->getLocStart(),
- NewConstruct->getArg(NumArgs - 1)->getLocEnd());
+ // 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>); };
+ // smart_ptr<S>(new S{1, 2, 3}); // C++11 direct list-initialization
+ // smart_ptr<S>(new S{}); // use initializer-list consturctor
+ // 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>({}));
+ return false;
} else {
// Direct initialization with ordinary constructors.
// struct S { S(int x); S(); };
@@ -316,6 +335,7 @@ void MakeSmartPtrCheck::replaceNew(Diagn
break;
}
}
+ return true;
}
void MakeSmartPtrCheck::insertHeader(DiagnosticBuilder &Diag, FileID FD) {
Modified: clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h?rev=311078&r1=311077&r2=311078&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.h Thu Aug 17 03:14:52 2017
@@ -57,7 +57,8 @@ private:
void checkReset(SourceManager &SM, const CXXMemberCallExpr *Member,
const CXXNewExpr *New);
- void replaceNew(DiagnosticBuilder &Diag, const CXXNewExpr *New,
+ /// Returns true when the fixes for replacing CXXNewExpr are generated.
+ bool replaceNew(DiagnosticBuilder &Diag, const CXXNewExpr *New,
SourceManager &SM);
void insertHeader(DiagnosticBuilder &Diag, FileID FD);
};
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=311078&r1=311077&r2=311078&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 Thu Aug 17 03:14:52 2017
@@ -246,52 +246,75 @@ void initialization(int T, Base b) {
std::unique_ptr<E> PE1 = std::unique_ptr<E>(new E{});
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
// CHECK-FIXES: std::unique_ptr<E> PE1 = std::make_unique<E>();
+ PE1.reset(new E{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
+ // CHECK-FIXES: PE1 = std::make_unique<E>();
+
+ //============================================================================
+ // NOTE: For initlializer-list constructors, the check only gives warnings,
+ // and no fixes are generated.
+ //============================================================================
// Initialization with the initializer-list constructor.
std::unique_ptr<E> PE2 = std::unique_ptr<E>(new E{1, 2});
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
- // CHECK-FIXES: std::unique_ptr<E> PE2 = std::make_unique<E>({1, 2});
+ // CHECK-FIXES: std::unique_ptr<E> PE2 = std::unique_ptr<E>(new E{1, 2});
+ PE2.reset(new E{1, 2});
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
+ // CHECK-FIXES: PE2.reset(new E{1, 2});
// Initialization with default constructor.
std::unique_ptr<F> PF1 = std::unique_ptr<F>(new F());
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
// CHECK-FIXES: std::unique_ptr<F> PF1 = std::make_unique<F>();
+ PF1.reset(new F());
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
+ // CHECK-FIXES: PF1 = std::make_unique<F>();
// Initialization with default constructor.
std::unique_ptr<F> PF2 = std::unique_ptr<F>(new F{});
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
// CHECK-FIXES: std::unique_ptr<F> PF2 = std::make_unique<F>();
+ PF2.reset(new F());
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
+ // CHECK-FIXES: PF2 = std::make_unique<F>();
// Initialization with the initializer-list constructor.
std::unique_ptr<F> PF3 = std::unique_ptr<F>(new F{1});
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
- // CHECK-FIXES: std::unique_ptr<F> PF3 = std::make_unique<F>({1});
+ // CHECK-FIXES: std::unique_ptr<F> PF3 = std::unique_ptr<F>(new F{1});
+ PF3.reset(new F{1});
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
+ // CHECK-FIXES: PF3.reset(new F{1});
// Initialization with the initializer-list constructor.
std::unique_ptr<F> PF4 = std::unique_ptr<F>(new F{1, 2});
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
- // CHECK-FIXES: std::unique_ptr<F> PF4 = std::make_unique<F>({1, 2});
+ // CHECK-FIXES: std::unique_ptr<F> PF4 = std::unique_ptr<F>(new F{1, 2});
// Initialization with the initializer-list constructor.
std::unique_ptr<F> PF5 = std::unique_ptr<F>(new F({1, 2}));
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
- // CHECK-FIXES: std::unique_ptr<F> PF5 = std::make_unique<F>({1, 2});
+ // CHECK-FIXES: std::unique_ptr<F> PF5 = std::unique_ptr<F>(new F({1, 2}));
// Initialization with the initializer-list constructor as the default
// constructor is not present.
std::unique_ptr<G> PG1 = std::unique_ptr<G>(new G{});
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
- // CHECK-FIXES: std::unique_ptr<G> PG1 = std::make_unique<G>({});
+ // CHECK-FIXES: std::unique_ptr<G> PG1 = std::unique_ptr<G>(new G{});
+ PG1.reset(new G{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
+ // CHECK-FIXES: PG1.reset(new G{});
// Initialization with the initializer-list constructor.
std::unique_ptr<G> PG2 = std::unique_ptr<G>(new G{1});
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
- // CHECK-FIXES: std::unique_ptr<G> PG2 = std::make_unique<G>({1});
+ // CHECK-FIXES: std::unique_ptr<G> PG2 = std::unique_ptr<G>(new G{1});
// Initialization with the initializer-list constructor.
std::unique_ptr<G> PG3 = std::unique_ptr<G>(new G{1, 2});
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
- // CHECK-FIXES: std::unique_ptr<G> PG3 = std::make_unique<G>({1, 2});
+ // CHECK-FIXES: std::unique_ptr<G> PG3 = std::unique_ptr<G>(new G{1, 2});
std::unique_ptr<Foo> FF = std::unique_ptr<Foo>(new Foo());
// CHECK-MESSAGES: :[[@LINE-1]]:29: warning:
More information about the cfe-commits
mailing list