[PATCH] D49985: [ADT] ImmutableList no longer requires elements to be copy constructible

[Umann Kristóf via Phabricator via llvm-commits]

Szelethus added a comment.

Thanks for the quick and well detailed replies!

In https://reviews.llvm.org/D49985#1189582, @george.karpenkov wrote:

> > requires it's elements to be copy constructible for no good reason
> >  It seems like ImmutableList doesn't run the destructor for std::unique_ptrs
>
> Seems there was a reason: and ImmutableList and its members are assumed to be a POD (plain old datatype), which do not need destructors.
>
> E.g. see the code at the bottom of ImmutableList.h:
>
>   template <typename T> struct isPodLike;
>   template <typename T>
>   struct isPodLike<ImmutableList<T>> { static const bool value = true; };
>


Oh wow. Thank you so much for pointing this out, I was stuck on this one for days. However, to me, it seems like, `T` doesn't need to be `llvm::isPodLike` type, just `std::trivially_destructible`.

> Would it make more sense to store references in ImmutableList for your use case instead?

Yes, dynamic memory management has a way too great overhead, and since my algorithm relies on recursion, using references should be fine. Great idea!


https://reviews.llvm.org/D49985