Bug 151491

Summary: Fix coding style of Intl code
Product: WebKit Reporter: Sukolsak Sakshuwong <sukolsak>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, ggaren, keith_miller, mark.lam, msaboff, saam, sukolsak
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Sukolsak Sakshuwong 2015-11-20 05:18:31 PST
Fix coding style of Intl code according to Darin's comments in https://bugs.webkit.org/show_bug.cgi?id=147604

This patch does three things:
1. Rename pointers and references to ExecState from "exec" to "state".
2. Pass parameters by references instead of pointers if the parameters are required.
3. Remove the word "get" from the names of functions that don't return values through out arguments.

This should make future Intl patches smaller.
Comment 1 Sukolsak Sakshuwong 2015-11-20 05:23:22 PST
Created attachment 265950 [details]
Patch
Comment 2 Mark Lam 2015-11-20 09:09:28 PST
Comment on attachment 265950 [details]
Patch

Hmmm, I know Darin said to use "state" instead of "exec" for the ExecState*, but I wonder if we should do that.  Using "exec" to mean "ExecState*" is universally practiced in JSC code.  Changing to use "state" in just some places would actually make it harder to read.
Comment 3 Darin Adler 2015-11-20 15:56:49 PST
(In reply to comment #2)
> Hmmm, I know Darin said to use "state" instead of "exec" for the ExecState*,
> but I wonder if we should do that.  Using "exec" to mean "ExecState*" is
> universally practiced in JSC code.  Changing to use "state" in just some
> places would actually make it harder to read.

If you look you will see that we are starting to change "exec" to "state" in much new code, at least in WebCore.
Comment 4 Mark Lam 2015-11-20 16:20:37 PST
(In reply to comment #3)
> (In reply to comment #2)
> > Hmmm, I know Darin said to use "state" instead of "exec" for the ExecState*,
> > but I wonder if we should do that.  Using "exec" to mean "ExecState*" is
> > universally practiced in JSC code.  Changing to use "state" in just some
> > places would actually make it harder to read.
> 
> If you look you will see that we are starting to change "exec" to "state" in
> much new code, at least in WebCore.
Comment 5 Mark Lam 2015-11-20 16:22:08 PST
Sigh ... such a rookie mistake (clicked Save before adding comment) ...

(In reply to comment #3)
> If you look you will see that we are starting to change "exec" to "state" in
> much new code, at least in WebCore.

OK.  I'm good with this if this is the new direction we're taking.
Comment 6 Darin Adler 2015-11-20 18:30:18 PST
There might be people who don’t agree with me, but I think it’s a good change that we should eventually do everywhere.
Comment 7 Saam Barati 2015-11-20 19:48:56 PST
My vote would be to rename all ExecState to CallFrame.
And we have a natural name for the variable:
(CallFrame* callFrame)
Comment 8 WebKit Commit Bot 2015-11-30 14:21:31 PST
Comment on attachment 265950 [details]
Patch

Clearing flags on attachment: 265950

Committed r192831: <http://trac.webkit.org/changeset/192831>
Comment 9 WebKit Commit Bot 2015-11-30 14:21:35 PST
All reviewed patches have been landed.  Closing bug.