Changeset 55943 in webkit for trunk/JavaScriptCore/runtime/Identifier.cpp

Timestamp:

Mar 12, 2010, 3:49:05 PM (15 years ago)

Author:

[email protected]

Message:

Bug 36075 - Clean up screwyness re static string impls & Identifiers.

Reviewed by Oliver Hunt.

JavaScriptCore:

• API/JSClassRef.cpp:

(OpaqueJSClass::~OpaqueJSClass): Classname may be null/empty, and these are an identifer. This is okay, since the null/empty strings are shared across all threads.

• JavaScriptCore.exp:
• runtime/Identifier.cpp:

(JSC::Identifier::add): No need to explicitly hash null reps, this is done in the ststic UStringImpl constructor.
(JSC::Identifier::addSlowCase): UStringImpl::empty() handled & checkCurrentIdentifierTable now called in the header.
(JSC::Identifier::checkCurrentIdentifierTable): Replaces checkSameIdentifierTable (this no longer checked the rep since the identifierTable pointer was removed from UString::Rep long ago).

• runtime/Identifier.h:

(JSC::Identifier::add): Replace call to checkSameIdentifierTable with call to checkCurrentIdentifierTable at head of function.

• runtime/UStringImpl.cpp:

(JSC::UStringImpl::~UStringImpl): Remove call to checkConsistency - this function no longer checks anything interesting.

• runtime/UStringImpl.h:

(JSC::UStringOrRopeImpl::UStringOrRopeImpl): Set s_refCountFlagIsIdentifier in static constructor.
(JSC::UStringImpl::UStringImpl): remove calls to checkConsistency (see above), add new ASSERT to substring constructor.
(JSC::UStringImpl::setHash): ASSERT not static (static strings set the hash in their constructor, should not reach this code path).
(JSC::UStringImpl::create): Add missing ASSERT.
(JSC::UStringImpl::setIsIdentifier): ASSERT !isStatic() (static strings hash set in constructor).

WebCore:

• platform/text/StringImpl.cpp:

(WebCore::StringImpl::~StringImpl): Add ASSERT
(WebCore::StringImpl::sharedBuffer): Add ASSERT

• platform/text/StringImpl.h:

(WebCore::StringImpl::setHash): Add ASSERT
(WebCore::StringImpl::isStatic): added.

File:

• trunk/JavaScriptCore/runtime/Identifier.cpp (modified) (3 diffs)

trunk/JavaScriptCore/runtime/Identifier.cpp

@@ -125 +125 @@
 PassRefPtr<UString::Rep> Identifier::add(JSGlobalData* globalData, const char* c)
 {
-    if (!c) {
-        UString::Rep* rep = UString::null().rep();
-        rep->hash();
-        return rep;
-    }
+    if (!c)
+        return UString::null().rep();
     if (!c[0])
         return UString::Rep::empty();
@@ -210 +207 @@
 {
     ASSERT(!r->isIdentifier());
+    // The empty & null strings are static singletons, and static strings are handled
+    // in ::add() in the header, so we should never get here with a zero length string.
+    ASSERT(r->length());
+
     if (r->length() == 1) {
         UChar c = r->characters()[0];
         if (c <= 0xFF)
             r = globalData->smallStrings.singleCharacterStringRep(c);
-            if (r->isIdentifier()) {
-#ifndef NDEBUG
-                checkSameIdentifierTable(globalData, r);
-#endif
+            if (r->isIdentifier())
                 return r;
-            }
-    }
-    if (!r->length())
-        return UString::Rep::empty();
+    }
+
     return *globalData->identifierTable->add(r).first;
 }
@@ -253 +249 @@
 #ifndef NDEBUG
 
-void Identifier::checkSameIdentifierTable(ExecState* exec, UString::Rep*)
-{
-    ASSERT_UNUSED(exec, exec->globalData().identifierTable == currentIdentifierTable());
-}
-
-void Identifier::checkSameIdentifierTable(JSGlobalData* globalData, UString::Rep*)
-{
+void Identifier::checkCurrentIdentifierTable(JSGlobalData* globalData)
+{
+    // Check the identifier table accessible through the threadspecific matches the
+    // globalData's identifier table.
     ASSERT_UNUSED(globalData, globalData->identifierTable == currentIdentifierTable());
 }
 
+void Identifier::checkCurrentIdentifierTable(ExecState* exec)
+{
+    checkCurrentIdentifierTable(&exec->globalData());
+}
+
 #else
 
-void Identifier::checkSameIdentifierTable(ExecState*, UString::Rep*)
-{
-}
-
-void Identifier::checkSameIdentifierTable(JSGlobalData*, UString::Rep*)
-{
-}
+// These only exists so that our exports are the same for debug and release builds.
+// This would be an ASSERT_NOT_REACHED(), but we're in NDEBUG only code here!
+void Identifier::checkCurrentIdentifierTable(JSGlobalData*) { CRASH(); }
+void Identifier::checkCurrentIdentifierTable(ExecState*) { CRASH(); }
 
 #endif