[llvm] r340824 - [ADT] ImmutableList no longer requires elements to be copy constructible

[Kristof Umann via llvm-commits]

Author: szelethus
Date: Tue Aug 28 07:17:51 2018
New Revision: 340824

URL: http://llvm.org/viewvc/llvm-project?rev=340824&view=rev
Log:
[ADT] ImmutableList no longer requires elements to be copy constructible

ImmutableList used to require elements to have a copy constructor for no
good reason, this patch aims to fix this.
It also required but did not enforce its elements to be trivially
destructible, so a new static_assert is added to guard against misuse.

Differential Revision: https://reviews.llvm.org/D49985

Modified:
    llvm/trunk/include/llvm/ADT/ImmutableList.h
    llvm/trunk/unittests/ADT/ImmutableListTest.cpp

Modified: llvm/trunk/include/llvm/ADT/ImmutableList.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/ImmutableList.h?rev=340824&r1=340823&r2=340824&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/ImmutableList.h (original)
+++ llvm/trunk/include/llvm/ADT/ImmutableList.h Tue Aug 28 07:17:51 2018
@@ -31,8 +31,9 @@ class ImmutableListImpl : public Folding
   T Head;
   const ImmutableListImpl* Tail;
 
-  ImmutableListImpl(const T& head, const ImmutableListImpl* tail = nullptr)
-    : Head(head), Tail(tail) {}
+  template <typename ElemT>
+  ImmutableListImpl(ElemT &&head, const ImmutableListImpl *tail = nullptr)
+    : Head(std::forward<ElemT>(head)), Tail(tail) {}
 
 public:
   ImmutableListImpl(const ImmutableListImpl &) = delete;
@@ -66,6 +67,9 @@ public:
   using value_type = T;
   using Factory = ImmutableListFactory<T>;
 
+  static_assert(std::is_trivially_destructible<T>::value,
+                "T must be trivially destructible!");
+
 private:
   const ImmutableListImpl<T>* X;
 
@@ -166,7 +170,8 @@ public:
     if (ownsAllocator()) delete &getAllocator();
   }
 
-  LLVM_NODISCARD ImmutableList<T> concat(const T &Head, ImmutableList<T> Tail) {
+  template <typename ElemT>
+  LLVM_NODISCARD ImmutableList<T> concat(ElemT &&Head, ImmutableList<T> Tail) {
     // Profile the new list to see if it already exists in our cache.
     FoldingSetNodeID ID;
     void* InsertPos;
@@ -179,7 +184,7 @@ public:
       // The list does not exist in our cache.  Create it.
       BumpPtrAllocator& A = getAllocator();
       L = (ListTy*) A.Allocate<ListTy>();
-      new (L) ListTy(Head, TailImpl);
+      new (L) ListTy(std::forward<ElemT>(Head), TailImpl);
 
       // Insert the new list into the cache.
       Cache.InsertNode(L, InsertPos);
@@ -188,16 +193,24 @@ public:
     return L;
   }
 
-  LLVM_NODISCARD ImmutableList<T> add(const T& D, ImmutableList<T> L) {
-    return concat(D, L);
+  template <typename ElemT>
+  LLVM_NODISCARD ImmutableList<T> add(ElemT &&Data, ImmutableList<T> L) {
+    return concat(std::forward<ElemT>(Data), L);
+  }
+
+  template <typename ...CtorArgs>
+  LLVM_NODISCARD ImmutableList<T> emplace(ImmutableList<T> Tail,
+                                          CtorArgs &&...Args) {
+    return concat(T(std::forward<CtorArgs>(Args)...), Tail);
   }
 
   ImmutableList<T> getEmptyList() const {
     return ImmutableList<T>(nullptr);
   }
 
-  ImmutableList<T> create(const T& X) {
-    return concat(X, getEmptyList());
+  template <typename ElemT>
+  ImmutableList<T> create(ElemT &&Data) {
+    return concat(std::forward<ElemT>(Data), getEmptyList());
   }
 };
 

Modified: llvm/trunk/unittests/ADT/ImmutableListTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/ImmutableListTest.cpp?rev=340824&r1=340823&r2=340824&view=diff
==============================================================================
--- llvm/trunk/unittests/ADT/ImmutableListTest.cpp (original)
+++ llvm/trunk/unittests/ADT/ImmutableListTest.cpp Tue Aug 28 07:17:51 2018
@@ -150,6 +150,49 @@ TEST_F(ImmutableListTest, MultiElemIntLi
   EXPECT_TRUE(L5.isEqual(L5));
 }
 
+template <typename Fundamental>
+struct ExplicitCtorWrapper : public Wrapper<Fundamental> {
+  explicit ExplicitCtorWrapper(Fundamental F) : Wrapper<Fundamental>(F) {}
+  ExplicitCtorWrapper(const ExplicitCtorWrapper &) = delete;
+  ExplicitCtorWrapper(ExplicitCtorWrapper &&) = default;
+  ExplicitCtorWrapper &operator=(const ExplicitCtorWrapper &) = delete;
+  ExplicitCtorWrapper &operator=(ExplicitCtorWrapper &&) = default;
+};
+
+TEST_F(ImmutableListTest, EmplaceIntListTest) {
+  ImmutableList<ExplicitCtorWrapper<int>>::Factory f;
+
+  ImmutableList<ExplicitCtorWrapper<int>> L = f.getEmptyList();
+  ImmutableList<ExplicitCtorWrapper<int>> L2 = f.emplace(L, 3);
+
+  ImmutableList<ExplicitCtorWrapper<int>> L3 =
+      f.add(ExplicitCtorWrapper<int>(2), L2);
+
+  ImmutableList<ExplicitCtorWrapper<int>> L4 =
+      f.emplace(L3, ExplicitCtorWrapper<int>(1));
+
+  ImmutableList<ExplicitCtorWrapper<int>> L5 =
+      f.add(ExplicitCtorWrapper<int>(1), L3);
+
+  EXPECT_FALSE(L2.isEmpty());
+  EXPECT_TRUE(L2.getTail().isEmpty());
+  EXPECT_EQ(3, L2.getHead());
+  EXPECT_TRUE(L.isEqual(L2.getTail()));
+  EXPECT_TRUE(L2.getTail().isEqual(L));
+
+  EXPECT_FALSE(L3.isEmpty());
+  EXPECT_FALSE(L2 == L3);
+  EXPECT_EQ(2, L3.getHead());
+  EXPECT_TRUE(L2 == L3.getTail());
+
+  EXPECT_FALSE(L4.isEmpty());
+  EXPECT_EQ(1, L4.getHead());
+  EXPECT_TRUE(L3 == L4.getTail());
+
+  EXPECT_TRUE(L4 == L5);
+  EXPECT_TRUE(L3 == L5.getTail());
+}
+
 TEST_F(ImmutableListTest, CharListOrderingTest) {
   ImmutableList<Wrapper<char>>::Factory f;
   ImmutableList<Wrapper<char>> L = f.getEmptyList();