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

[Artem Dergachev via Phabricator via llvm-commits]

NoQ added a comment.

I strongly support argument order swap because it's consistent with other immutable data structures.

Not sure if i support turning `add` into some sort of `emplace`. I'd rather just restrict it to a move-constructor by accepting a single rvalue, because it'll make caller code easy to understand. Maybe add another `emplace`-like method if you need it. I think it's an important distinction to make.

In https://reviews.llvm.org/D49985#1180850, @dblaikie wrote:

> Test coverage?
>  ...
>  & does changing these signatures (of concat and add) not break any existing
>  code?


This is used only in clang and there's https://reviews.llvm.org/D49986 and there should be no functional change here.

Not sure why didn't we ever write unit tests for these data structures. But it should more or less work as tested by clang lit tests.


Repository:
  rL LLVM

https://reviews.llvm.org/D49985