Unreviewed, rolling out r247286.

Caused TestWTF.WTF.StringOperators to fail on debug bots

Reverted changeset:

"Add StringBuilder member function which allows makeString()
style variadic argument construction"
https://bugs.webkit.org/show_bug.cgi?id=198997
https://trac.webkit.org/changeset/247286

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247307 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
index 0d29c2e..9bb23cd 100644 (file)
--- a/Source/WTF/ChangeLog
+++ b/Source/WTF/ChangeLog
@@ -1,3 +1,16 @@
+2019-07-10  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r247286.
+
+        Caused TestWTF.WTF.StringOperators to fail on debug bots
+
+        Reverted changeset:
+
+        "Add StringBuilder member function which allows makeString()
+        style variadic argument construction"
+        https://bugs.webkit.org/show_bug.cgi?id=198997
+        https://trac.webkit.org/changeset/247286
+
 2019-07-09  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         [WPE][GTK] GUniqueOutPtr::release should return a raw pointer

diff --git a/Source/WTF/wtf/posix/FileSystemPOSIX.cpp b/Source/WTF/wtf/posix/FileSystemPOSIX.cpp
index c030f9c..ad3c926 100644 (file)
--- a/Source/WTF/wtf/posix/FileSystemPOSIX.cpp
+++ b/Source/WTF/wtf/posix/FileSystemPOSIX.cpp
@@ -301,8 +301,10 @@ String pathByAppendingComponents(StringView path, const Vector<StringView>& comp
 {
     StringBuilder builder;
     builder.append(path);
-    for (auto& component : components)
-        builder.flexibleAppend('/', component);
+    for (auto& component : components) {
+        builder.append('/');
+        builder.append(component);
+    }
     return builder.toString();
 }
 

diff --git a/Source/WTF/wtf/text/StringBuilder.cpp b/Source/WTF/wtf/text/StringBuilder.cpp
index 89e16ef..da1d4a4 100644 (file)
--- a/Source/WTF/wtf/text/StringBuilder.cpp
+++ b/Source/WTF/wtf/text/StringBuilder.cpp
@@ -163,7 +163,7 @@ void StringBuilder::allocateBufferUpConvert(const LChar* currentCharacters, unsi
     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 @@ void StringBuilder::reallocateBuffer<LChar>(unsigned requiredLength)
     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,55 +231,37 @@ void StringBuilder::reserveCapacity(unsigned newCapacity)
     ASSERT(hasOverflowed() || !newCapacity || m_buffer->length() >= newCapacity);
 }
 
-// Make 'additionalLength' additional capacity be available in m_buffer, update m_string & m_length,
+// Make 'length' 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 CharacterType>
-ALWAYS_INLINE CharacterType* StringBuilder::appendUninitialized(unsigned additionalLength)
+template <typename CharType>
+ALWAYS_INLINE CharType* StringBuilder::appendUninitialized(unsigned length)
 {
-    ASSERT(additionalLength);
+    ASSERT(length);
 
     // Calculate the new size of the builder after appending.
-    CheckedInt32 requiredLength = m_length + additionalLength;
+    CheckedInt32 requiredLength = m_length + length;
     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>());
         unsigned currentLength = m_length.unsafeGet();
         m_string = String();
         m_length = requiredLength;
-        return getBufferCharacters<CharacterType>() + currentLength;
+        return getBufferCharacters<CharType>() + currentLength;
     }
     
-    return appendUninitializedSlow<CharacterType>(requiredLength.unsafeGet());
-}
-
-UChar* StringBuilder::appendUninitializedWithoutOverflowCheckForUChar(CheckedInt32 requiredLength)
-{
-    return appendUninitializedWithoutOverflowCheck<UChar>(requiredLength);
-}
-
-LChar* StringBuilder::appendUninitializedWithoutOverflowCheckForLChar(CheckedInt32 requiredLength)
-{
-    return appendUninitializedWithoutOverflowCheck<LChar>(requiredLength);
+    return appendUninitializedSlow<CharType>(requiredLength.unsafeGet());
 }
 
-// Make 'requiredLength' capacity be available in m_buffer, update m_string & m_length,
+// Make 'length' additional capacity be available in m_buffer, update m_string & m_length,
 // return a pointer to the newly allocated storage.
-template<typename CharacterType>
-CharacterType* StringBuilder::appendUninitializedSlow(unsigned requiredLength)
+template <typename CharType>
+CharType* StringBuilder::appendUninitializedSlow(unsigned requiredLength)
 {
     ASSERT(!hasOverflowed());
     ASSERT(requiredLength);
@@ -288,15 +270,15 @@ CharacterType* StringBuilder::appendUninitializedSlow(unsigned requiredLength)
         // 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<CharacterType>(expandedCapacity(capacity(), requiredLength));
+        reallocateBuffer<CharType>(expandedCapacity(capacity(), requiredLength));
     } else {
         ASSERT(m_string.length() == m_length.unsafeGet<unsigned>());
-        allocateBuffer(m_length ? m_string.characters<CharacterType>() : nullptr, expandedCapacity(capacity(), requiredLength));
+        allocateBuffer(m_length ? m_string.characters<CharType>() : nullptr, expandedCapacity(capacity(), requiredLength));
     }
     if (UNLIKELY(hasOverflowed()))
         return nullptr;
 
-    CharacterType* result = getBufferCharacters<CharacterType>() + m_length.unsafeGet();
+    CharType* result = getBufferCharacters<CharType>() + m_length.unsafeGet();
     m_length = requiredLength;
     ASSERT(!hasOverflowed());
     ASSERT(m_buffer->length() >= m_length.unsafeGet<unsigned>());

diff --git a/Source/WTF/wtf/text/StringBuilder.h b/Source/WTF/wtf/text/StringBuilder.h
index 18b66de..998b857 100644 (file)
--- a/Source/WTF/wtf/text/StringBuilder.h
+++ b/Source/WTF/wtf/text/StringBuilder.h
@@ -231,9 +231,6 @@ public:
     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()) {
@@ -353,19 +350,16 @@ private:
     void allocateBuffer(const LChar* currentCharacters, unsigned requiredLength);
     void allocateBuffer(const UChar* currentCharacters, unsigned requiredLength);
     void allocateBufferUpConvert(const LChar* currentCharacters, unsigned requiredLength);
-    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();
+    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();
     WTF_EXPORT_PRIVATE void reifyString() const;
 
-    template<typename... StringTypeAdapters> void flexibleAppendFromAdapters(StringTypeAdapters...);
-
     mutable String m_string;
     RefPtr<StringImpl> m_buffer;
     union {
@@ -380,54 +374,22 @@ private:
 #endif
 };
 
-template<>
+template <>
 ALWAYS_INLINE LChar* StringBuilder::getBufferCharacters<LChar>()
 {
     ASSERT(m_is8Bit);
     return m_bufferCharacters8;
 }
 
-template<>
+template <>
 ALWAYS_INLINE UChar* StringBuilder::getBufferCharacters<UChar>()
 {
     ASSERT(!m_is8Bit);
     return m_bufferCharacters16;
 }
 
-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)
+template <typename CharType>
+bool equal(const StringBuilder& s, const CharType* buffer, unsigned length)
 {
     if (s.length() != length)
         return false;
@@ -438,7 +400,7 @@ bool equal(const StringBuilder& s, const CharacterType* buffer, unsigned length)
     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())

diff --git a/Source/WTF/wtf/text/StringConcatenate.h b/Source/WTF/wtf/text/StringConcatenate.h
index 0537b10..c8cba07 100644 (file)
--- a/Source/WTF/wtf/text/StringConcatenate.h
+++ b/Source/WTF/wtf/text/StringConcatenate.h
@@ -248,16 +248,16 @@ inline bool are8Bit(Adapter adapter, Adapters ...adapters)
 }
 
 template<typename ResultType, typename Adapter>
-inline void stringTypeAdapterAccumulator(ResultType* result, Adapter adapter)
+inline void makeStringAccumulator(ResultType* result, Adapter adapter)
 {
     adapter.writeTo(result);
 }
 
 template<typename ResultType, typename Adapter, typename... Adapters>
-inline void stringTypeAdapterAccumulator(ResultType* result, Adapter adapter, Adapters ...adapters)
+inline void makeStringAccumulator(ResultType* result, Adapter adapter, Adapters ...adapters)
 {
     adapter.writeTo(result);
-    stringTypeAdapterAccumulator(result + adapter.length(), adapters...);
+    makeStringAccumulator(result + adapter.length(), adapters...);
 }
 
 template<typename StringTypeAdapter, typename... StringTypeAdapters>
@@ -276,7 +276,7 @@ String tryMakeStringFromAdapters(StringTypeAdapter adapter, StringTypeAdapters .
         if (!resultImpl)
             return String();
 
-        stringTypeAdapterAccumulator(buffer, adapter, adapters...);
+        makeStringAccumulator(buffer, adapter, adapters...);
 
         return resultImpl;
     }
@@ -286,7 +286,7 @@ String tryMakeStringFromAdapters(StringTypeAdapter adapter, StringTypeAdapters .
     if (!resultImpl)
         return String();
 
-    stringTypeAdapterAccumulator(buffer, adapter, adapters...);
+    makeStringAccumulator(buffer, adapter, adapters...);
 
     return resultImpl;
 }

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index a237477..7a07b40 100644 (file)
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,16 @@
+2019-07-10  Ryan Haddad  <ryanhaddad@apple.com>
+
+        Unreviewed, rolling out r247286.
+
+        Caused TestWTF.WTF.StringOperators to fail on debug bots
+
+        Reverted changeset:
+
+        "Add StringBuilder member function which allows makeString()
+        style variadic argument construction"
+        https://bugs.webkit.org/show_bug.cgi?id=198997
+        https://trac.webkit.org/changeset/247286
+
 2019-07-10  Aakash Jain  <aakash_jain@apple.com>
 
         [ews-build] Explicitly use perl or python while invoking scripts

diff --git a/Tools/TestWebKitAPI/Tests/WTF/StringBuilder.cpp b/Tools/TestWebKitAPI/Tests/WTF/StringBuilder.cpp
index e4b7c02..ab67ce1 100644 (file)
--- a/Tools/TestWebKitAPI/Tests/WTF/StringBuilder.cpp
+++ b/Tools/TestWebKitAPI/Tests/WTF/StringBuilder.cpp
@@ -116,29 +116,6 @@ TEST(StringBuilderTest, Append)
     }
 }
 
-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;