Need to distinguish between Symbol() and Symbol("").
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 Mar 2016 23:20:27 +0000 (23:20 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 Mar 2016 23:20:27 +0000 (23:20 +0000)
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.

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/PrivateName.h
Source/WTF/ChangeLog
Source/WTF/wtf/text/StringImpl.cpp
Source/WTF/wtf/text/StringImpl.h
Source/WTF/wtf/text/StringStatics.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WTF/StringImpl.cpp

index dbf7f63..abf03b4 100644 (file)
@@ -1,3 +1,13 @@
+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>
 
         Temporarily disable the separated heap.
index 2b6ba01..85c139e 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * 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
  * modification, are permitted provided that the following conditions
@@ -33,7 +33,7 @@ namespace JSC {
 class PrivateName {
 public:
     PrivateName()
-        : m_uid(StringImpl::createSymbolEmpty())
+        : m_uid(StringImpl::createNullSymbol())
     {
     }
 
index e9d073e..01e7141 100644 (file)
@@ -1,3 +1,32 @@
+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>
 
         Remove ENABLE(ES6_TEMPLATE_LITERAL_SYNTAX) guards
index 7f79489..fc16274 100644 (file)
@@ -306,9 +306,9 @@ Ref<SymbolImpl> StringImpl::createSymbol(PassRefPtr<StringImpl> rep)
     return adoptRef(static_cast<SymbolImpl&>(*new (NotNull, stringImpl) StringImpl(CreateSymbol, rep->m_data16, rep->length(), ownerRep)));
 }
 
-Ref<SymbolImpl> StringImpl::createSymbolEmpty()
+Ref<SymbolImpl> StringImpl::createNullSymbol()
 {
-    return createSymbol(empty());
+    return createSymbol(null());
 }
 
 bool StringImpl::containsOnlyWhitespace()
index 2823434..4a69ddc 100644 (file)
@@ -408,7 +408,7 @@ public:
         return constructInternal<T>(resultImpl, length);
     }
 
-    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);
 
     // Reallocate the StringImpl. The originalString must be only owned by the PassRefPtr,
@@ -484,6 +484,7 @@ public:
     StringKind stringKind() const { return static_cast<StringKind>(m_hashAndFlags & s_hashMaskStringKind); }
     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)
     {
@@ -863,6 +864,7 @@ private:
     template <typename CharType> static Ref<StringImpl> createInternal(const CharType*, unsigned);
     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.
     static const unsigned s_refCountFlagIsStaticString = 0x1;
index ee21c75..f0eb06f 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * 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
  * modification, are permitted provided that the following conditions
 
 namespace WTF {
 
+StringImpl* StringImpl::null()
+{
+    static NeverDestroyed<StringImpl> nullString(ConstructEmptyString);
+    return &nullString.get();
+}
+
 StringImpl* StringImpl::empty()
 {
     static NeverDestroyed<StringImpl> emptyString(ConstructEmptyString);
index 41c87ed..355fd35 100644 (file)
@@ -1,3 +1,14 @@
+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>
 
         Remove blank lines after #include "config.h"
index 1747529..8cd6c09 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * 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
  * modification, are permitted provided that the following conditions
@@ -513,10 +513,11 @@ TEST(WTF, StringImplEndsWithIgnoringASCIICaseWithEmpty)
     ASSERT_FALSE(empty->endsWithIgnoringASCIICase(*reference.get()));
 }
 
-TEST(WTF, StringImplCreateSymbolEmpty)
+TEST(WTF, StringImplCreateNullSymbol)
 {
-    RefPtr<StringImpl> reference = StringImpl::createSymbolEmpty();
+    RefPtr<StringImpl> reference = StringImpl::createNullSymbol();
     ASSERT_TRUE(reference->isSymbol());
+    ASSERT_TRUE(reference->isNullSymbol());
     ASSERT_FALSE(reference->isAtomic());
     ASSERT_EQ(0u, reference->length());
     ASSERT_TRUE(equal(reference.get(), ""));
@@ -527,11 +528,23 @@ TEST(WTF, StringImplCreateSymbol)
     RefPtr<StringImpl> original = stringFromUTF8("original");
     RefPtr<StringImpl> reference = StringImpl::createSymbol(original);
     ASSERT_TRUE(reference->isSymbol());
+    ASSERT_FALSE(reference->isNullSymbol());
     ASSERT_FALSE(reference->isAtomic());
     ASSERT_FALSE(original->isSymbol());
     ASSERT_FALSE(original->isAtomic());
     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(), ""));
 }
 
 TEST(WTF, StringImplSymbolToAtomicString)
@@ -548,9 +561,9 @@ TEST(WTF, StringImplSymbolToAtomicString)
     ASSERT_FALSE(reference->isAtomic());
 }
 
-TEST(WTF, StringImplSymbolEmptyToAtomicString)
+TEST(WTF, StringImplNullSymbolToAtomicString)
 {
-    RefPtr<StringImpl> reference = StringImpl::createSymbolEmpty();
+    RefPtr<StringImpl> reference = StringImpl::createNullSymbol();
     ASSERT_TRUE(reference->isSymbol());
     ASSERT_FALSE(reference->isAtomic());