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

[George Karpenkov via Phabricator via llvm-commits]

george.karpenkov added inline comments.


================
Comment at: include/llvm/ADT/ImmutableList.h:172
 
-  LLVM_NODISCARD ImmutableList<T> concat(const T &Head, ImmutableList<T> Tail) {
+  LLVM_NODISCARD ImmutableList<T> concat(T Head, ImmutableList<T>  Tail) {
     // Profile the new list to see if it already exists in our cache.
----------------
Szelethus wrote:
> george.karpenkov wrote:
> > Shouldn't we have `&&` here as well for Head?
> I intentionally left it like this, because one can just `std::move` to it. That does achieve the same thing, right?
In my understanding, one creates an extra copy, and one does not, but my c++fu is not that strong for templates+references.
I'm pretty sure you want to use universal references here (https://isocpp.org/blog/2012/11/universal-references-in-c11-scott-meyers)

@NoQ might clarify


https://reviews.llvm.org/D49985