Skip to content

Commit

Permalink
[ADT] ImmutableList no longer requires elements to be copy constructible
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Szelethus committed Aug 28, 2018
1 parent fa8ce34 commit d020239
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 8 deletions.
29 changes: 21 additions & 8 deletions llvm/include/llvm/ADT/ImmutableList.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <typename ElemT>
ImmutableListImpl(ElemT &&head, const ImmutableListImpl *tail = nullptr)
: Head(std::forward<ElemT>(head)), Tail(tail) {}

public:
ImmutableListImpl(const ImmutableListImpl &) = delete;
Expand Down Expand Up @@ -66,6 +67,9 @@ class ImmutableList {
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;

Expand Down Expand Up @@ -166,7 +170,8 @@ class ImmutableListFactory {
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;
Expand All @@ -179,7 +184,7 @@ class ImmutableListFactory {
// 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);
Expand All @@ -188,16 +193,24 @@ class ImmutableListFactory {
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());
}
};

Expand Down
43 changes: 43 additions & 0 deletions llvm/unittests/ADT/ImmutableListTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,49 @@ TEST_F(ImmutableListTest, MultiElemIntListTest) {
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();
Expand Down

0 comments on commit d020239

Please sign in to comment.