Support every kind of initialization.

[angelgarcia]

Summary

modernize-make-unique now correctly supports the different kinds of list initialization.

[klimek]

lg

clang-tidy/modernize/MakeUniqueCheck.cpp

125–128

Nit: this is not a doxygen comment, so \code \endcode doesn't make too much sense; just leave them out. (here and elsewhere)

[angelgarcia]

Remove \code and \endcode.

clang-tidy/modernize/MakeUniqueCheck.cpp

Finder->addMatcher(
         this);
   }
 }
 
 void MakeUniqueCheck::check(const MatchFinder::MatchResult &Result) {
   SourceManager &SM = *Result.SourceManager;
   const auto *Construct =
       Result.Nodes.getNodeAs<CXXConstructExpr>(ConstructorCall);
-  const auto *New = Result.Nodes.getNodeAs<CXXNewExpr>(NewExpression);
   const auto *Type = Result.Nodes.getNodeAs<QualType>(PointerType);
+  const auto *New = Result.Nodes.getNodeAs<CXXNewExpr>(NewExpression);
+
+  if (New->getNumPlacementArgs() != 0)
+    return;
 
   SourceLocation ConstructCallStart = Construct->getExprLoc();
 
   bool Invalid = false;
   StringRef ExprStr = Lexer::getSourceText(
       CharSourceRange::getCharRange(
           ConstructCallStart, Construct->getParenOrBraceRange().getBegin()),
       SM, LangOptions(), &Invalid);
void MakeUniqueCheck::check(const MatchFinder::MatchResult &Result) {
   } else {
     ConstructCallEnd = ConstructCallStart.getLocWithOffset(LAngle);
   }
 
   Diag << FixItHint::CreateReplacement(
       CharSourceRange::getCharRange(ConstructCallStart, ConstructCallEnd),
       "std::make_unique");
 
+  // If the unique_ptr is built with brace enclosed direct initialization, use
+  // parenthesis instead.
+  if (Construct->isListInitialization()) {
+    SourceRange BraceRange = Construct->getParenOrBraceRange();
+    Diag << FixItHint::CreateReplacement(
+        CharSourceRange::getCharRange(
+            BraceRange.getBegin(), BraceRange.getBegin().getLocWithOffset(1)),
+        "(");
+    Diag << FixItHint::CreateReplacement(
+        CharSourceRange::getCharRange(BraceRange.getEnd(),
+                                      BraceRange.getEnd().getLocWithOffset(1)),
+        ")");
+  }
+
   SourceLocation NewStart = New->getSourceRange().getBegin();
   SourceLocation NewEnd = New->getSourceRange().getEnd();
   switch (New->getInitializationStyle()) {
   case CXXNewExpr::NoInit: {
     Diag << FixItHint::CreateRemoval(SourceRange(NewStart, NewEnd));
     break;
   }
   case CXXNewExpr::CallInit: {
     SourceRange InitRange = New->getDirectInitRange();
     Diag << FixItHint::CreateRemoval(
         SourceRange(NewStart, InitRange.getBegin()));
     Diag << FixItHint::CreateRemoval(SourceRange(InitRange.getEnd(), NewEnd));
     break;
   }
   case CXXNewExpr::ListInit: {
-    SourceRange InitRange = New->getInitializer()->getSourceRange();
+    // Range of the substring that we do not want to remove.
+    SourceRange InitRange;
+    if (const auto *NewConstruct = New->getConstructExpr()) {
+      // Direct initialization with initialization list.
+      //   struct S { S(int x) {} };
+      //   std::unique_ptr<S>(new S{5});
+      // The arguments in the initialization list are going to be forwarded to
+      // the constructor, so this has to be replaced with:
+      //   struct S { S(int x) {} };
+      //   std::make_unique<S>(5);
+      InitRange = SourceRange(
+          NewConstruct->getParenOrBraceRange().getBegin().getLocWithOffset(1),
+          NewConstruct->getParenOrBraceRange().getEnd().getLocWithOffset(-1));
+    } else {
+      // Aggregate initialization.
+      //   std::unique_ptr<Pair>(new Pair{first, second});
+      // Has to be replaced with:
+      //   std::make_unique<Pair>(Pair{first, second});
+      InitRange = SourceRange(
+          New->getAllocatedTypeSourceInfo()->getTypeLoc().getLocStart(),
+          New->getInitializer()->getSourceRange().getEnd());
+    }
     Diag << FixItHint::CreateRemoval(
-        SourceRange(NewStart, InitRange.getBegin().getLocWithOffset(-1)));
+        CharSourceRange::getCharRange(NewStart, InitRange.getBegin()));
     Diag << FixItHint::CreateRemoval(
         SourceRange(InitRange.getEnd().getLocWithOffset(1), NewEnd));
     break;
   }
   }
 }
 
 } // namespace modernize
 } // namespace tidy
 } // namespace clang

test/clang-tidy/modernize-make-unique.cpp

struct Base {
   Base(int, int);
 };
 
 struct Derived : public Base {
   Derived();
   Derived(int, int);
 };
 
-struct Pair {
+struct APair {
   int a, b;
 };
 
+struct DPair {
+  DPair() : a(0), b(0) {}
+  DPair(int x, int y) : a(y), b(x) {}
+  int a, b;
+};
+
+struct Empty {};
+
 template<class T> using unique_ptr_ = std::unique_ptr<T>;
 
+void *operator new(unsigned long Count, void *Ptr);
+
 int g(std::unique_ptr<int> P);
 
 std::unique_ptr<Base> getPointer() {
   return std::unique_ptr<Base>(new Base);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use std::make_unique instead
   // CHECK-FIXES: return std::make_unique<Base>();
 }
 
-void f() {
+void basic() {
   std::unique_ptr<int> P1 = std::unique_ptr<int>(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: std::unique_ptr<int> P1 = std::make_unique<int>();
 
   // Without parenthesis.
   std::unique_ptr<int> P2 = std::unique_ptr<int>(new int);
   // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use std::make_unique instead [modernize-make-unique]
   // CHECK-FIXES: std::unique_ptr<int> P2 = std::make_unique<int>();
void basic() {
 
   std::unique_ptr<int> R(new int());
 
   // Create the unique_ptr as a parameter to a function.
   int T = g(std::unique_ptr<int>(new int()));
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use std::make_unique instead
   // CHECK-FIXES: int T = g(std::make_unique<int>());
 
-  // Arguments are correctly handled.
-  std::unique_ptr<Base> Pbase = std::unique_ptr<Base>(new Base(5, T));
-  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr<Base> Pbase = std::make_unique<Base>(5, T);
-
-  // Works with init lists.
-  std::unique_ptr<Pair> Ppair = std::unique_ptr<Pair>(new Pair{T, 1});
-  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: use std::make_unique instead
-  // CHECK-FIXES: std::unique_ptr<Pair> Ppair = std::make_unique<Pair>({T, 1});
-
   // Only replace if the type in the template is the same than the type returned
   // by the new operator.
   auto Pderived = std::unique_ptr<Base>(new Derived());
 
   // The pointer is returned by the function, nothing to do.
   std::unique_ptr<Base> RetPtr = getPointer();
 
-  // Aliases.
+  // This emulates std::move.
+  std::unique_ptr<int> Move = static_cast<std::unique_ptr<int>&&>(P1);
+
+  // Placemenet arguments should not be removed.
+  int *PInt = new int;
+  std::unique_ptr<int> Placement = std::unique_ptr<int>(new (PInt) int{3});
+}
+
+void initialization(int T, Base b) {
+  // Test different kinds of initialization of the pointee.
+
+  // Direct initialization with parenthesis.
+  std::unique_ptr<DPair> PDir1 = std::unique_ptr<DPair>(new DPair(1, T));
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr<DPair> PDir1 = std::make_unique<DPair>(1, T);
+
+  // Direct initialization with braces.
+  std::unique_ptr<DPair> PDir2 = std::unique_ptr<DPair>(new DPair{2, T});
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr<DPair> PDir2 = std::make_unique<DPair>(2, T);
+
+  // Aggregate initialization.
+  std::unique_ptr<APair> PAggr = std::unique_ptr<APair>(new APair{T, 1});
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr<APair> PAggr = std::make_unique<APair>(APair{T, 1});
+
+
+  // Test different kinds of initialization of the pointee, when the unique_ptr
+  // is initialized with braces.
+
+  // Direct initialization with parenthesis.
+  std::unique_ptr<DPair> PDir3 = std::unique_ptr<DPair>{new DPair(3, T)};
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr<DPair> PDir3 = std::make_unique<DPair>(3, T);
+
+  // Direct initialization with braces.
+  std::unique_ptr<DPair> PDir4 = std::unique_ptr<DPair>{new DPair{4, T}};
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr<DPair> PDir4 = std::make_unique<DPair>(4, T);
+
+  // Aggregate initialization.
+  std::unique_ptr<APair> PAggr2 = std::unique_ptr<APair>{new APair{T, 2}};
+  // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr<APair> PAggr2 = std::make_unique<APair>(APair{T, 2});
+
+
+  // Direct initialization with parenthesis, without arguments.
+  std::unique_ptr<DPair> PDir5 = std::unique_ptr<DPair>(new DPair());
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr<DPair> PDir5 = std::make_unique<DPair>();
+
+  // Direct initialization with braces, without arguments.
+  std::unique_ptr<DPair> PDir6 = std::unique_ptr<DPair>(new DPair{});
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr<DPair> PDir6 = std::make_unique<DPair>();
+
+  // Aggregate initialization without arguments.
+  std::unique_ptr<Empty> PEmpty = std::unique_ptr<Empty>(new Empty{});
+  // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: use std::make_unique instead
+  // CHECK-FIXES: std::unique_ptr<Empty> PEmpty = std::make_unique<Empty>(Empty{});
+}
+
+void aliases() {
   typedef std::unique_ptr<int> IntPtr;
   IntPtr Typedef = IntPtr(new int);
   // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use std::make_unique instead
   // CHECK-FIXES: IntPtr Typedef = std::make_unique<int>();
 
 #define PTR unique_ptr<int>
   std::unique_ptr<int> Macro = std::PTR(new int);
   // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead
   // CHECK-FIXES: std::unique_ptr<int> Macro = std::make_unique<int>();
 #undef PTR
 
   std::unique_ptr<int> Using = unique_ptr_<int>(new int);
   // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use std::make_unique instead
   // CHECK-FIXES: std::unique_ptr<int> Using = std::make_unique<int>();
+}
 
-  // This emulates std::move.
-  std::unique_ptr<int> Move = static_cast<std::unique_ptr<int>&&>(P1);
-
-  // Adding whitespaces.
+void whitespaces() {
   auto Space = std::unique_ptr <int>(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use std::make_unique instead
   // CHECK-FIXES: auto Space = std::make_unique<int>();
 
   auto Spaces = std  ::    unique_ptr  <int>(new int());
   // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use std::make_unique instead
   // CHECK-FIXES: auto Spaces = std::make_unique<int>();
 }