- Revision
- 247286
- Author
- wei...@apple.com
- Date
- 2019-07-09 16:54:07 -0700 (Tue, 09 Jul 2019)
Log Message
Add StringBuilder member function which allows makeString() style variadic argument construction
https://bugs.webkit.org/show_bug.cgi?id=198997
Reviewed by Darin Adler.
Source/WTF:
Adds new StringBuilder::flexibleAppend(...) member function which allows passing one or more
string-adaptable (in the sense that there is StringTypeAdapter specialization for the
type) parameters. This re-ususes the variadic template infrastructure in StringConcatenate.h
that is used for makeString(...) allowing for improvements in one to benefit the other.
The advantage of StringBuilder::flexibleAppend(...) over calling StringBuilder::append(...)
multiple times (beyond the code sharing with makeString(...) is that it can avoid unnecessary
additional re-allocations when the StringBuilder needs to expand it's capacity. It does this
by computing the complete required length for all the passed arguments and then ensuring enough
capacity is available. It also reduces the allocation overhead versus the anti-pattern of
builder.append(makeString(...)).
Ideally, this member function should eventually just be called StringBuilder::append(...), but
the current overload set for StringBuilder::append(...) makes this complicated due to overloads
that take two arguments such as StringBuilder::append(const UChar*, unsigned). Going forward, we
should rename or remove those overloads and move to a standard interface.
* wtf/posix/FileSystemPOSIX.cpp:
(WTF::FileSystemImpl::pathByAppendingComponents):
Adopt StringBuilder::flexibleAppend, using to combine the append of '/' and component.
* wtf/text/StringBuilder.cpp:
(WTF::StringBuilder::appendUninitialized):
(WTF::StringBuilder::appendUninitializedWithoutOverflowCheck):
Extract the part of appendUnitialized that doesn't do the overflow check
into it's own member function to allow callers that have already done the
overflow check to bypass it.
(WTF::StringBuilder::appendUninitializedWithoutOverflowCheckForUChar):
(WTF::StringBuilder::appendUninitializedWithoutOverflowCheckForLChar):
Added to allow template member function flexibleAppendFromAdapters to call
appendUninitializedWithoutOverflowCheck without moving it to the header.
* wtf/text/StringBuilder.h:
(WTF::StringBuilder::flexibleAppendFromAdapters):
(WTF::StringBuilder::flexibleAppend):
Modeled on tryMakeStringFromAdapters in StringConcatenate.h, these
eagerly compute the required length, expand the buffer and then use
the existing string type adaptor accumulation functions used by makeString.
* wtf/text/StringConcatenate.h:
(WTF::stringTypeAdapterAccumulator):
(WTF::tryMakeStringFromAdapters):
(WTF::makeStringAccumulator): Deleted.
Renames makeStringAccumulator to stringTypeAdapterAccumulator now that it is used
by more than just makeString.
Tools:
* TestWebKitAPI/Tests/WTF/StringBuilder.cpp:
Add basic test showing that StringBuilder::flexibleAppend can be used to
append one or more string adaptable types.
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (247285 => 247286)
--- trunk/Source/WTF/ChangeLog 2019-07-09 23:20:53 UTC (rev 247285)
+++ trunk/Source/WTF/ChangeLog 2019-07-09 23:54:07 UTC (rev 247286)
@@ -1,3 +1,57 @@
+2019-07-09 Sam Weinig <wei...@apple.com>
+
+ Add StringBuilder member function which allows makeString() style variadic argument construction
+ https://bugs.webkit.org/show_bug.cgi?id=198997
+
+ Reviewed by Darin Adler.
+
+ Adds new StringBuilder::flexibleAppend(...) member function which allows passing one or more
+ string-adaptable (in the sense that there is StringTypeAdapter specialization for the
+ type) parameters. This re-ususes the variadic template infrastructure in StringConcatenate.h
+ that is used for makeString(...) allowing for improvements in one to benefit the other.
+
+ The advantage of StringBuilder::flexibleAppend(...) over calling StringBuilder::append(...)
+ multiple times (beyond the code sharing with makeString(...) is that it can avoid unnecessary
+ additional re-allocations when the StringBuilder needs to expand it's capacity. It does this
+ by computing the complete required length for all the passed arguments and then ensuring enough
+ capacity is available. It also reduces the allocation overhead versus the anti-pattern of
+ builder.append(makeString(...)).
+
+ Ideally, this member function should eventually just be called StringBuilder::append(...), but
+ the current overload set for StringBuilder::append(...) makes this complicated due to overloads
+ that take two arguments such as StringBuilder::append(const UChar*, unsigned). Going forward, we
+ should rename or remove those overloads and move to a standard interface.
+
+ * wtf/posix/FileSystemPOSIX.cpp:
+ (WTF::FileSystemImpl::pathByAppendingComponents):
+ Adopt StringBuilder::flexibleAppend, using to combine the append of '/' and component.
+
+ * wtf/text/StringBuilder.cpp:
+ (WTF::StringBuilder::appendUninitialized):
+ (WTF::StringBuilder::appendUninitializedWithoutOverflowCheck):
+ Extract the part of appendUnitialized that doesn't do the overflow check
+ into it's own member function to allow callers that have already done the
+ overflow check to bypass it.
+
+ (WTF::StringBuilder::appendUninitializedWithoutOverflowCheckForUChar):
+ (WTF::StringBuilder::appendUninitializedWithoutOverflowCheckForLChar):
+ Added to allow template member function flexibleAppendFromAdapters to call
+ appendUninitializedWithoutOverflowCheck without moving it to the header.
+
+ * wtf/text/StringBuilder.h:
+ (WTF::StringBuilder::flexibleAppendFromAdapters):
+ (WTF::StringBuilder::flexibleAppend):
+ Modeled on tryMakeStringFromAdapters in StringConcatenate.h, these
+ eagerly compute the required length, expand the buffer and then use
+ the existing string type adaptor accumulation functions used by makeString.
+
+ * wtf/text/StringConcatenate.h:
+ (WTF::stringTypeAdapterAccumulator):
+ (WTF::tryMakeStringFromAdapters):
+ (WTF::makeStringAccumulator): Deleted.
+ Renames makeStringAccumulator to stringTypeAdapterAccumulator now that it is used
+ by more than just makeString.
+
2019-07-09 Alex Christensen <achristen...@webkit.org>
When parsing an IPv4 address, wait until after deciding it is indeed an IPv4 address before reporting syntax violations
Modified: trunk/Source/WTF/wtf/posix/FileSystemPOSIX.cpp (247285 => 247286)
--- trunk/Source/WTF/wtf/posix/FileSystemPOSIX.cpp 2019-07-09 23:20:53 UTC (rev 247285)
+++ trunk/Source/WTF/wtf/posix/FileSystemPOSIX.cpp 2019-07-09 23:54:07 UTC (rev 247286)
@@ -301,10 +301,8 @@
{
StringBuilder builder;
builder.append(path);
- for (auto& component : components) {
- builder.append('/');
- builder.append(component);
- }
+ for (auto& component : components)
+ builder.flexibleAppend('/', component);
return builder.toString();
}
Modified: trunk/Source/WTF/wtf/text/StringBuilder.cpp (247285 => 247286)
--- trunk/Source/WTF/wtf/text/StringBuilder.cpp 2019-07-09 23:20:53 UTC (rev 247285)
+++ trunk/Source/WTF/wtf/text/StringBuilder.cpp 2019-07-09 23:54:07 UTC (rev 247286)
@@ -163,7 +163,7 @@
ASSERT(m_buffer->length() == requiredLength);
}
-template <>
+template<>
void StringBuilder::reallocateBuffer<LChar>(unsigned requiredLength)
{
// If the buffer has only one ref (by this StringBuilder), reallocate it,
@@ -183,7 +183,7 @@
ASSERT(hasOverflowed() || m_buffer->length() == requiredLength);
}
-template <>
+template<>
void StringBuilder::reallocateBuffer<UChar>(unsigned requiredLength)
{
// If the buffer has only one ref (by this StringBuilder), reallocate it,
@@ -231,21 +231,29 @@
ASSERT(hasOverflowed() || !newCapacity || m_buffer->length() >= newCapacity);
}
-// Make 'length' additional capacity be available in m_buffer, update m_string & m_length,
+// Make 'additionalLength' additional capacity be available in m_buffer, update m_string & m_length,
// return a pointer to the newly allocated storage.
// Returns nullptr if the size of the new builder would have overflowed
-template <typename CharType>
-ALWAYS_INLINE CharType* StringBuilder::appendUninitialized(unsigned length)
+template<typename CharacterType>
+ALWAYS_INLINE CharacterType* StringBuilder::appendUninitialized(unsigned additionalLength)
{
- ASSERT(length);
+ ASSERT(additionalLength);
// Calculate the new size of the builder after appending.
- CheckedInt32 requiredLength = m_length + length;
+ CheckedInt32 requiredLength = m_length + additionalLength;
if (requiredLength.hasOverflowed()) {
didOverflow();
return nullptr;
}
+ return appendUninitializedWithoutOverflowCheck<CharacterType>(requiredLength);
+}
+
+template<typename CharacterType>
+ALWAYS_INLINE CharacterType* StringBuilder::appendUninitializedWithoutOverflowCheck(CheckedInt32 requiredLength)
+{
+ ASSERT(!requiredLength.hasOverflowed());
+
if (m_buffer && (requiredLength.unsafeGet<unsigned>() <= m_buffer->length())) {
// If the buffer is valid it must be at least as long as the current builder contents!
ASSERT(m_buffer->length() >= m_length.unsafeGet<unsigned>());
@@ -252,16 +260,26 @@
unsigned currentLength = m_length.unsafeGet();
m_string = String();
m_length = requiredLength;
- return getBufferCharacters<CharType>() + currentLength;
+ return getBufferCharacters<CharacterType>() + currentLength;
}
- return appendUninitializedSlow<CharType>(requiredLength.unsafeGet());
+ return appendUninitializedSlow<CharacterType>(requiredLength.unsafeGet());
}
-// Make 'length' additional capacity be available in m_buffer, update m_string & m_length,
+UChar* StringBuilder::appendUninitializedWithoutOverflowCheckForUChar(CheckedInt32 requiredLength)
+{
+ return appendUninitializedWithoutOverflowCheck<UChar>(requiredLength);
+}
+
+LChar* StringBuilder::appendUninitializedWithoutOverflowCheckForLChar(CheckedInt32 requiredLength)
+{
+ return appendUninitializedWithoutOverflowCheck<LChar>(requiredLength);
+}
+
+// Make 'requiredLength' capacity be available in m_buffer, update m_string & m_length,
// return a pointer to the newly allocated storage.
-template <typename CharType>
-CharType* StringBuilder::appendUninitializedSlow(unsigned requiredLength)
+template<typename CharacterType>
+CharacterType* StringBuilder::appendUninitializedSlow(unsigned requiredLength)
{
ASSERT(!hasOverflowed());
ASSERT(requiredLength);
@@ -270,15 +288,15 @@
// If the buffer is valid it must be at least as long as the current builder contents!
ASSERT(m_buffer->length() >= m_length.unsafeGet<unsigned>());
- reallocateBuffer<CharType>(expandedCapacity(capacity(), requiredLength));
+ reallocateBuffer<CharacterType>(expandedCapacity(capacity(), requiredLength));
} else {
ASSERT(m_string.length() == m_length.unsafeGet<unsigned>());
- allocateBuffer(m_length ? m_string.characters<CharType>() : nullptr, expandedCapacity(capacity(), requiredLength));
+ allocateBuffer(m_length ? m_string.characters<CharacterType>() : nullptr, expandedCapacity(capacity(), requiredLength));
}
if (UNLIKELY(hasOverflowed()))
return nullptr;
- CharType* result = getBufferCharacters<CharType>() + m_length.unsafeGet();
+ CharacterType* result = getBufferCharacters<CharacterType>() + m_length.unsafeGet();
m_length = requiredLength;
ASSERT(!hasOverflowed());
ASSERT(m_buffer->length() >= m_length.unsafeGet<unsigned>());
Modified: trunk/Source/WTF/wtf/text/StringBuilder.h (247285 => 247286)
--- trunk/Source/WTF/wtf/text/StringBuilder.h 2019-07-09 23:20:53 UTC (rev 247285)
+++ trunk/Source/WTF/wtf/text/StringBuilder.h 2019-07-09 23:54:07 UTC (rev 247286)
@@ -231,6 +231,9 @@
WTF_EXPORT_PRIVATE void appendFixedWidthNumber(float, unsigned decimalPlaces);
WTF_EXPORT_PRIVATE void appendFixedWidthNumber(double, unsigned decimalPlaces);
+ // FIXME: Rename to append(...) after renaming any overloads of append that take more than one argument.
+ template<typename... StringTypes> void flexibleAppend(StringTypes...);
+
String toString()
{
if (!m_string.isNull()) {
@@ -350,16 +353,19 @@
void allocateBuffer(const LChar* currentCharacters, unsigned requiredLength);
void allocateBuffer(const UChar* currentCharacters, unsigned requiredLength);
void allocateBufferUpConvert(const LChar* currentCharacters, unsigned requiredLength);
- template <typename CharType>
- void reallocateBuffer(unsigned requiredLength);
- template <typename CharType>
- ALWAYS_INLINE CharType* appendUninitialized(unsigned length);
- template <typename CharType>
- CharType* appendUninitializedSlow(unsigned length);
- template <typename CharType>
- ALWAYS_INLINE CharType * getBufferCharacters();
+ template<typename CharacterType> void reallocateBuffer(unsigned requiredLength);
+ template<typename CharacterType> ALWAYS_INLINE CharacterType* appendUninitialized(unsigned additionalLength);
+ template<typename CharacterType> ALWAYS_INLINE CharacterType* appendUninitializedWithoutOverflowCheck(CheckedInt32 requiredLength);
+ template<typename CharacterType> CharacterType* appendUninitializedSlow(unsigned requiredLength);
+
+ WTF_EXPORT_PRIVATE UChar* appendUninitializedWithoutOverflowCheckForUChar(CheckedInt32 requiredLength);
+ WTF_EXPORT_PRIVATE LChar* appendUninitializedWithoutOverflowCheckForLChar(CheckedInt32 requiredLength);
+
+ template<typename CharacterType> ALWAYS_INLINE CharacterType* getBufferCharacters();
WTF_EXPORT_PRIVATE void reifyString() const;
+ template<typename... StringTypeAdapters> void flexibleAppendFromAdapters(StringTypeAdapters...);
+
mutable String m_string;
RefPtr<StringImpl> m_buffer;
union {
@@ -374,7 +380,7 @@
#endif
};
-template <>
+template<>
ALWAYS_INLINE LChar* StringBuilder::getBufferCharacters<LChar>()
{
ASSERT(m_is8Bit);
@@ -381,7 +387,7 @@
return m_bufferCharacters8;
}
-template <>
+template<>
ALWAYS_INLINE UChar* StringBuilder::getBufferCharacters<UChar>()
{
ASSERT(!m_is8Bit);
@@ -388,9 +394,41 @@
return m_bufferCharacters16;
}
-template <typename CharType>
-bool equal(const StringBuilder& s, const CharType* buffer, unsigned length)
+template<typename... StringTypeAdapters>
+void StringBuilder::flexibleAppendFromAdapters(StringTypeAdapters... adapters)
{
+ auto requiredLength = checkedSum<int32_t>(m_length, adapters.length()...);
+ if (requiredLength.hasOverflowed()) {
+ didOverflow();
+ return;
+ }
+
+ if (m_is8Bit && are8Bit(adapters...)) {
+ LChar* dest = appendUninitializedWithoutOverflowCheckForLChar(requiredLength);
+ if (!dest) {
+ ASSERT(hasOverflowed());
+ return;
+ }
+ stringTypeAdapterAccumulator(dest, adapters...);
+ } else {
+ UChar* dest = appendUninitializedWithoutOverflowCheckForUChar(requiredLength);
+ if (!dest) {
+ ASSERT(hasOverflowed());
+ return;
+ }
+ stringTypeAdapterAccumulator(dest, adapters...);
+ }
+}
+
+template<typename... StringTypes>
+void StringBuilder::flexibleAppend(StringTypes... strings)
+{
+ flexibleAppendFromAdapters(StringTypeAdapter<StringTypes>(strings)...);
+}
+
+template<typename CharacterType>
+bool equal(const StringBuilder& s, const CharacterType* buffer, unsigned length)
+{
if (s.length() != length)
return false;
@@ -400,7 +438,7 @@
return equal(s.characters16(), buffer, length);
}
-template <typename StringType>
+template<typename StringType>
bool equal(const StringBuilder& a, const StringType& b)
{
if (a.length() != b.length())
Modified: trunk/Source/WTF/wtf/text/StringConcatenate.h (247285 => 247286)
--- trunk/Source/WTF/wtf/text/StringConcatenate.h 2019-07-09 23:20:53 UTC (rev 247285)
+++ trunk/Source/WTF/wtf/text/StringConcatenate.h 2019-07-09 23:54:07 UTC (rev 247286)
@@ -248,16 +248,16 @@
}
template<typename ResultType, typename Adapter>
-inline void makeStringAccumulator(ResultType* result, Adapter adapter)
+inline void stringTypeAdapterAccumulator(ResultType* result, Adapter adapter)
{
adapter.writeTo(result);
}
template<typename ResultType, typename Adapter, typename... Adapters>
-inline void makeStringAccumulator(ResultType* result, Adapter adapter, Adapters ...adapters)
+inline void stringTypeAdapterAccumulator(ResultType* result, Adapter adapter, Adapters ...adapters)
{
adapter.writeTo(result);
- makeStringAccumulator(result + adapter.length(), adapters...);
+ stringTypeAdapterAccumulator(result + adapter.length(), adapters...);
}
template<typename StringTypeAdapter, typename... StringTypeAdapters>
@@ -276,7 +276,7 @@
if (!resultImpl)
return String();
- makeStringAccumulator(buffer, adapter, adapters...);
+ stringTypeAdapterAccumulator(buffer, adapter, adapters...);
return resultImpl;
}
@@ -286,7 +286,7 @@
if (!resultImpl)
return String();
- makeStringAccumulator(buffer, adapter, adapters...);
+ stringTypeAdapterAccumulator(buffer, adapter, adapters...);
return resultImpl;
}
Modified: trunk/Tools/ChangeLog (247285 => 247286)
--- trunk/Tools/ChangeLog 2019-07-09 23:20:53 UTC (rev 247285)
+++ trunk/Tools/ChangeLog 2019-07-09 23:54:07 UTC (rev 247286)
@@ -1,3 +1,14 @@
+2019-07-09 Sam Weinig <wei...@apple.com>
+
+ Add StringBuilder member function which allows makeString() style variadic argument construction
+ https://bugs.webkit.org/show_bug.cgi?id=198997
+
+ Reviewed by Darin Adler.
+
+ * TestWebKitAPI/Tests/WTF/StringBuilder.cpp:
+ Add basic test showing that StringBuilder::flexibleAppend can be used to
+ append one or more string adaptable types.
+
2019-07-09 Sihui Liu <sihui_...@apple.com>
Only allow fetching and removing session credentials from WebsiteDataStore
Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/StringBuilder.cpp (247285 => 247286)
--- trunk/Tools/TestWebKitAPI/Tests/WTF/StringBuilder.cpp 2019-07-09 23:20:53 UTC (rev 247285)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/StringBuilder.cpp 2019-07-09 23:54:07 UTC (rev 247286)
@@ -116,6 +116,29 @@
}
}
+TEST(StringBuilderTest, FlexibleAppend)
+{
+ {
+ StringBuilder builder;
+ builder.flexibleAppend(String("0123456789"));
+ expectBuilderContent("0123456789", builder);
+ builder.flexibleAppend("abcd");
+ expectBuilderContent("0123456789abcd", builder);
+ builder.flexibleAppend('e');
+ expectBuilderContent("0123456789abcde", builder);
+ builder.flexibleAppend("");
+ expectBuilderContent("0123456789abcde", builder);
+ }
+
+ {
+ StringBuilder builder;
+ builder.flexibleAppend(String("0123456789"), "abcd", 'e', "");
+ expectBuilderContent("0123456789abcde", builder);
+ builder.flexibleAppend(String("A"), "B", 'C', "");
+ expectBuilderContent("0123456789abcdeABC", builder);
+ }
+}
+
TEST(StringBuilderTest, ToString)
{
StringBuilder builder;
