Hi,
https://wiki.mozilla.org/JavaScript:SpiderMonkey:C%2B%2B_Coding_Style says this:
> Member variable names, private or public, are not decorated. Examples of
> improper decoration include:
>
> class Fail
> {
> size_t capacity_; // unsightly
> T *mBegin; // makes babies cry
> T *m_end; // much typing
> }
This causes problems. Consider this example, which is what official
style would have us use:
class X
{
int x;
X(int x) : x(x) { }
int x() { return x; }
};
Having two members (a field and a method) with the same name isn't
allowed. The standard fix in SpiderMonkey is to change the data field
to 'x_'
Also, if we turn on -Wshadow, we get a warning about the parameter in
the constructor. The official suggestion for this is to use |this->x|
for the data member, but nobody ever does that precisely because we
don't enable -Wshadow. (And it doesn't work for initializer lists in
constructors.)
Here's what the same class would look like in Gecko:
class X
{
int mX;
X(int aX) : mX(aX) { }
int x() { return mX; }
};
No problems, even with -Wshadow. And while it's arguably uglier, in
my experience with Gecko it makes code more understandable, especially
when you have complex methods -- you never have to wonder if an
identifier is a local variable, a parameter, or a class member.
I also think -Wshadow is a wonderful tool; I've been bitten more than
once by its lack of presence when building SpiderMonkey. I always get
nervous when I add a local variable with a generic name like "length"
or "obj" in a complex method.
The use of "foo_" style members seems to be increasing; IonMonkey
does it a lot, albeit not universally. And if you look closely, there
are actually quite a few cases of |mFoo| data members in the code
(public/Vector.h is a notable example).
Given that the official "no decoration ever" rule is clearly untenable
and ignored, is it worth blessing a new style? Obviously we have to
deal with the fact that we have lots of code that is mostly
undecorated.
Here's a straw-man suggestion that would be doable:
- Stuff done up front:
- Turn on -Wshadow.
- Convert all parameters that clash with a member to |aFoo|.
- Convert all existing |foo_| data members to |mFoo|.
- Modify the style guide to say that:
- All new data members should be |mFoo|.
- Parameters don't need decoration unless they conflict, in which
case they should be |aFoo|.
- Things to gradually change:
- Convert existing |foo_| data members to |mFoo|.
The end result would be a bit of a mishmash of decoration and
non-decoration, but the current situation is also a mishmash, and this
new scheme would protect us against shadowing bugs, while also making
data members more obvious. It would also bring us slightly closer to
Gecko style, which isn't a bad thing. (In passing, I note that mfbt/
seems to use SpiderMonkey style, which is interesting.)
Do other people care about this, or am I just tilting at windmills?
If so, I'd like at minimum to be able to turn on -Wshadow, which would
require a blessed way to avoid conflicts when necessary. The existing
|foo_| convention is good for data members, we'd just need an
equivalent convention for parameters. |aFoo| would do -- it doesn't
match |foo_|, but I can live with that. Or other suggestions are
welcome. "foo__?" :P
Nick
_______________________________________________
dev-tech-js-engine-internals mailing list
[email protected]
https://lists.mozilla.org/listinfo/dev-tech-js-engine-internals
