Bug 143424

Summary: [ES6] DFG and FTL should be aware of that StringConstructor behavior for symbols becomes different from ToString
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fpizlo, ggaren, joepeck
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 141279    
Attachments:
Description Flags
Patch
none
Patch none

Yusuke Suzuki
Reported 2015-04-05 14:49:45 PDT
In ES6, StringConstructor behavior becomes different from ToString abstract operations in the spec. (and JSValue::toString). ToString(symbol) throws a type error. String(symbol) produces SymbolDescriptiveString(value). So, in DFG and FTL phase, they should not inline String constructor to ToString. Now, in the template literals patch, ToString DFG operation is planned to be used for it. And current ToString behavior is aligned to the spec (and JSValue::toString). I think keeping it is better. So intead of changing ToString behavior, I think adding StringConstructor operation into DFG and FTL is nice.
Attachments
Patch (21.77 KB, patch)
2015-04-05 15:49 PDT, Yusuke Suzuki
no flags
Patch (21.95 KB, patch)
2015-04-05 18:04 PDT, Yusuke Suzuki
no flags
Yusuke Suzuki
Comment 1 2015-04-05 15:49:11 PDT
Yusuke Suzuki
Comment 2 2015-04-05 17:42:44 PDT
*** Bug 143427 has been marked as a duplicate of this bug. ***
Joseph Pecoraro
Comment 3 2015-04-05 17:46:40 PDT
Comment on attachment 250171 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250171&action=review > Source/JavaScriptCore/runtime/StringConstructor.cpp:112 > + return jsString(exec, asSymbol(argument)->descriptiveString()); Nit: This can be jsNontrivialString, which can be used for Strings guaranteed to be 2 or more characters long, and this is guaranteed to have "Symbol()".
Yusuke Suzuki
Comment 4 2015-04-05 18:01:28 PDT
Comment on attachment 250171 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=250171&action=review >> Source/JavaScriptCore/runtime/StringConstructor.cpp:112 >> + return jsString(exec, asSymbol(argument)->descriptiveString()); > > Nit: This can be jsNontrivialString, which can be used for Strings guaranteed to be 2 or more characters long, and this is guaranteed to have "Symbol()". Thank you! I've changed it.
Yusuke Suzuki
Comment 5 2015-04-05 18:04:10 PDT
Geoffrey Garen
Comment 6 2015-04-06 11:18:27 PDT
Comment on attachment 250182 [details] Patch r=me
Yusuke Suzuki
Comment 7 2015-04-06 11:19:42 PDT
Comment on attachment 250182 [details] Patch Thank you for your review :D
WebKit Commit Bot
Comment 8 2015-04-06 12:08:06 PDT
Comment on attachment 250182 [details] Patch Clearing flags on attachment: 250182 Committed r182433: <http://trac.webkit.org/changeset/182433>
WebKit Commit Bot
Comment 9 2015-04-06 12:08:10 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.