Cleanup: Initialize GURL by moving std::string

url/gurl.h

Index: url/gurl.h
diff --git a/url/gurl.h b/url/gurl.h
index cdff0fd146bb814105602ea4f4dd16439c842129..dae87ff630ab243fe8f37d8246074836a850b96c 100644
--- a/url/gurl.h
+++ b/url/gurl.h
@@ -46,13 +46,17 @@ class URL_EXPORT GURL {
        size_t canonical_spec_len,
        const url::Parsed& parsed,
        bool is_valid);
-  // Notice that we take the canonical_spec by value so that we can convert
-  // from WebURL without copying the string. When we call this constructor
-  // we pass in a temporary std::string, which lets the compiler skip the
-  // copy and just move the std::string into the function argument. In the
-  // implementation, we use swap to move the data into the GURL itself,
-  // which means we end up with zero copies.
-  GURL(std::string canonical_spec, const url::Parsed& parsed, bool is_valid);
+
+  // Notice that we can take the canonical_spec by rvalue reference so that we
+  // can convert from WebURL without copying the string. When we call this

[ki.stfu, 2015/09/21 05:55:55]

(the original message is here:
https://codereview.chromium.org/1349783006/diff/20001/url/gurl.h#newcode51)

Please take into account the 2nd version which uses movable arguments. In this
case, compiler moves an object and there is no copying of the string.
Initially I wanted to do only the version with movable std::string, but the 1st
is required for unittests.

Thus, it does not reduce the performance, but on the contrary, it causes the
compiler to move the object.

+  // constructor we pass in a temporary std::string, which lets the compiler
+  // skip the copy and just move the std::string into the function argument.
+  // The const reference version is used in unittests.
+  GURL(const std::string& canonical_spec,
+       const url::Parsed& parsed,
+       bool is_valid);
+  GURL(std::string&& canonical_spec, const url::Parsed& parsed,

[danakj, 2016/04/29 02:17:54]

According to the google style guide the way to write this is "std::string", and
when you std::move() at the callsite, it will move the contents into this string
here.

Moving a string is pretty cheap, I'd expect that trying to avoid an extra move
here is not buying you much?

Do you have performance numbers justifying this?

[ki.stfu, 2016/05/25 16:53:33]

I can't find a mention about this way in style guide.
Right.
No, I didn't measure it in practice. Just theoretical reasoning.

[danakj, 2016/05/25 19:59:37]

https://google.github.io/styleguide/cppguide.html#Rvalue_references

"Use rvalue references only to define move constructors and move assignment
operators, or for perfect forwarding."

i.e. Don't use it for passing to methods.

[ki.stfu, 2016/05/26 05:15:30]

Doesn't it look like a moving "for perfect forwarding"?

+       bool is_valid) noexcept;
 
   ~GURL();