Added StringPiece overloads for base::JoinString.

base/strings/string_util.h

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
 // This file defines utility functions for working with strings.
 
 #ifndef BASE_STRINGS_STRING_UTIL_H_
 #define BASE_STRINGS_STRING_UTIL_H_
 
 #include <ctype.h>
@@ skipping 411 matching lines @@
 // immediately write over this memory, but there is no other way to set the size
 // of the string, and not doing that will mean people who access |str| rather
 // than str.c_str() will get back a string of whatever size |str| had on entry
 // to this function (probably 0).
 BASE_EXPORT char* WriteInto(std::string* str, size_t length_with_null);
 BASE_EXPORT char16* WriteInto(string16* str, size_t length_with_null);
 #ifndef OS_WIN
 BASE_EXPORT wchar_t* WriteInto(std::wstring* str, size_t length_with_null);
 #endif
 
-// Does the opposite of SplitString().
+// Does the opposite of SplitString()/SplitStringPiece(). Joins a vector or list
+// of strings into a single string, inserting |separator| (which may be empty)
+// in between all elements.
+//
+// Prefer the StringPiece variant if possible, to avoid unnecessary copying.

[dcheng, 2017/02/22 07:22:18]

I don't think it prevents copies though? It all depends on what you already have
as your input type: if you already have std::string objects, converting to
StringPiece won't help.

[danakj, 2017/02/22 15:10:46]

Right, the setup code to make the vector may have less copies, but that's not
related to this API per se.

[Matt Giuca, 2017/02/23 04:48:06]

Right, JoinString doesn't create any more/less copies based on which overload.

The point I'm trying to get across is that the caller should be structured to
take advantage of StringPiece where possible, so you avoid copying in the caller
before calling JoinString. I rewrote this to be a bit clearer about that, but
it's also quite verbose. Do you think I should say this? Happy to just delete
the paragraph also.

 BASE_EXPORT std::string JoinString(const std::vector<std::string>& parts,
                                    StringPiece separator);
 BASE_EXPORT string16 JoinString(const std::vector<string16>& parts,
                                 StringPiece16 separator);
+BASE_EXPORT std::string JoinString(const std::vector<StringPiece>& parts,
+                                   StringPiece separator);
+BASE_EXPORT string16 JoinString(const std::vector<StringPiece16>& parts,
+                                StringPiece16 separator);
+// Explicit initializer_list overloads are required to break ambiguity when used
+// with a literal initializer list (otherwise the compiler would not be able to
+// decide between the two other overloads).
+BASE_EXPORT std::string JoinString(
+    const std::initializer_list<StringPiece>& parts,

[dcheng, 2017/02/22 07:22:18]

FWIW, it seems much more common to pass by value (as this is just a pointer +
size object, like StringPiece)

[Matt Giuca, 2017/02/23 04:48:06]

Done.

+    StringPiece separator);
+BASE_EXPORT string16
+JoinString(const std::initializer_list<StringPiece16>& parts,
+           StringPiece16 separator);
 
 // Replace $1-$2-$3..$9 in the format string with values from |subst|.
 // Additionally, any number of consecutive '$' characters is replaced by that
 // number less one. Eg $$->$, $$$->$$, etc. The offsets parameter here can be
 // NULL. This only allows you to use up to nine replacements.
 BASE_EXPORT string16 ReplaceStringPlaceholders(
     const string16& format_string,
     const std::vector<string16>& subst,
     std::vector<size_t>* offsets);
 
@@ skipping 11 matching lines @@
 
 #if defined(OS_WIN)
 #include "base/strings/string_util_win.h"
 #elif defined(OS_POSIX)
 #include "base/strings/string_util_posix.h"
 #else
 #error Define string operations appropriately for your platform
 #endif
 
 #endif  // BASE_STRINGS_STRING_UTIL_H_