[JSC] String.fromCharCode's slow path always generates 16bit string
Created attachment 361577 [details] Patch
Comment on attachment 361577 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=361577&action=review r=me with small fix. > Source/JavaScriptCore/ChangeLog:10 > + and even worse, taints ropes 16bit if 16bit string is included in the given rope. We find that acorn-wtb Typo: We find => We found. Or if you want to take more credit: I found. > Source/JavaScriptCore/ChangeLog:12 > + 16bit string. However, only few strings are actually 16bit strings. This patch attempts to make 8bit string Typo: only few => only a few > Source/JavaScriptCore/runtime/StringConstructor.cpp:91 > + if (UNLIKELY(character & 0xFF00)) { Can we use or create a named WTF function for this?
Comment on attachment 361577 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=361577&action=review >> Source/JavaScriptCore/runtime/StringConstructor.cpp:91 >> + if (UNLIKELY(character & 0xFF00)) { > > Can we use or create a named WTF function for this? Yeah, I added `is8BitCharacter(ch)`, since ASCIICType::isASCII(ch) is different from this definition. (It does not allow the upper most bit is set, since it is precisely out of ASCII range).
Comment on attachment 361577 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=361577&action=review >> Source/JavaScriptCore/ChangeLog:10 >> + and even worse, taints ropes 16bit if 16bit string is included in the given rope. We find that acorn-wtb > > Typo: We find => We found. Or if you want to take more credit: I found. Fixed. >> Source/JavaScriptCore/ChangeLog:12 >> + 16bit string. However, only few strings are actually 16bit strings. This patch attempts to make 8bit string > > Typo: only few => only a few Fixed.
(In reply to Yusuke Suzuki from comment #3) > Comment on attachment 361577 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=361577&action=review > > >> Source/JavaScriptCore/runtime/StringConstructor.cpp:91 > >> + if (UNLIKELY(character & 0xFF00)) { > > > > Can we use or create a named WTF function for this? > > Yeah, I added `is8BitCharacter(ch)`, since ASCIICType::isASCII(ch) is > different from this definition. (It does not allow the upper most bit is > set, since it is precisely out of ASCII range). I think we should actually call it isLatin1. IIUC, that's the most precise term.
Comment on attachment 361577 [details] Patch Attachment 361577 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/11087027 New failing tests: imported/w3c/web-platform-tests/webrtc/simplecall.https.html
Created attachment 361591 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Committed r241233: <https://trac.webkit.org/changeset/241233>
<rdar://problem/47938406>
(In reply to Keith Miller from comment #5) > I think we should actually call it isLatin1. IIUC, that's the most precise > term. There are isLatin1 functions in Lexer.cpp already. This patch adds a new one. Could we consolidate?
(In reply to Darin Adler from comment #10) > (In reply to Keith Miller from comment #5) > > I think we should actually call it isLatin1. IIUC, that's the most precise > > term. > > There are isLatin1 functions in Lexer.cpp already. This patch adds a new > one. Could we consolidate? Oh, nice! I'll land it in a follow-up patch.
Committed r241246: <https://trac.webkit.org/changeset/241246>