Skip to content
This repository was archived by the owner on Mar 28, 2020. It is now read-only.

Commit 0977b80

Browse files
committed
[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 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@340824 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent 939bd02 commit 0977b80

File tree

2 files changed

+64
-8
lines changed

2 files changed

+64
-8
lines changed

include/llvm/ADT/ImmutableList.h

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@ class ImmutableListImpl : public FoldingSetNode {
3131
T Head;
3232
const ImmutableListImpl* Tail;
3333

34-
ImmutableListImpl(const T& head, const ImmutableListImpl* tail = nullptr)
35-
: Head(head), Tail(tail) {}
34+
template <typename ElemT>
35+
ImmutableListImpl(ElemT &&head, const ImmutableListImpl *tail = nullptr)
36+
: Head(std::forward<ElemT>(head)), Tail(tail) {}
3637

3738
public:
3839
ImmutableListImpl(const ImmutableListImpl &) = delete;
@@ -66,6 +67,9 @@ class ImmutableList {
6667
using value_type = T;
6768
using Factory = ImmutableListFactory<T>;
6869

70+
static_assert(std::is_trivially_destructible<T>::value,
71+
"T must be trivially destructible!");
72+
6973
private:
7074
const ImmutableListImpl<T>* X;
7175

@@ -166,7 +170,8 @@ class ImmutableListFactory {
166170
if (ownsAllocator()) delete &getAllocator();
167171
}
168172

169-
LLVM_NODISCARD ImmutableList<T> concat(const T &Head, ImmutableList<T> Tail) {
173+
template <typename ElemT>
174+
LLVM_NODISCARD ImmutableList<T> concat(ElemT &&Head, ImmutableList<T> Tail) {
170175
// Profile the new list to see if it already exists in our cache.
171176
FoldingSetNodeID ID;
172177
void* InsertPos;
@@ -179,7 +184,7 @@ class ImmutableListFactory {
179184
// The list does not exist in our cache. Create it.
180185
BumpPtrAllocator& A = getAllocator();
181186
L = (ListTy*) A.Allocate<ListTy>();
182-
new (L) ListTy(Head, TailImpl);
187+
new (L) ListTy(std::forward<ElemT>(Head), TailImpl);
183188

184189
// Insert the new list into the cache.
185190
Cache.InsertNode(L, InsertPos);
@@ -188,16 +193,24 @@ class ImmutableListFactory {
188193
return L;
189194
}
190195

191-
LLVM_NODISCARD ImmutableList<T> add(const T& D, ImmutableList<T> L) {
192-
return concat(D, L);
196+
template <typename ElemT>
197+
LLVM_NODISCARD ImmutableList<T> add(ElemT &&Data, ImmutableList<T> L) {
198+
return concat(std::forward<ElemT>(Data), L);
199+
}
200+
201+
template <typename ...CtorArgs>
202+
LLVM_NODISCARD ImmutableList<T> emplace(ImmutableList<T> Tail,
203+
CtorArgs &&...Args) {
204+
return concat(T(std::forward<CtorArgs>(Args)...), Tail);
193205
}
194206

195207
ImmutableList<T> getEmptyList() const {
196208
return ImmutableList<T>(nullptr);
197209
}
198210

199-
ImmutableList<T> create(const T& X) {
200-
return concat(X, getEmptyList());
211+
template <typename ElemT>
212+
ImmutableList<T> create(ElemT &&Data) {
213+
return concat(std::forward<ElemT>(Data), getEmptyList());
201214
}
202215
};
203216

unittests/ADT/ImmutableListTest.cpp

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,49 @@ TEST_F(ImmutableListTest, MultiElemIntListTest) {
150150
EXPECT_TRUE(L5.isEqual(L5));
151151
}
152152

153+
template <typename Fundamental>
154+
struct ExplicitCtorWrapper : public Wrapper<Fundamental> {
155+
explicit ExplicitCtorWrapper(Fundamental F) : Wrapper<Fundamental>(F) {}
156+
ExplicitCtorWrapper(const ExplicitCtorWrapper &) = delete;
157+
ExplicitCtorWrapper(ExplicitCtorWrapper &&) = default;
158+
ExplicitCtorWrapper &operator=(const ExplicitCtorWrapper &) = delete;
159+
ExplicitCtorWrapper &operator=(ExplicitCtorWrapper &&) = default;
160+
};
161+
162+
TEST_F(ImmutableListTest, EmplaceIntListTest) {
163+
ImmutableList<ExplicitCtorWrapper<int>>::Factory f;
164+
165+
ImmutableList<ExplicitCtorWrapper<int>> L = f.getEmptyList();
166+
ImmutableList<ExplicitCtorWrapper<int>> L2 = f.emplace(L, 3);
167+
168+
ImmutableList<ExplicitCtorWrapper<int>> L3 =
169+
f.add(ExplicitCtorWrapper<int>(2), L2);
170+
171+
ImmutableList<ExplicitCtorWrapper<int>> L4 =
172+
f.emplace(L3, ExplicitCtorWrapper<int>(1));
173+
174+
ImmutableList<ExplicitCtorWrapper<int>> L5 =
175+
f.add(ExplicitCtorWrapper<int>(1), L3);
176+
177+
EXPECT_FALSE(L2.isEmpty());
178+
EXPECT_TRUE(L2.getTail().isEmpty());
179+
EXPECT_EQ(3, L2.getHead());
180+
EXPECT_TRUE(L.isEqual(L2.getTail()));
181+
EXPECT_TRUE(L2.getTail().isEqual(L));
182+
183+
EXPECT_FALSE(L3.isEmpty());
184+
EXPECT_FALSE(L2 == L3);
185+
EXPECT_EQ(2, L3.getHead());
186+
EXPECT_TRUE(L2 == L3.getTail());
187+
188+
EXPECT_FALSE(L4.isEmpty());
189+
EXPECT_EQ(1, L4.getHead());
190+
EXPECT_TRUE(L3 == L4.getTail());
191+
192+
EXPECT_TRUE(L4 == L5);
193+
EXPECT_TRUE(L3 == L5.getTail());
194+
}
195+
153196
TEST_F(ImmutableListTest, CharListOrderingTest) {
154197
ImmutableList<Wrapper<char>>::Factory f;
155198
ImmutableList<Wrapper<char>> L = f.getEmptyList();

0 commit comments

Comments
 (0)