[PATCH] Add llvm::make_unique, according to N3656.

Chandler Carruth chandlerc at gmail.com
Sun Mar 9 01:25:46 PST 2014



================
Comment at: include/llvm/ADT/STLExtras.h:274
@@ +273,3 @@
+template <class T>
+typename std::enable_if<std::is_array<T>::value &&std::extent<T>::value == 0,
+                        std::unique_ptr<T>>::type
----------------
this && seems like it needs a space on the RHS... Maybe a clang-format bug?

================
Comment at: include/llvm/ADT/STLExtras.h:281
@@ +280,3 @@
+template <class T, class... Args>
+typename std::enable_if<std::extent<T>::value != 0>::type
+make_unique(Args &&...) LLVM_DELETED_FUNCTION;
----------------
Do you not need the is_array test here?

================
Comment at: include/llvm/ADT/STLExtras.h:266
@@ +265,3 @@
+
+/// Implement make_unique according to N3656.
+template <class T, class... Args>
----------------
I think it would be good to give a reasonable full description of how to use both this and the array form. You should cite the paper its based on, but not require a caller to read it.

================
Comment at: unittests/ADT/MakeUniqueTest.cpp:17-18
@@ +16,4 @@
+
+// Test fixture
+class MakeUniqueTest : public testing::Test {};
+
----------------
No need for an empty fixture. Just remove the _F suffix below.


http://llvm-reviews.chandlerc.com/D3013



More information about the llvm-commits mailing list