Changeset 198168 in webkit

Timestamp:

Mar 14, 2016, 4:20:27 PM (9 years ago)

Author:

mark.lam@apple.com

Message:

Need to distinguish between Symbol() and Symbol("").
​https://bugs.webkit.org/show_bug.cgi?id=155438

Reviewed by Saam Barati.

Source/JavaScriptCore:

• runtime/PrivateName.h:

(JSC::PrivateName::PrivateName):

Source/WTF:

While toString of both Symbol() and Symbol("") yields "Symbol()", Function.name
should yield "" and "[]" respectively. Hence, we need to tell between the two.
This functionality will be needed later in ​https://bugs.webkit.org/show_bug.cgi?id=155437.

We achieve this by creating another singleton instance like the empty StringImpl
as the null StringImpl. isNullSymbol() tests if the Stringimpl instance is a
symbol, and its substring is the null() singleton.

• wtf/text/StringImpl.cpp:

(WTF::StringImpl::createSymbol):
(WTF::StringImpl::createNullSymbol):
(WTF::StringImpl::containsOnlyWhitespace):
(WTF::StringImpl::createSymbolEmpty): Deleted.

• wtf/text/StringImpl.h:

(WTF::StringImpl::tryCreateUninitialized):
(WTF::StringImpl::stringKind):
(WTF::StringImpl::isSymbol):
(WTF::StringImpl::isAtomic):
(WTF::StringImpl::isNullSymbol):

• wtf/text/StringStatics.cpp:

(WTF::StringImpl::null):

Tools:

• TestWebKitAPI/Tests/WTF/StringImpl.cpp:

(TestWebKitAPI::TEST):

• Test that the a symbol with an empty string is not equivalent to a null symbol.

Location:

trunk

Files:

• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/PrivateName.h (modified) (2 diffs)
• Source/WTF/ChangeLog (modified) (1 diff)
• Source/WTF/wtf/text/StringImpl.cpp (modified) (1 diff)
• Source/WTF/wtf/text/StringImpl.h (modified) (3 diffs)
• Source/WTF/wtf/text/StringStatics.cpp (modified) (2 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WTF/StringImpl.cpp (modified) (5 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-03-14  Mark Lam  <mark.lam@apple.com>
+
+        Need to distinguish between Symbol() and Symbol("").
+        https://bugs.webkit.org/show_bug.cgi?id=155438
+
+        Reviewed by Saam Barati.
+
+        * runtime/PrivateName.h:
+        (JSC::PrivateName::PrivateName):
+
 2016-03-14  Oliver Hunt  <oliver@apple.com>
 

trunk/Source/JavaScriptCore/runtime/PrivateName.h

@@ -1 +1 @@
 /*
- * Copyright (C) 2012 Apple Inc. All rights reserved.
+ * Copyright (C) 2012, 2016 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -34 +34 @@
 public:
     PrivateName()
-        : m_uid(StringImpl::createSymbolEmpty())
+        : m_uid(StringImpl::createNullSymbol())
     {
     }

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2016-03-14  Mark Lam  <mark.lam@apple.com>
+
+        Need to distinguish between Symbol() and Symbol("").
+        https://bugs.webkit.org/show_bug.cgi?id=155438
+
+        Reviewed by Saam Barati.
+
+        While toString of both Symbol() and Symbol("") yields "Symbol()", Function.name
+        should yield "" and "[]" respectively.  Hence, we need to tell between the two.
+        This functionality will be needed later in https://bugs.webkit.org/show_bug.cgi?id=155437.
+
+        We achieve this by creating another singleton instance like the empty StringImpl
+        as the null StringImpl.  isNullSymbol() tests if the Stringimpl instance is a
+        symbol, and its substring is the null() singleton.
+
+        * wtf/text/StringImpl.cpp:
+        (WTF::StringImpl::createSymbol):
+        (WTF::StringImpl::createNullSymbol):
+        (WTF::StringImpl::containsOnlyWhitespace):
+        (WTF::StringImpl::createSymbolEmpty): Deleted.
+        * wtf/text/StringImpl.h:
+        (WTF::StringImpl::tryCreateUninitialized):
+        (WTF::StringImpl::stringKind):
+        (WTF::StringImpl::isSymbol):
+        (WTF::StringImpl::isAtomic):
+        (WTF::StringImpl::isNullSymbol):
+        * wtf/text/StringStatics.cpp:
+        (WTF::StringImpl::null):
+
 2016-03-13  Joseph Pecoraro  <pecoraro@apple.com>
 

trunk/Source/WTF/wtf/text/StringImpl.cpp

@@ -307 +307 @@
 }
 
-Ref<SymbolImpl> StringImpl::createSymbolEmpty()
-{
-    return createSymbol(empty());
+Ref<SymbolImpl> StringImpl::createNullSymbol()
+{
+    return createSymbol(null());
 }
 

trunk/Source/WTF/wtf/text/StringImpl.h

@@ -409 +409 @@
     }
 
-    WTF_EXPORT_STRING_API static Ref<SymbolImpl> createSymbolEmpty();
+    WTF_EXPORT_STRING_API static Ref<SymbolImpl> createNullSymbol();
     WTF_EXPORT_STRING_API static Ref<SymbolImpl> createSymbol(PassRefPtr<StringImpl> rep);
 
@@ -485 +485 @@
     bool isSymbol() const { return m_hashAndFlags & s_hashFlagStringKindIsSymbol; }
     bool isAtomic() const { return m_hashAndFlags & s_hashFlagStringKindIsAtomic; }
+    bool isNullSymbol() const { return isSymbol() && (substringBuffer() == null()); }
 
     void setIsAtomic(bool isAtomic)
@@ -864 +865 @@
     WTF_EXPORT_PRIVATE NEVER_INLINE unsigned hashSlowCase() const;
     WTF_EXPORT_PRIVATE static unsigned nextHashForSymbol();
+    WTF_EXPORT_PRIVATE static StringImpl* null();
 
     // The bottom bit in the ref count indicates a static (immortal) string.

trunk/Source/WTF/wtf/text/StringStatics.cpp

@@ -1 +1 @@
 /*
- * Copyright (C) 2010 Apple Inc. All Rights Reserved.
+ * Copyright (C) 2010, 2016 Apple Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -41 +41 @@
 
 namespace WTF {
+
+StringImpl* StringImpl::null()
+{
+    static NeverDestroyed<StringImpl> nullString(ConstructEmptyString);
+    return &nullString.get();
+}
 
 StringImpl* StringImpl::empty()

trunk/Tools/ChangeLog

@@ +1 @@
+2016-03-14  Mark Lam  <mark.lam@apple.com>
+
+        Need to distinguish between Symbol() and Symbol("").
+        https://bugs.webkit.org/show_bug.cgi?id=155438
+
+        Reviewed by Saam Barati.
+
+        * TestWebKitAPI/Tests/WTF/StringImpl.cpp:
+        (TestWebKitAPI::TEST):
+        - Test that the a symbol with an empty string is not equivalent to a null symbol. 
+
 2016-03-14  David Kilzer  <ddkilzer@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WTF/StringImpl.cpp

@@ -1 +1 @@
 /*
- * Copyright (C) 2012 Apple Inc. All rights reserved.
+ * Copyright (C) 2012, 2016 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -514 +514 @@
 }
 
-TEST(WTF, StringImplCreateSymbolEmpty)
-{
-    RefPtr<StringImpl> reference = StringImpl::createSymbolEmpty();
+TEST(WTF, StringImplCreateNullSymbol)
+{
+    RefPtr<StringImpl> reference = StringImpl::createNullSymbol();
     ASSERT_TRUE(reference->isSymbol());
+    ASSERT_TRUE(reference->isNullSymbol());
     ASSERT_FALSE(reference->isAtomic());
     ASSERT_EQ(0u, reference->length());
@@ -528 +529 @@
     RefPtr<StringImpl> reference = StringImpl::createSymbol(original);
     ASSERT_TRUE(reference->isSymbol());
+    ASSERT_FALSE(reference->isNullSymbol());
     ASSERT_FALSE(reference->isAtomic());
     ASSERT_FALSE(original->isSymbol());
@@ -533 +535 @@
     ASSERT_EQ(original->length(), reference->length());
     ASSERT_TRUE(equal(reference.get(), "original"));
+
+    RefPtr<StringImpl> empty = stringFromUTF8("");
+    RefPtr<StringImpl> emptyReference = StringImpl::createSymbol(empty);
+    ASSERT_TRUE(emptyReference->isSymbol());
+    ASSERT_FALSE(emptyReference->isNullSymbol());
+    ASSERT_FALSE(emptyReference->isAtomic());
+    ASSERT_FALSE(empty->isSymbol());
+    ASSERT_FALSE(empty->isNullSymbol());
+    ASSERT_TRUE(empty->isAtomic());
+    ASSERT_EQ(empty->length(), emptyReference->length());
+    ASSERT_TRUE(equal(emptyReference.get(), ""));
 }
 
@@ -549 +562 @@
 }
 
-TEST(WTF, StringImplSymbolEmptyToAtomicString)
-{
-    RefPtr<StringImpl> reference = StringImpl::createSymbolEmpty();
+TEST(WTF, StringImplNullSymbolToAtomicString)
+{
+    RefPtr<StringImpl> reference = StringImpl::createNullSymbol();
     ASSERT_TRUE(reference->isSymbol());
     ASSERT_FALSE(reference->isAtomic());