Bug 152897

Summary: Rename StringFromCharCode to StringFromSingleCharCode.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fpizlo, keith_miller, msaboff, saam
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch. dbates: review+

Mark Lam
Reported 2016-01-08 09:45:07 PST
StringFromSingleCharCode is a better name because the intrinsic it represents only applies when we are converting from a single char code.
Attachments
proposed patch. (10.59 KB, patch)
2016-01-08 09:50 PST, Mark Lam 		dbates: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Mark Lam
Comment 1 2016-01-08 09:50:22 PST
Created attachment 268548 [details] proposed patch.
Daniel Bates
Comment 2 2016-01-08 10:02:37 PST
Comment on attachment 268548 [details] proposed patch. Mark Lam told me on IRC that the StringFromCharCode logic is called when the DFG detects that "String.fromCharCode is only passed a single value". For completeness the JavaScript API String.fromCharCode() takes more than one char code.
Mark Lam
Comment 3 2016-01-08 10:07:06 PST
(In reply to comment #2) > Comment on attachment 268548 [details] > proposed patch. > > Mark Lam told me on IRC that the StringFromCharCode logic is called when the > DFG detects that "String.fromCharCode is only passed a single value". For > completeness the JavaScript API String.fromCharCode() takes more than one > char code. To clarify, the StringFromCharCode logic here refers to a DFG intrinsic implementation of String.fromCharCode(). See DFGByteCodeParser.cpp for where it will only emit this intrinsic node if the number of arguments is 2 (this + the single char code).
Mark Lam
Comment 4 2016-01-08 10:12:00 PST
Thanks for the review. Landed in r194767: <http://trac.webkit.org/r194767>.
Filip Pizlo
Comment 5 2016-01-08 10:22:13 PST
I think that it should take more than a non-JSC reviewer to rename parts of DFG IR.
Mark Lam
Comment 6 2016-01-08 10:40:03 PST
Fil has told me offline that it is common practice to name DFG intrinsic nodes based on the functions they replace, even if the intrinsic does not fully implement the most generic form of said functions. For example, ArithMax, ArithMin, and NeewArray. I will roll out the patch.
Mark Lam
Comment 7 2016-01-08 10:45:40 PST
The patch has been rolled out in r194770: <http://trac.webkit.org/r194770>.
Note You need to log in before you can comment on or make changes to this bug.