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

Raymond Chen

February 19th, 202012 0

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.)

Raymond Chen

Read next

12 comments

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

  • David Lowndes 0

    value/values typos?

  • Phil Deets 0

    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 0

      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 0

        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 the parameter. When you do this:

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

        You are forcing a new buffer to be allocated to create the parameter. That buffer is then moved into m_values and the original buffer within m_values is freed. There will never be buffer reuse for the case when the data is set from an lvalue reference. If SetValue is called frequently, that can be a significant performance regression.

        The second function is perfectly correct, but the performance loss can be much higher than just the loss from the extra move.

        • Em X 0

          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 break un-recompiled callers on version upgrades.

  • Ismo Salonen 0

    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.

    • Tim Weis 0

      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 0

        Herb Sutter is pretty damn credible.

    • Reuven Abliyev 0

      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 0

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

        • Reuven Abliyev 0

          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?

Feedback usabilla icon