Search code examples
c++c++17move-semanticsstdstring

What is the best way to pass a (temporary?) std::string to a function that uses it to construct an object that takes a copy?


Consider the following code:

struct Foo {
  std::string s;
  Foo(std::string s_) : s(s_) { }
};

Foo* f(std::string s)
{
  return new Foo(s);
}

where f() can be called with an lvalue or rvalue std::string, or with a char const* causing a temporary (but that's the same as an rvalue I resume). For example:

int main()
{
  f("test");
  f(std::string());
  std::string s("test");
  f(std::move(s));
  std::string s2("test");
  f(s2); // This MUST cause one copy.
}

Only in the last case one copy is really needed. In all other cases I'd like that no copy is made at all and the std::string is constructed just once (into the allocated Foo).

Is this possible? If so, how would the signature of f() look like?

EDIT:

Using Tracked from cwds I created a little test program with a string class that prints when it is constructed, moved etc. For the main() function please see this blob.

The program as-is above gives the following output:

NOTICE  : Calling f("test")... <unfinished>
TRACKED :     string0*
TRACKED :     string1*(string0)
TRACKED :     string2*(string1)
TRACKED :     string1~
TRACKED :     string0~
NOTICE  : <continued> done
TRACKED : string2~
NOTICE  : Calling f(string())... <unfinished>
TRACKED :     string3*
TRACKED :     string4*(string3)
TRACKED :     string5*(string4)
TRACKED :     string4~
TRACKED :     string3~
NOTICE  : <continued> done
TRACKED : string5~
NOTICE  : Constructing s("test")... <unfinished>
TRACKED :     string6*
NOTICE  : <continued> done
NOTICE  : Calling f(std::move(s))... <unfinished>
TRACKED :     string6=>string7*
TRACKED :     string8*(string7)
TRACKED :     string9*(string8)
TRACKED :     string8~
TRACKED :     string7~
NOTICE  : <continued> done
TRACKED : string9~
NOTICE  : Constructing s2("test")... <unfinished>
TRACKED :     string10*
NOTICE  : <continued> done
NOTICE  : Calling f(std::move(s))... <unfinished>
TRACKED :     string11*(string10)
TRACKED :     string12*(string11)
TRACKED :     string13*(string12)
TRACKED :     string12~
TRACKED :     string11~
NOTICE  : <continued> done
TRACKED : string13~
NOTICE  : Leaving main()...
TRACKED : string10~
TRACKED : string6~

Which, as expected, shows a lot of copying.

Using max66's first suggestion, see this change I get the following output:

NOTICE  : Calling f("test")... <unfinished>
TRACKED :     string0*
TRACKED :     string0=>string1*
TRACKED :     string1=>string2*
TRACKED :     string1~
TRACKED :     string0~
NOTICE  : <continued> done
TRACKED : string2~
NOTICE  : Calling f(string())... <unfinished>
TRACKED :     string3*
TRACKED :     string3=>string4*
TRACKED :     string4=>string5*
TRACKED :     string4~
TRACKED :     string3~
NOTICE  : <continued> done
TRACKED : string5~
NOTICE  : Constructing s("test")... <unfinished>
TRACKED :     string6*
NOTICE  : <continued> done
NOTICE  : Calling f(std::move(s))... <unfinished>
TRACKED :     string6=>string7*
TRACKED :     string7=>string8*
TRACKED :     string8=>string9*
TRACKED :     string8~
TRACKED :     string7~
NOTICE  : <continued> done
TRACKED : string9~
NOTICE  : Constructing s2("test")... <unfinished>
TRACKED :     string10*
NOTICE  : <continued> done
NOTICE  : Calling f(std::move(s))... <unfinished>
TRACKED :     string11*(string10)
TRACKED :     string11=>string12*
TRACKED :     string12=>string13*
TRACKED :     string12~
TRACKED :     string11~
NOTICE  : <continued> done
TRACKED : string13~
NOTICE  : Leaving main()...
TRACKED : string10~
TRACKED : string6~

Which is perfect in terms of copies being made! There is a lot of moving though, which makes me think I might as well just use the good old std::string const& (that, if I was using it should be replaced by std::string_view I understand-- which I shouldn't be using here since in the end I DO take ownership of the string; as per this answer).

Using max66's second suggestion, although I left out the template because I don't think that is needed here? See the different in this commit, the output becomes:

NOTICE  : Calling f("test")... <unfinished>
TRACKED :     string0*
TRACKED :     string0=>string1*
TRACKED :     string0~
NOTICE  : <continued> done
TRACKED : string1~
NOTICE  : Calling f(string())... <unfinished>
TRACKED :     string2*
TRACKED :     string2=>string3*
TRACKED :     string2~
NOTICE  : <continued> done
TRACKED : string3~
NOTICE  : Constructing s("test")... <unfinished>
TRACKED :     string4*
NOTICE  : <continued> done
NOTICE  : Calling f(std::move(s))... <unfinished>
TRACKED :     string4=>string5*
NOTICE  : <continued> done
TRACKED : string5~
NOTICE  : Leaving main()...
TRACKED : string4~

Which it pretty damn close to ideal if it wasn't for the fact that I had to comment out the passing of the s2 lvalue because with that I get the error:

error: cannot bind rvalue reference of type ‘string&&’ to lvalue of type ‘string’

Is there a way to fix this, so I get the ideal output and still have it work when I try to pass an lvalue? Note that if I add an overload for f() that takes a string then the first two calls become ambiguous :(.


Solution

  • You tagged C++17, so you can use move semantics (from C++11) and you use it in main().

    It seems to me that the signature, that receive a copy, is OK.

    But you have to use std::move() inside f()

    Foo * f (std::string s)
     { return new Foo{std::move(s)}; }
    // ...............^^^^^^^^^
    

    and inside the Foo() constructor

    Foo (std::string s_) : s{std::move(s_)} { }
    // ......................^^^^^^^^^
    

    or unnecessary copies are made.

    An alternative is use template types, universal reference and std::forward.

    I mean

    struct Foo
     {
       std::string s;
    
       template <typename S>
       Foo (S && s_) : s{std::forward<std::string>(s_)} { }
     };
    
    template <typename S>
    Foo * f (S && s)
     { return new Foo{std::forward<S>(s)}; }