Let's put field in SymbolImpl. And add a flag for Null symbol (`Symbol()`).
Created attachment 295830 [details] Patch
Attachment 295830 [details] did not pass style-queue: ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 3 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 295831 [details] Patch
Created attachment 295832 [details] Patch
Attachment 295832 [details] did not pass style-queue: ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 3 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 295836 [details] Patch
Attachment 295836 [details] did not pass style-queue: ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 3 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 295837 [details] Patch
Attachment 295837 [details] did not pass style-queue: ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 3 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 295838 [details] Patch
Attachment 295838 [details] did not pass style-queue: ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 3 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 295839 [details] Patch
Attachment 295839 [details] did not pass style-queue: ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 3 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 295840 [details] Patch
Attachment 295840 [details] did not pass style-queue: ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 3 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 295840 [details] Patch Attachment 295840 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2601095 Number of test failures exceeded the failure limit.
Created attachment 295848 [details] Archive of layout-test-results from ews114 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 295927 [details] Patch
Attachment 295927 [details] did not pass style-queue: ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:39: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WTF/wtf/text/UniquedStringImpl.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 3 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
And this also paves the way for static SymbolImpl.
Comment on attachment 295927 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=295927&action=review > Source/WTF/wtf/text/StringImpl.cpp:118 > + SymbolImpl& symbol = static_cast<SymbolImpl&>(*this); I would use auto& here. > Source/WTF/wtf/text/StringImpl.cpp:120 > + if (symbol.symbolRegistry()) > + symbol.symbolRegistry()->remove(symbol); I typically would put this into a scoped local variable, but not everyone agrees and I think it’s likely that the generated code will be identical. > Source/WTF/wtf/text/StringImpl.h:732 > + enum CreateSymbolTag { CreateSymbol }; > + // Used to create new symbol strings that holds existing 8-bit [[Description]] string as a substring buffer (BufferSubstring). I suggest a blank line here after the enum. > Source/WTF/wtf/text/SymbolImpl.cpp:56 > + return adoptRef(*new SymbolImpl()); No need for () here after SymbolImpl. > Source/WTF/wtf/text/SymbolImpl.h:109 > +} > + > + > + > #if !ASSERT_DISABLED Should just have one blank line, not three.
Comment on attachment 295927 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=295927&action=review >> Source/WTF/wtf/text/StringImpl.cpp:118 >> + SymbolImpl& symbol = static_cast<SymbolImpl&>(*this); > > I would use auto& here. Fixed. >> Source/WTF/wtf/text/StringImpl.cpp:120 >> + symbol.symbolRegistry()->remove(symbol); > > I typically would put this into a scoped local variable, but not everyone agrees and I think it’s likely that the generated code will be identical. OK, i'll change this to `auto* symbolRegistry = symbol.symbolRegistry()`. >> Source/WTF/wtf/text/StringImpl.h:732 >> + // Used to create new symbol strings that holds existing 8-bit [[Description]] string as a substring buffer (BufferSubstring). > > I suggest a blank line here after the enum. Inserted. >> Source/WTF/wtf/text/SymbolImpl.cpp:56 >> + return adoptRef(*new SymbolImpl()); > > No need for () here after SymbolImpl. Dropped. >> Source/WTF/wtf/text/SymbolImpl.h:109 >> #if !ASSERT_DISABLED > > Should just have one blank line, not three. Ooops. Fixed.
Committed r209309: <http://trac.webkit.org/changeset/209309>