A discussion with @sean-parent regarding passing an sink arguments of arbitrary type T by value.

sink.cc

/**
 *   If we know that the sink argument is movable,
 *   then passing by value costs nothing if an rvalue is passed and
 *   only an extra move if an lvalue is passed.
 *   I agree that the extra move is no big deal here since it should be
 *   a constant time operation in most cases.
 *
 *   So I like use cases such as:
 *
 *     class TSink {
 *       public:
 *
 *       TSink(std::string text) : Text(std::move(text)) {}
 *
 *       private:
 *
 *       std::string Text;
 *
 *     };  // TSink
 *
 *   However, if the sink argument is not movable,
 *   then passing by value costs nothing if an rvalue is passed but
 *   an extra __copy__ if an lvalue is passed.
 *   I'm not sure if an extra copy is acceptable?
 *
 *   So my question is, do we still want this pattern for arbitrary Ts which
 *   may not be movable and therefore incur an extra copy rather than an extra move?
 *
 *     // Copies twice if TVal is not movable on the way into the function, and
 *     // a move operation which falls back to copy.
 *     template <typename TVal>
 *     class TSink {
 *       public:
 *
 *       TSink(TVal val) : Val(std::move(val)) {}
 *
 *       private:
 *
 *       TVal Val;
 *
 *     };  // TSink<TVal>
 *
 *   Note. This is what `model` looks like at: https://t.co/l1fnqjaOhD
 **/

#include <iostream>

// Non-movable object.
class TObj {
  public:

  TObj(int x) : X(x) {
    std::cout << "TObj(" << X << ")" << std::endl;
  }

  TObj(const TObj &that) : X(that.X) {
    std::cout << "TObj(const TObj &): " << X << std::endl;
  }

  ~TObj() {
    std::cout << "~TObj(" << X << ")" << std::endl;
  }

  int X;

};  // TObj

template <typename TVal>
class TSinkByValue {
  public:

  TSinkByValue(TVal val) : Val(std::move(val)) {}

  private:

  TVal Val;

};  // TSinkByValue

template <typename TVal>
class TSinkByForward {
  public:

  template <typename TForwardVal>
  TSinkByForward(TForwardVal &&val)
      : Val(std::forward<TForwardVal>(val)) {}

  private:

  TVal Val;

};  // TSinkByForward

int main() {
  std::cout << "Sink by value, Rvalue." << std::endl;
  TSinkByValue<TObj>{TObj{42}};
  std::cout << "Sink by forward, Rvalue." << std::endl;
  TSinkByForward<TObj>{TObj{42}};
  std::cout << "Construct a TObj." << std::endl;
  TObj obj(42);
  std::cout << "Sink by value, Lvalue." << std::endl;
  TSinkByValue<TObj>{obj};
  std::cout << "Sink by forward, Lvalue." << std::endl;
  TSinkByForward<TObj>{obj};
}

My answer would be to static assert that the type is noexcept movable. All types should be. If you must support C++98 types (i.e., you have some library you can't easily modify), and the additional copy is an issue, then you can use type traits and fall back to swap() - under the assumption that the type correctly overloaded swap. There are ways to check for a correct overload (Dave Abrahams wrote such a trait, I don't recall the trick) - but even though I might be convinced to support old code, I'm not convinced there is value in supporting bad old code... See my C++Now 2014 talk about why all types are inherently movable.

Interesting, I was trying to come up with a case where a type should be copyable but not movable but couldn't come up with any myself.

Falling back to swap for non-movable types however seem like it would add an extra requirement that a default constructor is available.

Also, if we have multiple arguments, doesn't it become the combinatorics problem again?
We need to choose between move-construct vs default-construct + swap for each argument.
(with a proper type trait that detects if a type is copyable but not movable, TIL std::is_move_constructible doesn't do that).

Perhaps a forwarding constructor with template parameters constrained with std::is_convertible is preferred for this case?

template <typename TLhs, typename TRhs>
class TSinkByForward {
  public:

  template <typename TForwardLhs,
            typename TForwardRhs,
            typename = std::enable_if_t<
                std::is_convertible<TForwardLhs, TLhs>::value &&
                std::is_convertible<TForwardRhs, TRhs>::value>>
  TSinkByForward(TForwardLhs &&lhs, TForwardRhs &&rhs)
      : Lhs(std::forward<TForwardLhs>(lhs)),
        Rhs(std::forward<TForwardRhs>(rhs)) {}

  TLhs Lhs;

  TRhs Rhs;

};  // TSinkByForward<TLhs, TRhs>

I'd accept that though it is difficult to read and I wouldn't teach it. Rather teach developers to make their types movable and or pay the cost of additional copies.

What do you think about a sink-by-swap pattern?

template <typename TLhs, typename TRhs>
class TSinkBySwap {
  public:

  TSinkBySwap(TLhs lhs, TRhs rhs) {
    using std::swap;
    swap(Lhs, lhs);
    swap(Rhs, rhs);
  }

  private:

  TLhs Lhs;

  TRhs Rhs;

};  // TSinkBySwap

For movable types:

	Rvalue	Lvalue
Forward	Move	Copy
Value	Move	Copy + Move
Swap	Default + Swap	Copy + Default + Swap

For non-movable types:

	Rvalue	Lvalue
Forward	Copy	Copy
Value	Copy	Copy + Copy
Swap	Default + Swap	Copy + Default + Swap

So the moves turn into default construction + swap. Are these comparable in efficiency? I think they are since default construction + swap is effectively move where we leave the moved-from object in a default constructed state rather than specified, but I'm not sure.

What I thought was interesting is the case where non-movable type is being passed by rvalue.
Passing by forward and value still performs a copy in this case. Forwarding copies because there's no available move constructor. Value took the advantage of having the first copy elided by the compiler, but fails in moving it into the member variable due to the absence of the move constructor. Swap however, takes advantage of the first copy elision into the constructor, then performs a default + swap.

While I agree that all new code should have move constructors and share your views on teachability and such. If move =~ default + swap, perhaps this isn't a bad pattern?

It isn't bad - the move library in ASL (adobe source libraries) used to do a similar thing. The one issue with doing it as a general solution is that we (meaning those who follow Elements of Programming) don't require that a default constructed object be swapable - this is a strengthening of those requirements. The requirements of a default constructed object is only that it be partially formed, here you are requiring that you can assign from it. In reality this typically isn't an issue but it feeds into the discussion of efficient basis and what it means for a value to be unspecified. I wrote a blog post on the requirements of move() here http://sean-parent.github.io/2014/05/30/about-move/.

Right, I did consider the fact that it enforces requirements on the types. I thought practically speaking perhaps they are okay. I shall read Elements of Programming in the near future.

I have indeed been following the discussion regarding destructive move and your blog post in specific. In general I share your opinion that we should first focus on utilizing other forms of hardware.

Thanks for the discussion Sean, I really appreciate your feedback.