| Summary: | Made room for a free word in JSCell | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Geoffrey Garen <ggaren> | ||||
| Component: | JavaScriptCore | Assignee: | Geoffrey Garen <ggaren> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | zwarich | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Mac | ||||||
| OS: | OS X 10.5 | ||||||
| Attachments: |
|
||||||
|
Description
Geoffrey Garen
2008-08-17 01:54:09 PDT
Created attachment 22840 [details]
patch
Mostly mechanical. SunSpider says no change.
I'll review this soon. 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.
> 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. Committed revision 35807. |