From d0202395f17408b554384034b6476c6272bf1be3 Mon Sep 17 00:00:00 2001 From: Kristof Umann Date: Tue, 28 Aug 2018 14:17:51 +0000 Subject: [PATCH] [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 llvm-svn: 340824 --- llvm/include/llvm/ADT/ImmutableList.h | 29 +++++++++++++++------ llvm/unittests/ADT/ImmutableListTest.cpp | 43 ++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 8 deletions(-) diff --git a/llvm/include/llvm/ADT/ImmutableList.h b/llvm/include/llvm/ADT/ImmutableList.h index deafa27..4bd5620 100644 --- a/llvm/include/llvm/ADT/ImmutableList.h +++ b/llvm/include/llvm/ADT/ImmutableList.h @@ -31,8 +31,9 @@ class ImmutableListImpl : public FoldingSetNode { T Head; const ImmutableListImpl* Tail; - ImmutableListImpl(const T& head, const ImmutableListImpl* tail = nullptr) - : Head(head), Tail(tail) {} + template + ImmutableListImpl(ElemT &&head, const ImmutableListImpl *tail = nullptr) + : Head(std::forward(head)), Tail(tail) {} public: ImmutableListImpl(const ImmutableListImpl &) = delete; @@ -66,6 +67,9 @@ public: using value_type = T; using Factory = ImmutableListFactory; + static_assert(std::is_trivially_destructible::value, + "T must be trivially destructible!"); + private: const ImmutableListImpl* X; @@ -166,7 +170,8 @@ public: if (ownsAllocator()) delete &getAllocator(); } - LLVM_NODISCARD ImmutableList concat(const T &Head, ImmutableList Tail) { + template + LLVM_NODISCARD ImmutableList concat(ElemT &&Head, ImmutableList 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(); - new (L) ListTy(Head, TailImpl); + new (L) ListTy(std::forward(Head), TailImpl); // Insert the new list into the cache. Cache.InsertNode(L, InsertPos); @@ -188,16 +193,24 @@ public: return L; } - LLVM_NODISCARD ImmutableList add(const T& D, ImmutableList L) { - return concat(D, L); + template + LLVM_NODISCARD ImmutableList add(ElemT &&Data, ImmutableList L) { + return concat(std::forward(Data), L); + } + + template + LLVM_NODISCARD ImmutableList emplace(ImmutableList Tail, + CtorArgs &&...Args) { + return concat(T(std::forward(Args)...), Tail); } ImmutableList getEmptyList() const { return ImmutableList(nullptr); } - ImmutableList create(const T& X) { - return concat(X, getEmptyList()); + template + ImmutableList create(ElemT &&Data) { + return concat(std::forward(Data), getEmptyList()); } }; diff --git a/llvm/unittests/ADT/ImmutableListTest.cpp b/llvm/unittests/ADT/ImmutableListTest.cpp index 0e72769..df150cb1 100644 --- a/llvm/unittests/ADT/ImmutableListTest.cpp +++ b/llvm/unittests/ADT/ImmutableListTest.cpp @@ -150,6 +150,49 @@ TEST_F(ImmutableListTest, MultiElemIntListTest) { EXPECT_TRUE(L5.isEqual(L5)); } +template +struct ExplicitCtorWrapper : public Wrapper { + explicit ExplicitCtorWrapper(Fundamental F) : Wrapper(F) {} + ExplicitCtorWrapper(const ExplicitCtorWrapper &) = delete; + ExplicitCtorWrapper(ExplicitCtorWrapper &&) = default; + ExplicitCtorWrapper &operator=(const ExplicitCtorWrapper &) = delete; + ExplicitCtorWrapper &operator=(ExplicitCtorWrapper &&) = default; +}; + +TEST_F(ImmutableListTest, EmplaceIntListTest) { + ImmutableList>::Factory f; + + ImmutableList> L = f.getEmptyList(); + ImmutableList> L2 = f.emplace(L, 3); + + ImmutableList> L3 = + f.add(ExplicitCtorWrapper(2), L2); + + ImmutableList> L4 = + f.emplace(L3, ExplicitCtorWrapper(1)); + + ImmutableList> L5 = + f.add(ExplicitCtorWrapper(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>::Factory f; ImmutableList> L = f.getEmptyList(); -- 2.7.4