February 19th, 2020

If you plan on keeping the parameter anyway, then there’s no need to have separate T const& and T&& overloads

Suppose you have a function that takes a std::vector and it wants to take ownership of the vector. You might decide to write two overloads:

class Widget
{
public:
  void SetValue(std::vector<int> const& value)
  {
    m_values = values;
  }

  void SetValue(std::vector<int>&& value)
  {
    m_values = std::move(values);
  }
private:
  std::vector<int> m_values;
};

If the caller passes a read-only lvalue reference, then you have to copy the vector. But if the caller passes an rvalue reference, then you can steal the vector by moving it into the m_values.

But really, the extra overload is just duplicated code for no real purpose. You’re going to keep the object either way, so just make the caller provide it in a form you can keep: Pass it by value.

class Widget
{
public:
  void SetValue(std::vector<int> value)
  {
    m_values = std::move(values);
  }

private:
  std::vector<int> m_values;
};

If the caller tries to pass an lvalue reference, then the compiler will use the copy constructor to create the inbound value parameter, which you then move into m_values.

If the caller tries to pass an rvalue reference, then the compiler will move it into the inbound value parameter, which you then move onward into m_values.

You get the same result without needing any overloads. Lvalues are copied and rvalues are moved.¹

Related reading: Rvalue references in constructor: when less is more.

¹ Now, there is a bit of extra cost associated with this simplification: The copying or moving into the parameter is done at the call site, and the formal parameter now needs to be destructed. But you avoid having to write two versions of every function, and I think that simplification is worth it. (Besides, the compiler can often optimize the move.)

Topics
Code

Author

Raymond has been involved in the evolution of Windows for more than 30 years. In 2003, he began a Web site known as The Old New Thing which has grown in popularity far beyond his wildest imagination, a development which still gives him the heebie-jeebies. The Web site spawned a book, coincidentally also titled The Old New Thing (Addison Wesley 2007). He occasionally appears on the Windows Dev Docs Twitter account to tell stories which convey no useful information.

12 comments

Discussion is closed. Login to edit/delete existing comments.

    • Reuven Abliyev

      According to the Herb’s video above,
      Raymond’s blog is about “Option #3: Pass by value for Constructors”

      And in this video about at 1:26:24 Herb says:”For constructor do consider option three”

      • Shawn Mueller

        Yes, but it is ONLY for constructors, not for “setters” like the article author suggests.

      • Reuven Abliyev

        1) Author wrote article about some programming technique.
        2) You state that author is wrong and as evidence you provided link to the video
        3) Speaker in the video confirms that this programming technique is certainly reasonable in some cases
        4) Now you say that video is not related to the article at all
        5) So why it was used in the statement (2) at the first place?

    • Tim Weis

      You forgot to include the credibility indicator, something along the lines of: “According to someone on the internet, …”. Incidentally, according to someone else on the internet, this is perfectly reasonable (and even solves a very subtle issue).

      • Shawn Mueller

        Herb Sutter is pretty damn credible.

  • Ismo Salonen

    For vectors this might be good thing, depending on std:.vector implementation. But for strings with small buffer optimizations the small strings are copied once more than when having two overloads. Depending how time sensitive your application is this might be bad thing. Personally I’ll write both overloads usually, not always.

  • Phil Deets

    One thing to be aware of here is that this prevents buffer reuse for the buffer within m_values. The move will always end up taking the buffer from the parameter and freeing the existing buffer in m_values, rather than copying the ints over to the existing buffer.

    • Em X

      That’s just an implementation detail, not part of the contract. The contract here is that SetValue takes ownership of the passed-in value, and the post shows that there’s a perfectly correct way to do so without needing two different functions. If you want to copy the information into your existing buffer, you’re free to do so, instead of just taking the new one.

      • Phil Deets

        My point is that if the caller passes you an lvalue, you will end up having one additional allocation over what would happen if you used vector's assignment operator for an lvalue reference. With this:

        void SetValue(std::vector const& value)
        {
        m_values = values;
        }

        The implementation of vector will internally copy the ints if m_values has a buffer which is big enough to accommodate the values from...

        Read more
      • Em X

        Sorry, I didn't mean that the ABI was the contract, just the API (and perhaps not even then) if you can change arguments. Where performance is necessary, simplicity and clarity often have to be sacrificed.

        If the ABI is a contract, then you have a lot more up-front decisions to make about where performance is necessary, or just shoot for performance in all cases even if it's a bit more complex to write and maintain. Or...

        Read more
  • David Lowndes

    value/values typos?