Bug 20416

Summary: Made room for a free word in JSCell
Product: WebKit Reporter: Geoffrey Garen <ggaren>
Component: JavaScriptCoreAssignee: Geoffrey Garen <ggaren>
Status: RESOLVED FIXED    
Severity: Normal CC: zwarich
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Macintosh   
OS: OS X 10.5   
Attachments:
Description Flags
patch zwarich: review+

Description Geoffrey Garen 2008-08-17 01:54:09 PDT
Patch coming.
Comment 1 Geoffrey Garen 2008-08-17 01:56:15 PDT
Created attachment 22840 [details]
patch

Mostly mechanical. SunSpider says no change.
Comment 2 Cameron Zwarich (cpst) 2008-08-17 03:30:14 PDT
I'll review this soon.
Comment 3 Cameron Zwarich (cpst) 2008-08-17 09:57:58 PDT
Comment on attachment 22840 [details]
patch

It seems to be our style for member variables of *Data structs to not have an "m_" prefix, so you should do that for JSCallbackObjectData, ArgumentsData, RegExpObjectData, and JSDOMWindowBaseData, unless we somehow reach an agreement that the existing style should be changed. There is also a stray WebCore xcodeproj change, although that seems to happen whenever you open it with the new version, so someone should check it in for all the projects.

Other than that, r=me.
Comment 4 Geoffrey Garen 2008-08-17 12:50:55 PDT
> It seems to be our style for member variables of *Data structs to not have an
> "m_" prefix

Yeah, I think that's generally the style I've been using.

> so you should do that for JSCallbackObjectData, ArgumentsData,
> RegExpObjectData, and JSDOMWindowBaseData

Fixed.

> unless we somehow reach an agreement
> that the existing style should be changed.

I do think there's a lack of consensus in this area. We should probably talk it over and write something up in the style guidelines. One drawback to the "no m_" rule is that constructors, which have to differentiate between arguments and data members of the same name, get a little weird. I've been using a trailing "_" to solve that problem, but that's actually the *inverse* of the traditional C++ style.

> There is also a stray WebCore
> xcodeproj change, although that seems to happen whenever you open it with the
> new version, so someone should check it in for all the projects.

I'll take care of this in a separate patch.
Comment 5 Geoffrey Garen 2008-08-17 13:24:23 PDT
Committed revision 35807.