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

[Artem Dergachev via Phabricator via llvm-commits]

NoQ 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.
----------------
george.karpenkov wrote:
> 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
> I intentionally left it like this, because one can just `std::move` to it. That does achieve the same thing, right?

Nah, it doesn't achieve the same thing.

Reference is always a reference, passing it into a function is always free-of-charge, i.e. never invokes a constructor. Also `std::move` doesn't invoke move-constructor on its own, it simply marks the object as movable, i.e. move is simply an lvalue-to-xvalue-cast.

But initialization of a value-type argument is a //non-elidable// constructor, which will be copy-constructor if the object is not movable and move-constructor if the object is movable. So if you omit `&&` for `Head`, you are losing the benefits of a free-of-charge pass-by-reference and instead have to deal with an actual constructor call.


https://reviews.llvm.org/D49985