Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++

[Jeff Gilbert]

I propose that we stop recommending the universal use of an 'a' prefix for
arguments to functions in C and C++. If the prefix helps with
disambiguation, that's fine. However, use of this prefix should not be
prescribed in general.

`aFoo` does not provide any additional safety that I know of.[1] As a
superfluous prefix, it adds visual noise, reducing immediate readability of
all function declarations and subsequent usage of the variables within the
function definition.

Notable works or style guides [2] which do not recommend `aFoo`: [3]
* Google
* Linux Kernel
* Bjarne Stroustrup
* GCC
* LLVM
* Java Style (Java, non-C)
* PEP 0008 (Python, non-C)
* FreeBSD
* Unreal Engine
* Unity3D (largely C#)
* Spidermonkey
* Daala
* RR
* Rust
* Folly (from Facebook)
* C++ STL entrypoints
* IDL for web specs on W3C and WhatWG
* etc.

Notable works or style guides which *do* recommend `aFoo`:
* Mozilla (except for IDL, Java, and Python)
* ?

3rd-party projects in our tree which do not use `aFoo`:
* Cairo
* Skia
* ANGLE
* HarfBuzz
* ICU
* Chromium IPC
* everything under modules/ that isn't an nsFoo.c/cpp/h
* etc.?

3rd-party projects in our tree which *do* recommend `aFoo`:
* ?

As far as I can tell, the entire industry disagrees with us (as well as a
number of our own projects), which means we should have a good reason or
two for making our choice. No such reason is detailed in the style guide.

I propose we strike the `aFoo` recommendation from the Mozilla style guide.

-

[1]: Maybe it prevents accidental shadowing? No: Either this isn't allowed
by spec, or at least MSVC 2013 errors when compiling this.

[2]: I do not mean this as an endorsement of the listed works and guides,
but rather as illustration on how unusual our choice is.

[3]: I created an Etherpad into which people are welcome to gather other
works, projects, or style guides that I missed:
https://etherpad.mozilla.org/6FcHs9mJYQ

[Hubert Figuière]

On 06/07/15 11:12 PM, Jeff Gilbert wrote:
> I propose that we stop recommending the universal use of an 'a' prefix for
> arguments to functions in C and C++. If the prefix helps with
> disambiguation, that's fine. However, use of this prefix should not be
> prescribed in general.
>
> `aFoo` does not provide any additional safety that I know of.[1] As a
> superfluous prefix, it adds visual noise, reducing immediate readability of
> all function declarations and subsequent usage of the variables within the
> function definition.

[...]

> I propose we strike the `aFoo` recommendation from the Mozilla style guide.

I agree with this.

However, if we proceed to remove this from the style guide, will we run
a pass at removing the 'a' prefix or will we just let them be and have
individual code change eventually clean them up?

Do we have -Wshadow (or equivalent) enabled across the code base and
insurance that the warnings get dealt with?

Cheers,

Hub

[Mike Hommey]

On Mon, Jul 06, 2015 at 08:12:36PM -0700, Jeff Gilbert wrote:
> I propose that we stop recommending the universal use of an 'a' prefix for
> arguments to functions in C and C++. If the prefix helps with
> disambiguation, that's fine. However, use of this prefix should not be
> prescribed in general.
>
> `aFoo` does not provide any additional safety that I know of.[1] As a
> superfluous prefix, it adds visual noise, reducing immediate readability of
> all function declarations and subsequent usage of the variables within the
> function definition.
>

> I propose we strike the `aFoo` recommendation from the Mozilla style guide.

The existence of aFoo goes along with the existence of mFoo, sFoo, kFoo,
and others I might have forgotten. Not that I particularly care about
aFoo, but why strike this one and not the others?[1] It's not like they
have widespread use in the industry either. That is, the same reasoning
could be applied to them, yet, you're stopping at aFoo. Why?

Mike

[Bobby Holley]

I don't feel all that strongly about aFoo, but mFoo is incredibly important
for reviewing code when the reviewer isn't intimately familiar with the
code being changed (dom/ reviewers are a good example, because there's just
too much code to know by heart).

And if we accept mFoo, then the stylistic benefits of abolishing aFoo are
less compelling.

bholley

> _______________________________________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>

[L. David Baron]

On Monday 2015-07-06 23:25 -0400, Hubert Figuière wrote:

> On 06/07/15 11:12 PM, Jeff Gilbert wrote:
> > I propose that we stop recommending the universal use of an 'a' prefix for
> > arguments to functions in C and C++. If the prefix helps with
> > disambiguation, that's fine. However, use of this prefix should not be
> > prescribed in general.
> >
> > `aFoo` does not provide any additional safety that I know of.[1] As a
> > superfluous prefix, it adds visual noise, reducing immediate readability of
> > all function declarations and subsequent usage of the variables within the
> > function definition.
>

> [...]

>
> > I propose we strike the `aFoo` recommendation from the Mozilla style guide.
>

> I agree with this.
>
> However, if we proceed to remove this from the style guide, will we run
> a pass at removing the 'a' prefix or will we just let them be and have
> individual code change eventually clean them up?

It seems likely to be hard to automate, since there will be a
significant number of places where the obvious substitution (remove
"a" and lowercase the letter following it) yields name conflicts.

And I'm not sure how I'd feel about having inconsistent style for
years, if we can't automate it.

> Do we have -Wshadow (or equivalent) enabled across the code base and
> insurance that the warnings get dealt with?

No, we don't. I wish we did. I think it's among the more useful
warnings at finding real problems, but the JS engine and some other
pieces of code use a style that frequently uses shadowing.

(It was on many years ago, but we turned it off due to a gcc bug
(see bug 75621), C-only (not C++), that gave bogus shadowing
warnings for all variables with the same names as certain global
functions (|index|, etc.), that affected the JS engine in C. When
the JS engine switched from C to C++, I failed to remember to turn
it back on quickly, and then the JS engine, and later some other big
chunks of imported code (e.g., ipc), ended up using a style that's
very noisy with -Wshadow.)

Nick Nethercote did a good bit of work to try to get it on, but gave
up (see bug 800659).


I have -Wshadow turned on in my own builds, and have continuously
since we turned it off in general, and it's extremely noisy, but
also spits out a number of things that point to confusing code. The
noise basically means that compiler output as a whole is useless for
me (errors are very difficult to find, and I never look at warnings
at all), but I haven't been able to bring myself to give up
completely on it. (I effectively have all warnings disabled in the
hope that what I'm doing might one day help get -Wshadow turned back
on, even though I never actually do anything.)

It's possible that clang's -Wshadow might be easier to turn on that
gcc's, though (see
https://bugzilla.mozilla.org/show_bug.cgi?id=800659#c11 ).

-David

--
𝄞 L. David Baron http://dbaron.org/ 𝄂
𝄢 Mozilla https://www.mozilla.org/ 𝄂
Before I built a wall I'd ask to know
What I was walling in or walling out,
And to whom I was like to give offense.
- Robert Frost, Mending Wall (1914)

[Robert O'Callahan]

On Tue, Jul 7, 2015 at 3:26 PM, Mike Hommey <m...@glandium.org> wrote:

> The existence of aFoo goes along with the existence of mFoo, sFoo, kFoo,
> and others I might have forgotten. Not that I particularly care about
> aFoo, but why strike this one and not the others?[1]

FWIW many coding styles do have special style for class data members. I
think highlighting data members is more useful than parameters. In fact I
would love to get rid of aFoo but the last time I tried, the 'a' prefix had
considerable support.

Rob
--
lbir ye,ea yer.tnietoehr rdn rdsme,anea lurpr edna e hnysnenh hhe uresyf
toD
selthor stor edna siewaoeodm or v sstvr esBa kbvted,t
rdsme,aoreseoouoto
o l euetiuruewFa kbn e hnystoivateweh uresyf tulsa rehr rdm or rnea
lurpr
.a war hsrer holsa rodvted,t nenh hneireseoouot.tniesiewaoeivatewt sstvr
esn

[Karl Tomlinson]

Jeff Gilbert writes:

> we should have a good reason or
> two for making our choice. No such reason is detailed in the style guide.

I find the 'a' prefix useful to tell me that this variable has the
value that was provided to the function.
(I'm assuming that the prefix is used with this convention.)

There's no additional safety enforced, but I find the single
letter helps readability.

For example, if I want to know where the value of a variable comes
from, and it starts with 'a', then I know immediately that I can
skip looking at that particular function in the call chain/graph
and I need to look at the calling function(s).

That advantage does not exist in a declaration that is not a
definition; this is only helpful in the definition.

[Eric Rescorla]

I am in favor of getting rid of aFoo.

-Ekr

P.S. At the risk of convincing people I am crazy and thus discounting
my opinion above, I rather prefer foo_ to mFoo, but this seems like more
a matter of taste.


On Mon, Jul 6, 2015 at 8:55 PM, Robert O'Callahan <rob...@ocallahan.org>
wrote:

[Nicholas Nethercote]

I like 'aFoo'.

Nick

On Tue, Jul 7, 2015 at 1:12 PM, Jeff Gilbert <jgil...@mozilla.com> wrote:
> I propose that we stop recommending the universal use of an 'a' prefix for
> arguments to functions in C and C++. If the prefix helps with
> disambiguation, that's fine. However, use of this prefix should not be
> prescribed in general.
>
> `aFoo` does not provide any additional safety that I know of.[1] As a
> superfluous prefix, it adds visual noise, reducing immediate readability of
> all function declarations and subsequent usage of the variables within the
> function definition.
>

> number of our own projects), which means we should have a good reason or

> two for making our choice. No such reason is detailed in the style guide.
>

> I propose we strike the `aFoo` recommendation from the Mozilla style guide.
>

> -
>
> [1]: Maybe it prevents accidental shadowing? No: Either this isn't allowed
> by spec, or at least MSVC 2013 errors when compiling this.
>
> [2]: I do not mean this as an endorsement of the listed works and guides,
> but rather as illustration on how unusual our choice is.
>
> [3]: I created an Etherpad into which people are welcome to gather other
> works, projects, or style guides that I missed:
> https://etherpad.mozilla.org/6FcHs9mJYQ

[Xidorn Quan]

I agree with Karl that, the 'a' prefix sometimes helps me in that way
when I read code. Also it is sometimes convenient to have a local
variable use the parameter name without the prefix, like:
> SomeType foo = CastOrUnwrap(aFoo);

I don't have strong opinion on this, though. If the majority of the
industry disagrees with this style, probably we should change.

- Xidorn

[David Rajchenbach-Teller]

On 07/07/15 07:17, Eric Rescorla wrote:
> I am in favor of getting rid of aFoo.
>
> -Ekr
>
> P.S. At the risk of convincing people I am crazy and thus discounting
> my opinion above, I rather prefer foo_ to mFoo, but this seems like more
> a matter of taste.
>

I agree that `aFoo` is only useful very marginally, and rather ugly.

I also agree with Eric that `foo_` is somewhat nicer to read than
`mFoo`, which introduces a weird cAmelCase, but I can live with it. For
what it's worth, `this->foo` is also nice.

Cheers,
David

--
David Rajchenbach-Teller, PhD
Performance Team, Mozilla

[Honza Bambas]

I'm strongly against removing the prefix. I got used to this and it has
its meaning all the time I inspect code (even my own) and doing
reviews. Recognizing a variable is an argument is very very useful.
It's important to have it and it's good we enforce it!

-hb-

[Kartikaya Gupta]

I'd be interested to know: of those people who are in favour of
removing the prefix, how many regularly have to deal with functions
that are longer than two pages (a "page" is however much code you can
see at a time in your coding environment)? I'd be happy to support
removing the prefix if people also commit to splitting any giant
functions they touch as part of the prefix removal.

Also FWIW in the current world I do find the prefix useful when
debugging in gdb, because I know I can keep going up a frame as long
as I'm tracing a variable with the prefix, whereas otherwise I would
have to step backwards through each frame to see where the variable is
coming from. I'll probably find some way to adapt if we remove the
prefix though.

kats

[Jan Varga]

+1

[Honza Bambas]

On 7/7/2015 15:03, Kartikaya Gupta wrote:
> I'd be interested to know: of those people who are in favour of
> removing the prefix, how many regularly have to deal with functions
> that are longer than two pages (a "page" is however much code you can
> see at a time in your coding environment)?

All the time?

> I'd be happy to support
> removing the prefix if people also commit to splitting any giant
> functions they touch as part of the prefix removal.

That's (sorry) non-sense. In almost all cases longer methods/functions
cannot be just split. It would probably make the code just much harder
to read and maintain (with a lot of new arguments missing the 'a' prefix
;)) and is not necessary. Not an argument IMHO.

-hb-

>
> Also FWIW in the current world I do find the prefix useful when
> debugging in gdb, because I know I can keep going up a frame as long
> as I'm tracing a variable with the prefix, whereas otherwise I would
> have to step backwards through each frame to see where the variable is
> coming from. I'll probably find some way to adapt if we remove the
> prefix though.
>
> kats
>
>

[Gabriele Svelto]

On 07/07/2015 05:12, Jeff Gilbert wrote:
> Notable works or style guides [2] which do not recommend `aFoo`: [3]

[...]

To add another internal datapoint the FxOS gaia codebase is mostly
devoid of this style. There are some places using the m prefix for
pseudo member variables (really just JS attributes) but the a prefix for
arguments is quite rare, I could find only a hundred uses of it or so.

Gabriele

[Kartikaya Gupta]

On Tue, Jul 7, 2015 at 9:18 AM, Honza Bambas <hba...@mozilla.com> wrote:
>> I'd be happy to support
>> removing the prefix if people also commit to splitting any giant
>> functions they touch as part of the prefix removal.
>
>
> That's (sorry) non-sense. In almost all cases longer methods/functions
> cannot be just split. It would probably make the code just much harder to
> read and maintain (with a lot of new arguments missing the 'a' prefix ;))
> and is not necessary. Not an argument IMHO.

Can you point me to a couple of examples of long functions that you
think cannot be split reasonably? I'm curious to see what it looks
like. Obviously functions with giant switch statements and the like
are exceptions and should be treated exceptionally but I would like to
see some "regular" functions that can't be split.

Cheers,
kats

[Nick Fitzgerald]

(Posted this reply to the wrong thread, reposting to the right one... >_<)

One more group of defectors within Mozilla. From the DevTools coding
standards[0]:

"""

- aArguments aAre the aDevil (don't use them please)

"""

Although, there are still some files in tree with the legacy style.

[0] https://wiki.mozilla.org/DevTools/CodingStandards#Code_style

[Jeff Muizelaar]

FWIW, I did a quick poll of the people in our Gfx daily. Here are the results:

For aArguments:
Bas
Milan
Matt
Kats

Against aArguments:
Me

No strong opinion:
Sotoro
Lee
Benoit
Nical
Mason

-Jeff

[Boris Zbarsky]

On 7/7/15 11:36 AM, Jeff Muizelaar wrote:
> FWIW, I did a quick poll of the people in our Gfx daily. Here are the results:

To add some more split opinions to the situation, I rather like the
aArgument form precisely because it makes it easier to trace dataflow.
Though the fact that some functions assign to the aArgument does make it
harder.

On the other hand, the last time we had this conversation (it just keeps
happening, doesn't it?) roc pointed out that the aFoo convention makes
it harder to refactor things into helper functions (or out of them):
suddenly something that was a function local becomes an argument to the
helper, and you have to rename it throughout the helper function body.
I seem to recall that he also posited that this makes us less willing to
refactor things into smaller functions than we should be. I don't have
a good counterargument for this; I think he's right about this drawback
of the aFoo convention.

-Boris

[Mike Conley]

I suspect that knowing what things were passed into a method or function
is something that can be divined via static analysis.

Aren't there tools for our (admittedly varied) editors / IDEs to make
the readability that people are getting from aFoo readily available, but
that don't also require us to pack it into the actual name of the variable?

[Nathan Froyd]

On Tue, Jul 7, 2015 at 11:49 AM, Mike Conley <mco...@mozilla.com> wrote:

> Aren't there tools for our (admittedly varied) editors / IDEs to make
> the readability that people are getting from aFoo readily available, but
> that don't also require us to pack it into the actual name of the variable?

I find it useful for reading code in an editor, but I also find it useful
for reviewing patches when I don't have full context of the function.
Perhaps that will change with ReviewBoard and its ability to easily provide
context...or if ReviewBoard learned how to provide "this variable is a
(local|global|argument), set on line X" tooltips or something.

-Nathan

[Eric Rescorla]

On Tue, Jul 7, 2015 at 6:03 AM, Kartikaya Gupta <kgu...@mozilla.com> wrote:

> I'd be interested to know: of those people who are in favour of
> removing the prefix, how many regularly have to deal with functions
> that are longer than two pages (a "page" is however much code you can
> see at a time in your coding environment)?

All the time.

-Ekr

I'd be happy to support
> removing the prefix if people also commit to splitting any giant
> functions they touch as part of the prefix removal.
>

> >> Notable works or style guides [2] which do not recommend `aFoo`: [3]

[Gregory Szorc]

There are tentative plans to integrate MozReview with DXR. Specifically,
DXR and MozReview use the same code highlighting library (
http://pygments.org/). The goal is to develop a custom Pygments formatter
that annotates the source code in MozReview with DXR-extracted metadata
such as variable type, etc. This project is blocked on DXR indexing
MozReview pushes, which is blocked on DXR 2.0 / rolling out DXR's support
for indexing multiple revisions. Fortunately the latter is in progress. So
hopefully this can happen by EOY.

So, yeah, the end goal is for MozReview's code viewer to effectively be as
intelligent as your IDE.

I also have a pipe dream to enable code review to be conducted from IDEs.
Lots of people like to pull commits, run the code with breakpoints, etc as
part of code review. I think it would be rad if you could leave code review
feedback directly in the IDE and then submit the review to MozReview. This
would side-step the problem of DXR integration by offloading it to the IDE.
Unfortunately, not everyone uses the same IDE and the effort involved is
almost certainly greater. So DXR integration with MozReview will almost
certainly happen first.

[Eric Shepherd]

> Karl Tomlinson <mailto:moz...@karlt.net>
> July 7, 2015 at 12:55 AM

>
> I find the 'a' prefix useful to tell me that this variable has the
> value that was provided to the function.
> (I'm assuming that the prefix is used with this convention.)
>
> There's no additional safety enforced, but I find the single
> letter helps readability.
>
> For example, if I want to know where the value of a variable comes
> from, and it starts with 'a', then I know immediately that I can
> skip looking at that particular function in the call chain/graph
> and I need to look at the calling function(s).
>
> That advantage does not exist in a declaration that is not a
> definition; this is only helpful in the definition.

> _______________________________________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform

> ------------------------------------------------------------------------
FWIW, as a tech writer that has to take all this stuff and make sense of
it in documentation, I dislike "aFoo" but I'm fine with "mFood" and such
to differentiate methods from properties from constants, etc.

--

Eric Shepherd
Senior Technical Writer
Mozilla <https://www.mozilla.org/>
Blog: http://www.bitstampede.com/
Twitter: http://twitter.com/sheppy

[Boris Zbarsky]

On 7/7/15 11:49 AM, Mike Conley wrote:
> I suspect that knowing what things were passed into a method or function
> is something that can be divined via static analysis.
>
> Aren't there tools for our (admittedly varied) editors / IDEs

And debuggers. And dxr and blame views?

-Boris

[Honza Bambas]

IMO it's not good to be that dependent on viewers/editors when you can
get the info just from the var name.
-hb-

[Luke Wagner]

If we do unify Gecko/SpiderMonkey styles (something it seems like we're
moving towards and I think would be great), it would be a real shame to
switch 'cx' (a parameter to basically every function in SpiderMonkey) to
'aCx'; that would really make some eyes bleed. One compromise could be to
drop the 'a'-prefix requirement for 1- or 2-length parameter names, since
this is when it really looks silly. (But I'd prefer to drop the 'a' prefix
altogether.)

[Jeff Gilbert]

On Tue, Jul 7, 2015 at 4:54 AM, Honza Bambas <hba...@mozilla.com> wrote:

> I'm strongly against removing the prefix. I got used to this and it has
> its meaning all the time I inspect code (even my own) and doing reviews.
> Recognizing a variable is an argument is very very useful. It's important
> to have it and it's good we enforce it!
>
> -hb-

Please expand on this. I review a lot of code, and can't remember the last
time knowing a var was an arg was useful.
The only exception is 'out-vars', which I believe should be handled as a
different case. (WebGL code generally uses the out_ prefix)
`aFoo` does not discriminate, so it's impossible to tell if assignment to
`aFoo` is local or not without more context.

[Jeff Gilbert]

On Tue, Jul 7, 2015 at 6:03 AM, Kartikaya Gupta <kgu...@mozilla.com> wrote:

> I'd be interested to know: of those people who are in favour of
> removing the prefix, how many regularly have to deal with functions
> that are longer than two pages (a "page" is however much code you can

> see at a time in your coding environment)? I'd be happy to support

> removing the prefix if people also commit to splitting any giant
> functions they touch as part of the prefix removal.
>

I work with a number of these, but after a page or two, why is it at all
relevant which vars were args? For information flow? Should we mark locals
that purely derive from args as `aFoo` as well? Long functions (which have
poor readability anyway) generally have so much going on that the trivia of
which vars are args does not seem very useful..

I do not see how `aFoo` helps here, so please expand on this.

[Honza Bambas]

On 7/7/2015 21:27, Jeff Gilbert wrote:
> On Tue, Jul 7, 2015 at 4:54 AM, Honza Bambas <hba...@mozilla.com> wrote:
>
>> I'm strongly against removing the prefix. I got used to this and it has
>> its meaning all the time I inspect code (even my own) and doing reviews.
>> Recognizing a variable is an argument is very very useful. It's important
>> to have it and it's good we enforce it!
>>
>> -hb-
>
> Please expand on this.

Not sure how. I simply find it useful since I was once forced to obey
it strictly in a dom code. It simply has its meaning. It helps to
orient. I don't know what more you want from me to hear.

[Jeff Gilbert]

On Tue, Jul 7, 2015 at 12:36 PM, Honza Bambas <hba...@mozilla.com> wrote:

> On 7/7/2015 21:27, Jeff Gilbert wrote:
>
>> On Tue, Jul 7, 2015 at 4:54 AM, Honza Bambas <hba...@mozilla.com> wrote:
>>
>> I'm strongly against removing the prefix. I got used to this and it has
>>> its meaning all the time I inspect code (even my own) and doing reviews.
>>> Recognizing a variable is an argument is very very useful. It's
>>> important
>>> to have it and it's good we enforce it!
>>>
>>> -hb-
>>>
>>
>> Please expand on this.
>>
>
> Not sure how. I simply find it useful since I was once forced to obey it
> strictly in a dom code. It simply has its meaning. It helps to orient. I
> don't know what more you want from me to hear.
>

> I would like to have reasons why 'we' feel it's necessary or helpful when
the rest of the industry (and nearly half our own company) appears to do
fine without it. If we deviate from widespread standards, we should have
reasons to back our deviation.

More acutely, my module does not currently use `aFoo`, and our (few)
contributors do not use use or like it. `aFoo` gets in the way for us.
Recently, there has been pressure to unify the module's style with "the
rest" of Gecko. The main complaint I have with Gecko style is `aFoo` being
required.

Vague desires for `aFoo` are not compelling. There needs to be solid
reasons. If there are no compelling reasons, the requirement should be
removed. We have deprecated style before, and we can do it again.

[smaug]

As someone who spends more than 50% of working time doing reviews I'm strongly against this proposal.
aFoo helps with readability - reader knows immediately when the code is dealing with arguments.


-Olli

[Jeff Gilbert]

On Tue, Jul 7, 2015 at 1:03 PM, smaug <ope...@mozilla.com> wrote:

> As someone who spends more than 50% of working time doing reviews I'm
> strongly against this proposal.
> aFoo helps with readability - reader knows immediately when the code is
> dealing with arguments.
>

When and why is this useful to know?

[smaug]

readability / easier to follow the dataflow are rather compelling reasons.
I selfishly try to get the time I spend on reviewing a patch shorter, and aFoo helps with that.
Though even more important is consistent coding style everywhere (per programming language).

-Olli

[smaug]

Most common case in Gecko is to know that one is assigning value to outparam.

[Jet Villegas]

On Mon, Jul 6, 2015 at 8:12 PM, Jeff Gilbert <jgil...@mozilla.com> wrote:

>
> I propose we strike the `aFoo` recommendation from the Mozilla style guide.
>

Just so the proposal doesn't get lost in the bike shed, Jeff is only
proposing a change to the style guide, not a tree-wide find/replace
project. I take that to mean that "When in Rome" still applies to existing
C++ code.

Do we have consensus on that part?

--Jet

[Jeff Gilbert]

On Tue, Jul 7, 2015 at 1:20 PM, smaug <ope...@mozilla.com> wrote:

> readability / easier to follow the dataflow are rather compelling reasons.
>

It hurts readability for me and many others.
I don't see how it revolutionizes following dataflow, since we have locals
that are pure functions of args, but yet are not marked aFoo.

Outvars are a different beast, and in at least WebGL code, are marked as
such. (`out_` prefix)

`aFoo` is not a good solution for outvars. If outvars are the main reason
for `aFoo`, we should stop using `aFoo` for all arguments, and only mark
outvars.

I selfishly try to get the time I spend on reviewing a patch shorter, and
> aFoo helps with that.
>

It hinders my patch reviewing. I've been speaking to those around me, and
they do not see any value in differentiating args from locals. ("args are
just locals, anyway")

Though even more important is consistent coding style everywhere (per
> programming language).
>

Why don't we come into consistency with the industry at large, and also the
number of internal Mozilla projects which choose not to use `aFoo`.

I have found no other style guide that recommends `aFoo`. Why are we
different? Why do we accept reduced readability for all external
contributors? Why do so many other Mozilla projects not use this alleged
readability aid?

[Jeff Gilbert]

Outvars are good candidates for having markings in the variable name.
`aFoo` for all arguments is a poor solution for this, though.

[Nicholas Hurley]

On Mon, Jul 6, 2015 at 8:12 PM, Jeff Gilbert <jgil...@mozilla.com> wrote:

> I propose that we stop recommending the universal use of an 'a' prefix for
> arguments to functions in C and C++. If the prefix helps with
> disambiguation, that's fine. However, use of this prefix should not be
> prescribed in general.
>

I don't always respond in threads like this, but when I do, it's to say a
big +1 to no more aFoo.

[Milan Sreckovic]

Removing the style guide for “prefix function arguments with a” will not preclude people from naming a variable aFoo. At least the current style guide precludes people from naming non-function arguments that way, albeit indirectly.

I’m trying to understand the possible outcomes of this particular conversation:

a) Nothing happens. We leave a prefix in the style guide, some code ignores it, some follows it.
b) We change the style guide to remove the a prefix
1) We wholesale modify the code to remove the prefix, catching scenarios where we have a clash
2) We don’t do a wholesale modification
i) We get rid of a’s as we modify the code anyway
ii) We get rid of a’s one file at a time as we see fit
iii) We get rid of a’s one function at a time
c) We change the style guide to prohibit the a prefix
1) We wholesale modify the code to remove the prefix, catching scenarios where we have a clash
2) We don’t do a wholesale modification
i) We get rid of a’s as we modify the code anyway
ii) We get rid of a’s one file at a time as we see fit
iii) We get rid of a’s one function at a time

I can’t imagine the mess of any option that includes “1” and wholesale code modification, and if you remove those, the rest of the sort of start looking more or less the same.

I find a’s useful, but I’ve spent enough time in different codebases that I don’t think those types of things are ever worth the level of energy we expend on them. As long as we’re not adding _ in the variable names. That’s just wrong. ;)

—
- Milan



On Jul 7, 2015, at 16:33 , Jeff Gilbert <jgil...@mozilla.com> wrote:

> ...

>
> I have found no other style guide that recommends `aFoo`. Why are we
> different? Why do we accept reduced readability for all external
> contributors? Why do so many other Mozilla projects not use this alleged
> readability aid?

[smaug]

On 07/07/2015 11:45 PM, Milan Sreckovic wrote:
>
> Removing the style guide for “prefix function arguments with a” will not preclude people from naming a variable aFoo. At least the current style guide precludes people from naming non-function arguments that way, albeit indirectly.
>
> I’m trying to understand the possible outcomes of this particular conversation:
>
> a) Nothing happens. We leave a prefix in the style guide, some code ignores it, some follows it.

until the tools (and poiru) are run and make the code follow Mozilla coding style.

[Kartikaya Gupta]

The only concrete use case I have is what I said in my original post
(the paragraph you didn't quote): when debugging in gdb it's useful to
know if a variable came from the current stack frame or from a stack
frame further up.

[Mike Hommey]

On Tue, Jul 07, 2015 at 11:52:12PM +0300, smaug wrote:
> On 07/07/2015 11:45 PM, Milan Sreckovic wrote:
> >
> >Removing the style guide for “prefix function arguments with a” will not preclude people from naming a variable aFoo. At least the current style guide precludes people from naming non-function arguments that way, albeit indirectly.
> >
> >I’m trying to understand the possible outcomes of this particular conversation:
> >
> >a) Nothing happens. We leave a prefix in the style guide, some code ignores it, some follows it.
> until the tools (and poiru) are run and make the code follow Mozilla coding style.

Assuming you're talking about clang-format, that doesn't take care
about anything else than whitespaces.

Mike

[Bobby Holley]

On Tue, Jul 7, 2015 at 3:02 PM, Mike Hommey <m...@glandium.org> wrote:

> On Tue, Jul 07, 2015 at 11:52:12PM +0300, smaug wrote:

> > On 07/07/2015 11:45 PM, Milan Sreckovic wrote:
> > >
> > >Removing the style guide for “prefix function arguments with a” will
> not preclude people from naming a variable aFoo. At least the current
> style guide precludes people from naming non-function arguments that way,
> albeit indirectly.
> > >
> > >I’m trying to understand the possible outcomes of this particular
> conversation:
> > >
> > >a) Nothing happens. We leave a prefix in the style guide, some code
> ignores it, some follows it.
> > until the tools (and poiru) are run and make the code follow Mozilla
> coding style.
>

> Assuming you're talking about clang-format, that doesn't take care
> about anything else than whitespaces.
>

poiru built tools to automatically make all the adjustments that we
prescribe in the style guide, and has been iteratively running them on
various modules (thanks poiru!), so we should assume that whatever style we
pick for the style guide will end up applying to all/most of our code in
the not-too-distant future.

This is a good thing - specifying things in the style guide but keeping the
tree inconsistent on the grounds of "when in Rome" is the wrong approach.
This was discussed to death in various threads already, and the conclusion
is that we are actively moving towards a unified style in Gecko and SM.

bholley

[Birunthan Mohanathas]

On 7 July 2015 at 15:02, Mike Hommey <m...@glandium.org> wrote:
> On Tue, Jul 07, 2015 at 11:52:12PM +0300, smaug wrote:

>> until the tools (and poiru) are run and make the code follow Mozilla coding style.
>

> Assuming you're talking about clang-format, that doesn't take care
> about anything else than whitespaces.

I have a Clang tool to add the 'a' prefix, but it can be easily
modified to drop the prefix should we decide to do so.

[Eric Rahm]

I'm not a huge fan of the 'aFoo' style, but I am a huge fan of consistency. So if we want to change the style guide we should update our codebase, and I don't think we can reasonably do that automatically without introducing shadowing issues.

Additionally I don't spend 50% of my time reviewing, so I'd say my opinion here (meh to aFoo) is less important. It's not an undue burden for me to include an aPrefix and if we have static analysis to check for it that would make it even less of an issue.

re: refactoring, I suppose that could be an argument against the aPrefix, but then again various IDE's make this a bit easier (including Eclipse, which I believe roc is a user of).

-e

[Bobby Holley]

On Tue, Jul 7, 2015 at 3:59 PM, Eric Rahm <er...@mozilla.com> wrote:

> I'm not a huge fan of the 'aFoo' style, but I am a huge fan of
> consistency. So if we want to change the style guide we should update our
> codebase, and I don't think we can reasonably do that automatically without
> introducing shadowing issues.
>

This a great point, and perhaps the most useful one to be raised in this
thread.

The priority is to automatically rewrite our source with a unified style.
foo -> aFoo is reasonably safe, whereas aFoo->foo is not, at least with the
current tools. So we either need to combine the rewrite tools with static
analysis, or just go with aFoo.

[Xidorn Quan]

On Tue, Jul 7, 2015 at 1:12 PM, Jeff Gilbert <jgil...@mozilla.com> wrote:

>
> Notable works or style guides which *do* recommend `aFoo`:
> * Mozilla (except for IDL, Java, and Python)
> * ?
>

Just FYI, Someone in Twitter mentioned that, code generated by Xcode
uses this style by default. The languages are Obj-C and Swift, though.

Examples like:
https://github.com/search?l=objective-c&q=aNotification&ref=searchresults&type=Code&utf8=

- Xidorn

[Gregory Szorc]

On Tue, Jul 7, 2015 at 1:03 PM, smaug <sm...@welho.com> wrote:

> As someone who spends more than 50% of working time doing reviews I'm
> strongly against this proposal.
> aFoo helps with readability - reader knows immediately when the code is
> dealing with arguments.
>
>

I'd like to point out that MozReview allows you to expand context when
doing code reviews. This means the necessity of extra "hints" in naming
conventions (such as aFoo) loses its importance. The argument that aFoo
assists with readability, while historically accurate when applied to
review tools such as Splinter that rely on patch/diff context, is thus
somewhat undermined by the employment of modern code review tool.

[Jeff Gilbert]

On Tue, Jul 7, 2015 at 3:59 PM, Eric Rahm <er...@mozilla.com> wrote:

> I'm not a huge fan of the 'aFoo' style, but I am a huge fan of
> consistency. So if we want to change the style guide we should update our
> codebase, and I don't think we can reasonably do that automatically without
> introducing shadowing issues.
>

MSVC 2013 (which I believe is our main windows compiler right now) will
error during compilation if such a shadowing issue arises. Thus, if the
code compiles there, `aFoo`->`foo` is safe. I would be very surprised if
GCC or Clang didn't have an equivalent option.

> Additionally I don't spend 50% of my time reviewing, so I'd say my opinion
> here (meh to aFoo) is less important. It's not an undue burden for me to
> include an aPrefix and if we have static analysis to check for it that
> would make it even less of an issue.
>

It can be a burden on the hundreds of devs who have to read and understand
the code in order to write more code. With the exception of a couple
people, review is not the bottleneck. The opinions of a few over-harried
reviewers should not hold undue sway over the many many devs writing code.

[Mike Hommey]

On Tue, Jul 07, 2015 at 05:09:57PM -0700, Gregory Szorc wrote:
> On Tue, Jul 7, 2015 at 1:03 PM, smaug <sm...@welho.com> wrote:
>

> > As someone who spends more than 50% of working time doing reviews I'm
> > strongly against this proposal.
> > aFoo helps with readability - reader knows immediately when the code is
> > dealing with arguments.
> >
> >

> I'd like to point out that MozReview allows you to expand context when
> doing code reviews. This means the necessity of extra "hints" in naming
> conventions (such as aFoo) loses its importance. The argument that aFoo
> assists with readability, while historically accurate when applied to
> review tools such as Splinter that rely on patch/diff context, is thus
> somewhat undermined by the employment of modern code review tool.

While mozreview helps to some degree, searching for the function
declaration is not really something that is made easier by the
context expansion feature it has.

Mike

[Karl Tomlinson]

Jeff Gilbert writes:

> It can be a burden on the hundreds of devs who have to read and understand
> the code in order to write more code.

Some people find the prefix helps readability, because it makes
extra information immediately available in the code being
examined, while you are indicating that this is a significant
burden on readability.

Can you explain why the extra letter is a significant burden?

If the 'a' prefix is a burden then the 'm' prefix must be also,
and so we should be using this->member instead of mMember.

> The opinions of a few over-harried
> reviewers should not hold undue sway over the many many devs writing code.

unless people want code to be reviewed.

[Nicholas Nethercote]

On Tue, Jul 7, 2015 at 5:13 PM, Jeff Gilbert <jgil...@mozilla.com> wrote:
>
> MSVC 2013 (which I believe is our main windows compiler right now) will
> error during compilation if such a shadowing issue arises. Thus, if the
> code compiles there, `aFoo`->`foo` is safe. I would be very surprised if
> GCC or Clang didn't have an equivalent option.

They do, and dbaron mentioned them earlier. They're currently not on
because there are quite a lot of warnings because there is quite a lot
of shadowing occurring. (I have some experience with this thanks to
bug 800659.) So I'm surprised by your claim that this is a non-issue.

> It can be a burden on the hundreds of devs who have to read and understand

> the code in order to write more code. With the exception of a couple

> people, review is not the bottleneck. The opinions of a few over-harried

> reviewers should not hold undue sway over the many many devs writing code.

Reviewing code and reading code aren't that different, so let's not
get hung up on the differences there.

Some people like the aFoo style and find that it helps readability.
It's largely a matter of personal taste; you can't just dismiss these
people as being wrong.

Nick

[Milan Sreckovic]

Jeff encouraged me to add more things to this thread, so I’m blaming him. So, some random thoughts.

After getting paid to write code for 20+ years and then showing up at Mozilla, and seeing the a prefix, I thought “this is brilliant, how come we didn’t think of doing that before?!”, as a reasonable balance between nothing and the insanity of the full Hungarian.

I find a prefix useful when I’m writing code and when I’m reading it.

I have no trouble reading the code that isn’t using this convention. I don’t think I ran into a situation where only some of the arguments in the function were using the prefix (and some were not), but I can imagine that being the only situation where I’d argue that it’s confusing.

In other words, as weird as it may sound, I find the prefix improving the readability, but the lack of it not hindering it. And it makes no difference to me when I’m reviewing code, which is a couple of orders of magnitude fewer times than for most people on this thread.

If I was writing a new file from scratch, I’d use this convention. If I was in a file that wasn’t using it, it wouldn’t bother me.

I think it would be a bad idea to force this consistency on the whole codebase (e.g., either clear it out, or put it everywhere), as I don’t think it would actually solve anything. The “consistent is good” can be taken too far, and I think this would be taking it too far.

I honestly think the best thing to do here is nothing - remove it from the style guide if we don’t want to enforce it, but don’t stop me from using it.

Blame Jeff for the above.

—
- Milan

[Karl Tomlinson]

Jeff Gilbert writes:

> I work with a number of these, but after a page or two, why is it at all
> relevant which vars were args? For information flow? Should we mark locals
> that purely derive from args as `aFoo` as well? Long functions (which have
> poor readability anyway) generally have so much going on that the trivia of
> which vars are args does not seem very useful..
>
> I do not see how `aFoo` helps here, so please expand on this.

A simple variable name, such as "font" for example, may identify
any of a number of fonts. Such simple names, without any
qualifier in the name, are often used in loops, for example,
because it is the most important font in the immediate context.

However a simple variable may also be used in a parameter list
because when looking at the parameter list it is obvious which
font is relevant in the interface.

That means that if "font" is seen in the body of a function, the
first question that arises is "which font?"

If the variable is called "aFont" then we know which font because
we know what function we are in.

[Gregory Szorc]

On Tue, Jul 7, 2015 at 5:13 PM, Jeff Gilbert <jgil...@mozilla.com> wrote:

> On Tue, Jul 7, 2015 at 3:59 PM, Eric Rahm <er...@mozilla.com> wrote:
>

> > I'm not a huge fan of the 'aFoo' style, but I am a huge fan of
> > consistency. So if we want to change the style guide we should update our
> > codebase, and I don't think we can reasonably do that automatically
> without
> > introducing shadowing issues.
> >
>

> MSVC 2013 (which I believe is our main windows compiler right now) will
> error during compilation if such a shadowing issue arises. Thus, if the
> code compiles there, `aFoo`->`foo` is safe. I would be very surprised if
> GCC or Clang didn't have an equivalent option.
>
>

> > Additionally I don't spend 50% of my time reviewing, so I'd say my
> opinion
> > here (meh to aFoo) is less important. It's not an undue burden for me to
> > include an aPrefix and if we have static analysis to check for it that
> > would make it even less of an issue.
> >
>

> It can be a burden on the hundreds of devs who have to read and understand

> the code in order to write more code. With the exception of a couple

> people, review is not the bottleneck. The opinions of a few over-harried

> reviewers should not hold undue sway over the many many devs writing code.
>

I somewhat disagree. There will always be fewer code reviewers than
contributors. And, code reviewers tend to be more senior people. The time
of a code reviewer thus tends to be more valuable than the time of the
average code author. Coupled with the fact that code review is a barrier to
landing, this translates to an incentive to make the lives and workflows of
code reviewers as frictionless as possible.

I feel strongly that the bandwidth limitations of code reviewers does
dictate to some extent how code is written. For example, I feel that
authors should spend extra effort to write detailed commit messages and
split work into multiple, easier-to-review commits, as these can
drastically reduce the time it takes for review. How much this reasoning
extends to style and things like aFoo, I'm not sure. But if I hear a
frequent code reviewer say "X makes review easier," I tend to take that
opinion more seriously than that of a non-reviewer.

[David Anderson]

+1 for removing this. Gecko's use is inconsistent, and outside of Gecko code that does use it, I've never seen it used in any other codebase. I've never gone to another project and thought, "I miss decorating everything in a way that changes capitalization and impairs canonical naming".

Reasons for using it in the first place are suspect. None of them seem to justify the extra developer overhead or the odd variable names that result. I can't imagine we've solved some massive readability problem unique to Gecko or unsolved by other projects, or that we're catching important problems that static analysis can't find.

[Jeff Gilbert]

On Tue, Jul 7, 2015 at 5:41 PM, Karl Tomlinson <moz...@karlt.net> wrote:

> Jeff Gilbert writes:
>
> > It can be a burden on the hundreds of devs who have to read and
> understand
> > the code in order to write more code.
>
> Some people find the prefix helps readability, because it makes
> extra information immediately available in the code being
> examined, while you are indicating that this is a significant
> burden on readability.
>
> Can you explain why the extra letter is a significant burden?
>

Because extra noise is being forced into variable names for minimal
benefit. Every declaration is a sea of extra 'a's. Refactoring code means
doing a lot of s/aFoo/foo/ and vice-versa. Reading each arg name requires
first passing over 'a' before getting to anything relevant. Often this
means that short function bodies can have every fifth or sixth letter being
'a'.

Why does it matter that they're arguments? Arguments are just locals that
are passed into functions. (outparams are a different beast already
addressed elsewhere)

I would like to reiterate that *we are unusual* in our present preference
(not to mention requirement) for this in our style guideline. I'm not
proposing a change to something bizarre. I'm proposing the removal of an
extremely unusual style requirement.

If the 'a' prefix is a burden then the 'm' prefix must be also,
> and so we should be using this->member instead of mMember.
>

No, as this provides huge benefit by indicating when we are referencing a
member variable, which differ greatly from local variables in scope.
Arguments have the same scope as locals. There is benefit in mFoo,
particularly compared to requiring this->foo, which I don't think we even
have compiler or linter support for, and would clearly be superfluous in
terms of extra characters.

>
> > The opinions of a few over-harried
> > reviewers should not hold undue sway over the many many devs writing
> code.
>
> unless people want code to be reviewed.
>

`aFoo` is never going to make or break our ability to do code review. Low
code-review bandwidth is all but completely orthogonal to this discussion.

[Jeff Gilbert]

If it's named well, there should be no question which it refers to, with or
without argument decoration.

>
> If the variable is called "aFont" then we know which font because
> we know what function we are in.
>

Use aFont if it helps, just as we use iFoo and fFoo sometimes when doing
conversions.
Don't require it though.
In particular, `newSize` is better than `aSize` when also dealing with
mSize. Inferring the meaning of a variable from its status as an argument
is a crutch for poor variable naming. (and adds mental load)

[Karl Tomlinson]

Jeff Gilbert writes:

> On Tue, Jul 7, 2015 at 5:41 PM, Karl Tomlinson <moz...@karlt.net> wrote:
>
>> Some people find the prefix helps readability, because it makes
>> extra information immediately available in the code being
>> examined, while you are indicating that this is a significant
>> burden on readability.
>>
>> Can you explain why the extra letter is a significant burden?
>
> Because extra noise is being forced into variable names for minimal
> benefit. Every declaration is a sea of extra 'a's. Refactoring code means
> doing a lot of s/aFoo/foo/ and vice-versa. Reading each arg name requires
> first passing over 'a' before getting to anything relevant. Often this
> means that short function bodies can have every fifth or sixth letter being
> 'a'.

I wouldn't see a problem with removing the 'a' prefix from
parameter names in declarations and inline methods.

[Jeff Gilbert]

On Mon, Jul 6, 2015 at 8:26 PM, Mike Hommey <m...@glandium.org> wrote:

> The existence of aFoo goes along with the existence of mFoo, sFoo, kFoo,
> and others I might have forgotten. Not that I particularly care about
> aFoo, but why strike this one and not the others?[1] It's not like they
> have widespread use in the industry either. That is, the same reasoning
> could be applied to them, yet, you're stopping at aFoo. Why?


mFoo and sFoo have very different scopes compared to locals, so calling
them out is useful.
kFoo makes it clear that the variable is constant, and has connotations
regarding it being a hardcoded limit or value.
Note that commonly kFoo is actually a static constant. Immutable (`const`)
locals are often not kPrefixed.

[Justin Dolske]

On 7/7/15 6:17 PM, David Anderson wrote:
> +1 for removing this. Gecko's use is inconsistent, and outside of
> Gecko code that does use it, I've never seen it used in any other
> codebase. I've never gone to another project and thought, "I miss
> decorating everything in a way that changes capitalization and
> impairs canonical naming".

I don't care strongly about the naming, but this is an important point.
Jeff's original post was explicit about it too, yet it keeps being
overlooked.

Whenever style changes are proposed, there's a lot of of knee-jerk
resistance ("omgchange!"). It's natural. But the relative anchor point
here is that the rest of the world manages to do just fine without it.
And so any stylistic benefits of "aFoo" feel, well, overstated.

[This is a problem endemic to Mozilla, beyond simple code style. We get
stuck in ruts and local maxima, and become inflexible to doing things
differently from how they've always been done.]

Personally, for these kinds of issues, I find it useful to think about
if we would _add_ the thing if it didn't already exist. And if we
wouldn't, we should strongly bias towards change unless the costs are
_clearly_ not worth it.

Justin

[Gabor Krizsanits]

On Wed, Jul 8, 2015 at 1:05 AM, Bobby Holley <bobby...@gmail.com> wrote:

> On Tue, Jul 7, 2015 at 3:59 PM, Eric Rahm <er...@mozilla.com> wrote:
>

> > I'm not a huge fan of the 'aFoo' style, but I am a huge fan of
> > consistency. So if we want to change the style guide we should update our
> > codebase, and I don't think we can reasonably do that automatically
> without
> > introducing shadowing issues.
> >
>

> This a great point, and perhaps the most useful one to be raised in this
> thread.
>
> The priority is to automatically rewrite our source with a unified style.
> foo -> aFoo is reasonably safe, whereas aFoo->foo is not, at least with the
> current tools. So we either need to combine the rewrite tools with static
> analysis, or just go with aFoo.

> _______________________________________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>

+1 for consistency. Any volunteers who is willing to get rid of aFoo
EVERYWHERE
and someone else who is willing to review that work? If not then we should
do it
the other way around.

[Nathan Froyd]

On Wed, Jul 8, 2015 at 6:52 AM, Gabor Krizsanits <gkriz...@mozilla.com>
wrote:

> On Wed, Jul 8, 2015 at 1:05 AM, Bobby Holley <bobby...@gmail.com>
> wrote:
> > The priority is to automatically rewrite our source with a unified style.
> > foo -> aFoo is reasonably safe, whereas aFoo->foo is not, at least with
> the
> > current tools. So we either need to combine the rewrite tools with static
> > analysis, or just go with aFoo.

> > _______________________________________________
> > dev-platform mailing list
> > dev-pl...@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-platform
>

> +1 for consistency. Any volunteers who is willing to get rid of aFoo
> EVERYWHERE
> and someone else who is willing to review that work? If not then we should
> do it
> the other way around.
>

If somebody is willing to do the formatting, I'm willing to do the review.
I think the thread has reached the point of people repeating ad nauseum
what was already said earlier in the thread, so it's time for a decision.
Benjamin?

-Nathan

[smaug]

On 07/08/2015 04:05 AM, Milan Sreckovic wrote:
>
> Jeff encouraged me to add more things to this thread, so I’m blaming him. So, some random thoughts.
>
> After getting paid to write code for 20+ years and then showing up at Mozilla, and seeing the a prefix, I thought “this is brilliant, how come we
> didn’t think of doing that before?!”, as a reasonable balance between nothing and the insanity of the full Hungarian.
>
> I find a prefix useful when I’m writing code and when I’m reading it.
>
> I have no trouble reading the code that isn’t using this convention. I don’t think I ran into a situation where only some of the arguments in the
> function were using the prefix (and some were not), but I can imagine that being the only situation where I’d argue that it’s confusing.
>
> In other words, as weird as it may sound, I find the prefix improving the readability, but the lack of it not hindering it. And it makes no
> difference to me when I’m reviewing code, which is a couple of orders of magnitude fewer times than for most people on this thread.
>
> If I was writing a new file from scratch, I’d use this convention. If I was in a file that wasn’t using it, it wouldn’t bother me.
>
> I think it would be a bad idea to force this consistency on the whole codebase (e.g., either clear it out, or put it everywhere), as I don’t think
> it would actually solve anything. The “consistent is good” can be taken too far, and I think this would be taking it too far.
>
> I honestly think the best thing to do here is nothing - remove it from the style guide if we don’t want to enforce it, but don’t stop me from using
> it.

Removing it from the coding style, yet allowing it to be used would be the worst case. Whatever we do, better do it consistently.

[smaug]

Do you actually have any data how many % of Gecko devs would prefer not using aFoo?
I mean it makes no sense to change to foo, if most of the devs prefer aFoo.
Similarly I would stop objecting the change if majority of the devs say
"yes, please change this coding style which Mozilla has had forever[1]."
(it might mean me doing reviews a bit slower, which tends to lead fewer review requests, which might not be such a bad thing ;) )

Right now it feels like there are couple of devs in favor of aFoo, and couple of devs in favor of foo, and the rest
haven't said anything.



-Olli


[1] Note, the coding style has been there for a long time, but not followed in all the modules for some reason.


On 07/07/2015 06:12 AM, Jeff Gilbert wrote:
> I propose that we stop recommending the universal use of an 'a' prefix for
> arguments to functions in C and C++. If the prefix helps with
> disambiguation, that's fine. However, use of this prefix should not be
> prescribed in general.
>
> `aFoo` does not provide any additional safety that I know of.[1] As a
> superfluous prefix, it adds visual noise, reducing immediate readability of
> all function declarations and subsequent usage of the variables within the
> function definition.
>
> Notable works or style guides [2] which do not recommend `aFoo`: [3]
> * Google
> * Linux Kernel
> * Bjarne Stroustrup
> * GCC
> * LLVM
> * Java Style (Java, non-C)
> * PEP 0008 (Python, non-C)
> * FreeBSD
> * Unreal Engine
> * Unity3D (largely C#)
> * Spidermonkey
> * Daala
> * RR
> * Rust
> * Folly (from Facebook)
> * C++ STL entrypoints
> * IDL for web specs on W3C and WhatWG
> * etc.

>
> Notable works or style guides which *do* recommend `aFoo`:
> * Mozilla (except for IDL, Java, and Python)
> * ?
>

> 3rd-party projects in our tree which do not use `aFoo`:
> * Cairo
> * Skia
> * ANGLE
> * HarfBuzz
> * ICU
> * Chromium IPC
> * everything under modules/ that isn't an nsFoo.c/cpp/h
> * etc.?
>
> 3rd-party projects in our tree which *do* recommend `aFoo`:
> * ?
>
> As far as I can tell, the entire industry disagrees with us (as well as a
> number of our own projects), which means we should have a good reason or
> two for making our choice. No such reason is detailed in the style guide.
>
> I propose we strike the `aFoo` recommendation from the Mozilla style guide.
>
> -
>
> [1]: Maybe it prevents accidental shadowing? No: Either this isn't allowed
> by spec, or at least MSVC 2013 errors when compiling this.
>
> [2]: I do not mean this as an endorsement of the listed works and guides,
> but rather as illustration on how unusual our choice is.
>
> [3]: I created an Etherpad into which people are welcome to gather other
> works, projects, or style guides that I missed:
> https://etherpad.mozilla.org/6FcHs9mJYQ
>

[smaug]

On 07/07/2015 11:34 PM, Jeff Gilbert wrote:
> Outvars are good candidates for having markings in the variable name.
> `aFoo` for all arguments is a poor solution for this, though.
>
> On Tue, Jul 7, 2015 at 1:22 PM, smaug <ope...@mozilla.com> wrote:
>
>> On 07/07/2015 11:18 PM, Jeff Gilbert wrote:

>>
>>> On Tue, Jul 7, 2015 at 1:03 PM, smaug <ope...@mozilla.com> wrote:
>>>
>>> As someone who spends more than 50% of working time doing reviews I'm
>>>> strongly against this proposal.
>>>> aFoo helps with readability - reader knows immediately when the code is
>>>> dealing with arguments.
>>>>
>>>>

>>> When and why is this useful to know?
>>>
>>>
>>
>> Most common case in Gecko is to know that one is assigning value to
>> outparam.
>>



Another example where aFoo tends to be rather useful is lifetime management.
If I see aFoo being used somewhere in a method after some unsafe method call
(layout flush, any script callback handling, event dispatch, observer service notification etc.),
I know that I need to check that the _caller_ follows COM-rules and keeps aFoo object alive during the method call.
With non-aFoo variables I know the lifetime is controlled within the method.

[Bobby Holley]

On Wed, Jul 8, 2015 at 3:52 AM, Gabor Krizsanits <gkriz...@mozilla.com>
wrote:

> The priority is to automatically rewrite our source with a unified style.
>> foo -> aFoo is reasonably safe, whereas aFoo->foo is not, at least with
>> the
>> current tools. So we either need to combine the rewrite tools with static
>> analysis, or just go with aFoo.

>> _______________________________________________
>> dev-platform mailing list
>> dev-pl...@lists.mozilla.org
>> https://lists.mozilla.org/listinfo/dev-platform
>>
>

> +1 for consistency. Any volunteers who is willing to get rid of aFoo
> EVERYWHERE
> and someone else who is willing to review that work? If not then we should
> do it
> the other way around.
>

At the risk of mostly repeating myself, I want to emphasize once more that:
(1) We have already made the decision (in other threads) that consistency
trumps everything else.
(2) We will not get consistency on this issue without an automated tool.
(3) We do not, at present, have an automated tool that will safely take us
from aFoo -> foo, but we do have the reverse.

It seems like the most viable approach to leveling the playing field in (3)
is to have somebody spend the time to (a) get -Wshadow working, and (b)
commit to fixing all the new shadowing issues generating by an automated
aFoo -> foo conversions. Until we have a volunteer to do that in the near
term, switching aFoo -> foo basically isn't on the table.

I think this issue needs to be discussed before making any more stylistic
arguments in favor of abolishing aFoo.

bholley

[Honza Bambas]

On 7/8/2015 3:08, Gregory Szorc wrote:

> On Tue, Jul 7, 2015 at 5:13 PM, Jeff Gilbert <jgil...@mozilla.com> wrote:
>
>> On Tue, Jul 7, 2015 at 3:59 PM, Eric Rahm <er...@mozilla.com> wrote:
>>

>>> I'm not a huge fan of the 'aFoo' style, but I am a huge fan of
>>> consistency. So if we want to change the style guide we should update our
>>> codebase, and I don't think we can reasonably do that automatically
>> without
>>> introducing shadowing issues.
>>>

>> MSVC 2013 (which I believe is our main windows compiler right now) will
>> error during compilation if such a shadowing issue arises. Thus, if the
>> code compiles there, `aFoo`->`foo` is safe. I would be very surprised if
>> GCC or Clang didn't have an equivalent option.
>>
>>

>>> Additionally I don't spend 50% of my time reviewing, so I'd say my
>> opinion
>>> here (meh to aFoo) is less important. It's not an undue burden for me to
>>> include an aPrefix and if we have static analysis to check for it that
>>> would make it even less of an issue.
>>>

>> It can be a burden on the hundreds of devs who have to read and understand

>> the code in order to write more code. With the exception of a couple

>> people, review is not the bottleneck. The opinions of a few over-harried

>> reviewers should not hold undue sway over the many many devs writing code.
>>

> I somewhat disagree. There will always be fewer code reviewers than
> contributors. And, code reviewers tend to be more senior people. The time
> of a code reviewer thus tends to be more valuable than the time of the
> average code author. Coupled with the fact that code review is a barrier to
> landing, this translates to an incentive to make the lives and workflows of
> code reviewers as frictionless as possible.
>
> I feel strongly that the bandwidth limitations of code reviewers does
> dictate to some extent how code is written. For example, I feel that
> authors should spend extra effort to write detailed commit messages and
> split work into multiple, easier-to-review commits, as these can
> drastically reduce the time it takes for review. How much this reasoning
> extends to style and things like aFoo, I'm not sure. But if I hear a
> frequent code reviewer say "X makes review easier," I tend to take that
> opinion more seriously than that of a non-reviewer.

+1

Few more cents: A year ago I was not using "a" in my code. Then I start
using it. And I got used to it! And - when used consistently - find it
useful from time to time to quickly get the "this is an arg" variable
info, both as a reviewer and a coder.

So, year ago I would probably be on the "get rid of it" side. Now I'm
definitely on the "use it everywhere" side.

Cheers

-hb-

[Karl Tomlinson]

Bobby Holley writes:

> On Wed, Jul 8, 2015 at 3:52 AM, Gabor Krizsanits <gkriz...@mozilla.com>
> wrote:
>
>> The priority is to automatically rewrite our source with a unified style.
>>> foo -> aFoo is reasonably safe, whereas aFoo->foo is not, at least with
>>> the
>>> current tools. So we either need to combine the rewrite tools with static
>>> analysis, or just go with aFoo.
>>> _______________________________________________
>>> dev-platform mailing list
>>> dev-pl...@lists.mozilla.org
>>> https://lists.mozilla.org/listinfo/dev-platform
>>>
>>
>> +1 for consistency.
>

> At the risk of mostly repeating myself, I want to emphasize once more that:
> (1) We have already made the decision (in other threads) that consistency
> trumps everything else.

If I hear correctly, it sounds like Jeff is arguing that it is not
always helpful to know that a variable is a parameter, and so it
shouldn't be a requirement that parameters are always named to
indicate that they are a parameter.

If I were to always use "firstObject" for the name of the first
object of immediate relevance, then this would be consistent
naming.

If there is only one object, then "firstObject" would still be a
consistent name, but I wouldn't say that using the name "object"
is inconsistent.

When it is helpful to know that a variable is a parameter then
consistency in the convention to identify parameters is helpful.
If it is not necessary to indicate that the variable is a
parameter, then it doesn't matter.

If we choose the conversion to remove existing 'a' prefixes, then
we would need to either automatically move to a different naming
convention that clearly identifies the purposes of the variables
(out params as non-const references, and maybe pointers may be
specially indicated) or manually go through and work out what
names to give the variables that would benefit from clear
identification. The scope of the manual process of course makes
this impractical.

I think we could relax the 'a' prefix requirement to be a
convention used when identifying the variable as a parameter is
useful. My opinion is that this is useful for most parameters in
all non-trivial functions, but others disagree.

[Bobby Holley]

On Wed, Jul 8, 2015 at 4:45 PM, Karl Tomlinson <moz...@karlt.net> wrote:

> I think we could relax the 'a' prefix requirement to be a
> convention used when identifying the variable as a parameter is
> useful. My opinion is that this is useful for most parameters in
> all non-trivial functions, but others disagree.
>

I don't think that helps.

One of the primary benefits of style rules is that they eliminate (or
rather, reduce) the uncertainty and churn that comes from different
reviewers having different preferences.

Suppose I add a new method signature in a patch. If Alice finds a-prefixing
useful and Bob does not, my likelihood of receiving a review nit depends on
who's reviewing it - i.e. per-reviewer style guides, which are strictly
worse than per-module style guides, which are strictly worse than a single
style guide.

bholley

[Karl Tomlinson]

Yes, I think you are right.
That would be a likely unfortunate consequence if there is not a
simple-enough set of rules to distinguish.
A complicated set of rules is more trouble than it is worth to
save a few letters IMO.

[Jeff Gilbert]

On Wed, Jul 8, 2015 at 7:48 AM, smaug <ope...@mozilla.com> wrote:

> Another example where aFoo tends to be rather useful is lifetime
> management.
> If I see aFoo being used somewhere in a method after some unsafe method
> call
> (layout flush, any script callback handling, event dispatch, observer
> service notification etc.),
> I know that I need to check that the _caller_ follows COM-rules and keeps
> aFoo object alive during the method call.
> With non-aFoo variables I know the lifetime is controlled within the
> method.
>

Arguments have the same lifetimes as locals, and the only exceptions to
this apply to both args and locals. (references and pointers)
There are the same problems with locals as there are with args, as args are
just locals passed into the function.

FWIW, I hacked a python script that does s/aF[Oo]o/foo/, and MSVC does has
a code analysis mode that supports an error-on-warn for shadowed variable
names.

I can take a look at how bad the shadowing is in the tree.

[Bobby Holley]

On Thu, Jul 9, 2015 at 7:01 PM, Jeff Gilbert <jgil...@mozilla.com> wrote:

> FWIW, I hacked a python script that does s/aF[Oo]o/foo/

poiru has the tools to do this already, as mentioned above.

> , and MSVC does has
> a code analysis mode that supports an error-on-warn for shadowed variable
> names.
>
> I can take a look at how bad the shadowing is in the tree.
>

Probably worth taking a look at bug 563195.

bholley

[Thomas Zimmermann]

Am 08.07.2015 um 16:36 schrieb smaug:
> Do you actually have any data how many % of Gecko devs would prefer
> not using aFoo?

I strongly prefer 'aFoo' over 'foo' for the extra context that it gives
to the variable. If we want to change anything we should rather
introduce a separate prefix for output parameters.

Best regards
Thomas

[Nicholas Nethercote]

On Thu, Jul 9, 2015 at 7:01 PM, Jeff Gilbert <jgil...@mozilla.com> wrote:
>

> Arguments have the same lifetimes as locals, and the only exceptions to
> this apply to both args and locals. (references and pointers)

Maybe I've misunderstood what you're saying here, but locals are
frequently block-scoped which gives them a different lifetime to args.

Nick

[Jeff Gilbert]

Right, args have lifetimes of function-block-scope locals.

[Jeff Gilbert]

On Sun, Jul 12, 2015 at 6:45 PM, Thomas Zimmermann <tzimm...@mozilla.com>
wrote:

> Am 08.07.2015 um 16:36 schrieb smaug:

> > Do you actually have any data how many % of Gecko devs would prefer
> > not using aFoo?
>

> I strongly prefer 'aFoo' over 'foo' for the extra context that it gives
> to the variable. If we want to change anything we should rather
> introduce a separate prefix for output parameters.
>

Which part of this extra context is useful?

[Kearwood "Kip" Gilbert]

I would defer to the expert on the subject:

https://imgflip.com/i/o5r8m

- Kip

On 2015-07-07 6:17 PM, David Anderson wrote:
> +1 for removing this. Gecko's use is inconsistent, and outside of Gecko code that does use it, I've never seen it used in any other codebase. I've never gone to another project and thought, "I miss decorating everything in a way that changes capitalization and impairs canonical naming".
>
> Reasons for using it in the first place are suspect. None of them seem to justify the extra developer overhead or the odd variable names that result. I can't imagine we've solved some massive readability problem unique to Gecko or unsolved by other projects, or that we're catching important problems that static analysis can't find.

[Thomas Zimmermann]

Hi

Am 13.07.2015 um 21:07 schrieb Jeff Gilbert:
> On Sun, Jul 12, 2015 at 6:45 PM, Thomas Zimmermann
> <tzimm...@mozilla.com <mailto:tzimm...@mozilla.com>> wrote:
>
> Am 08.07.2015 um 16:36 schrieb smaug:

> > Do you actually have any data how many % of Gecko devs would prefer
> > not using aFoo?
>

> I strongly prefer 'aFoo' over 'foo' for the extra context that it
> gives
> to the variable. If we want to change anything we should rather
> introduce a separate prefix for output parameters.
>
>
> Which part of this extra context is useful?

When writing code, I consider it good style to not write into anything
that starts with an 'a' prefix, except result arguments. Makes the code
cleaner, more readable, and often gives it a clear structure. When
reading the code later on, it's easy to spot the parts of a the code
that directly depend on external parameters by looking for 'a' and 'm'
prefixes. Everything else is helper code or for temporary results.
Longer functions that have 'a's and 'm's all over the place are good
candidates for refactoring.

Best regards
Thomas

[Ehsan Akhgari]

On 2015-07-13 3:07 PM, Jeff Gilbert wrote:
> On Sun, Jul 12, 2015 at 6:45 PM, Thomas Zimmermann <tzimm...@mozilla.com>

> wrote:
>
>> Am 08.07.2015 um 16:36 schrieb smaug:

>>> Do you actually have any data how many % of Gecko devs would prefer
>>> not using aFoo?
>>

>> I strongly prefer 'aFoo' over 'foo' for the extra context that it gives
>> to the variable. If we want to change anything we should rather
>> introduce a separate prefix for output parameters.
>>
>
> Which part of this extra context is useful?

Repeating what Kats said elsewhere in the thread which seems to have
been completely ignored in the pile of messages here:

When debugging in a text based debugger such as gdb, sometimes you have
a variable called aFoo which has the wrong value, and with the existing
naming convention, you can quickly run "up" in the debugger to go to
caller frames looking for the first time the argument is called
something without an 'a' prefix, and then you look to see where the
value was computed. If we remove this naming convention, you would have
to do that work in every frame, which would make debugging the same
scenario much more time consuming.

Note that if you mostly use a graphical debugger such as Visual Studio,
you may not rely on this because the debugger would show you more of the
code in each frame, but I believe graphical debuggers are a niche among
Mozilla developers.

[Kyle Huey]

On Tue, Jul 14, 2015 at 6:57 AM, Ehsan Akhgari <ehsan....@gmail.com>
wrote:

> On 2015-07-13 3:07 PM, Jeff Gilbert wrote:
>
>> On Sun, Jul 12, 2015 at 6:45 PM, Thomas Zimmermann <
>> tzimm...@mozilla.com>
>> wrote:
>>
>> Am 08.07.2015 um 16:36 schrieb smaug:
>>>

>>>> Do you actually have any data how many % of Gecko devs would prefer
>>>> not using aFoo?
>>>>
>>>

>>> I strongly prefer 'aFoo' over 'foo' for the extra context that it gives
>>> to the variable. If we want to change anything we should rather
>>> introduce a separate prefix for output parameters.
>>>
>>>
>> Which part of this extra context is useful?
>>
>
> Repeating what Kats said elsewhere in the thread which seems to have been
> completely ignored in the pile of messages here:
>
> When debugging in a text based debugger such as gdb, sometimes you have a
> variable called aFoo which has the wrong value, and with the existing
> naming convention, you can quickly run "up" in the debugger to go to caller
> frames looking for the first time the argument is called something without
> an 'a' prefix, and then you look to see where the value was computed. If
> we remove this naming convention, you would have to do that work in every
> frame, which would make debugging the same scenario much more time
> consuming.
>
> Note that if you mostly use a graphical debugger such as Visual Studio,
> you may not rely on this because the debugger would show you more of the
> code in each frame, but I believe graphical debuggers are a niche among
> Mozilla developers.
>

That assumes that the 'Foo' of aFoo is stable across function boundaries,
which is not always the case.

- Kyle

[Ehsan Akhgari]

On 2015-07-14 9:59 AM, Kyle Huey wrote:
> On Tue, Jul 14, 2015 at 6:57 AM, Ehsan Akhgari <ehsan....@gmail.com
> <mailto:ehsan....@gmail.com>> wrote:
>
> On 2015-07-13 3:07 PM, Jeff Gilbert wrote:
>
> On Sun, Jul 12, 2015 at 6:45 PM, Thomas Zimmermann

> <tzimm...@mozilla.com <mailto:tzimm...@mozilla.com>>

> wrote:
>
> Am 08.07.2015 um 16:36 schrieb smaug:
>

> Do you actually have any data how many % of Gecko devs
> would prefer
> not using aFoo?
>
>

> I strongly prefer 'aFoo' over 'foo' for the extra context
> that it gives
> to the variable. If we want to change anything we should rather
> introduce a separate prefix for output parameters.
>
>
> Which part of this extra context is useful?
>
>
> Repeating what Kats said elsewhere in the thread which seems to have
> been completely ignored in the pile of messages here:
>
> When debugging in a text based debugger such as gdb, sometimes you
> have a variable called aFoo which has the wrong value, and with the
> existing naming convention, you can quickly run "up" in the
> debugger to go to caller frames looking for the first time the
> argument is called something without an 'a' prefix, and then you
> look to see where the value was computed. If we remove this naming
> convention, you would have to do that work in every frame, which
> would make debugging the same scenario much more time consuming.
>
> Note that if you mostly use a graphical debugger such as Visual
> Studio, you may not rely on this because the debugger would show you
> more of the code in each frame, but I believe graphical debuggers
> are a niche among Mozilla developers.
>
>
> That assumes that the 'Foo' of aFoo is stable across function
> boundaries, which is not always the case.

No, it doesn't. In the scenario above, all you're looking for is when a
value was computed, so you can quickly see an aDuck, aQuack, aFoopyFoo
and determine that the value was passed down from the caller, until you
get to a call site which passes in something that doesn't start with an 'a'.

[Tom Tromey]

>> That assumes that the 'Foo' of aFoo is stable across function
>> boundaries, which is not always the case.

Ehsan> No, it doesn't. In the scenario above, all you're looking for is when
Ehsan> a value was computed, so you can quickly see an aDuck, aQuack,
Ehsan> aFoopyFoo and determine that the value was passed down from the
Ehsan> caller, until you get to a call site which passes in something that
Ehsan> doesn't start with an 'a'.

It was mentioned elsewhere in this thread that some code assigns to
arguments. In these cases going up to the point of origination may miss
the spot that actually introduced the value.

Tom

[Ehsan Akhgari]

That's true. In practice though, I use this technique all the time and
I can't remember the last time I was bit by someone assigning to an
argument when chasing a bad value in the debugger.

[Joshua Cranmer 🐧]

On 7/14/2015 1:39 AM, Thomas Zimmermann wrote:
> When writing code, I consider it good style to not write into anything
> that starts with an 'a' prefix, except result arguments.

"You should never write into something with an 'a' prefix except when
you should," if you simplify it. I've actually avoided using the a
prefix for outparams precisely because it feels more consistent to never
assign to a variable with an a value (and also because it distinguishes
between Foo *aInArray and Foo *outparam), yet I did see someone upthread
praising that it helped you see which values were outparams.

> Makes the code cleaner, more readable, and often gives it a clear
> structure. When reading the code later on, it's easy to spot the parts
> of a the code that directly depend on external parameters by looking
> for 'a' and 'm' prefixes.

This, I feel, is an aspiration which is not supported by any of the code
I work on (which admittedly is heavily COMtaminated). Any intuition
about a difference between aFoo and foo in terms of "relies on
arguments" is bound to be wrong.


Given that the aFoo rule is one of the least adhered-to portions of our
style guide, and has been for as long as I've worked on Mozilla code;
that the ancillary rule of "don't assign to an argument" has also been
ignored on quite a few occasions; and that there hasn't been any real
history of people complaining about the lack of adherence to this style
guide point, I rather suspect that whatever people might say in how
useful the 'a' prefix is, they get along quite fine without it.

--
Joshua Cranmer
Thunderbird and DXR developer
Source code archæologist

[Kartikaya Gupta]

On Tue, Jul 14, 2015 at 10:10 AM, Tom Tromey <ttr...@mozilla.com> wrote:
>>> That assumes that the 'Foo' of aFoo is stable across function
>>> boundaries, which is not always the case.
>
> Ehsan> No, it doesn't. In the scenario above, all you're looking for is when
> Ehsan> a value was computed, so you can quickly see an aDuck, aQuack,
> Ehsan> aFoopyFoo and determine that the value was passed down from the
> Ehsan> caller, until you get to a call site which passes in something that
> Ehsan> doesn't start with an 'a'.
>
> It was mentioned elsewhere in this thread that some code assigns to
> arguments. In these cases going up to the point of origination may miss
> the spot that actually introduced the value.
>

Admittedly not perfect, but as a first-order approximation:

kats@kgupta-air mozilla-git$ find . -name "*.cpp" | xargs grep "^
*a[A-Z]\w* = " | wc -l
5414

That's not a lot considering the size of the codebase.

[L. David Baron]

On Tuesday 2015-07-14 10:27 -0400, Kartikaya Gupta wrote:
> Admittedly not perfect, but as a first-order approximation:
>
> kats@kgupta-air mozilla-git$ find . -name "*.cpp" | xargs grep "^
> *a[A-Z]\w* = " | wc -l
> 5414
>
> That's not a lot considering the size of the codebase.

And a decent portion of those are assigning to out-params that are
references rather than pointers.

-David

--
𝄞 L. David Baron http://dbaron.org/ 𝄂
𝄢 Mozilla https://www.mozilla.org/ 𝄂
Before I built a wall I'd ask to know
What I was walling in or walling out,
And to whom I was like to give offense.
- Robert Frost, Mending Wall (1914)

[Kartikaya Gupta]

(and 600+ plus of those are from my objdirs, actually). Here it is
broken down by dir:

kats@kgupta-air mozilla-git$ find . -type d -maxdepth 1 -mindepth 1 |
grep -v ".git" | while read dir; do pushd $dir >/dev/null; find .
-name "*.cpp" | xargs grep "^ *a[A-Z]\w* = " | wc -l | xargs echo
$dir; popd >/dev/null; done
./accessible 10
./addon-sdk 0
./b2g 0
./browser 0
./build 0
./caps 5
./chrome 4
./config 0
./db 0
./docshell 19
./dom 1739
./editor 95
./embedding 12
./extensions 20
./gfx 229
./hal 2
./image 33
./intl 55
./ipc 23
./js 1
./layout 1063
./media 15
./memory 0
./mfbt 0
./mobile 0
./modules 6
./mozglue 2
./netwerk 124
./nsprpub 0
./obj-b2g-desktop 320
./obj-host 329
./other-licenses 2
./parser 80
./probes 0
./python 0
./rdf 3
./security 38
./services 0
./startupcache 0
./storage 2
./testing 0
./toolkit 68
./tools 3
./uriloader 43
./view 8
./webapprt 0
./widget 876
./xpcom 177
./xpfe 8
./xulrunner 0

[Thomas Zimmermann]

Hi

Am 14.07.2015 um 16:19 schrieb Joshua Cranmer 🐧:
> On 7/14/2015 1:39 AM, Thomas Zimmermann wrote:
>> When writing code, I consider it good style to not write into
>> anything that starts with an 'a' prefix, except result arguments.
>
> "You should never write into something with an 'a' prefix except when
> you should," if you simplify it. I've actually avoided using the a
> prefix for outparams precisely because it feels more consistent to
> never assign to a variable with an a value (and also because it
> distinguishes between Foo *aInArray and Foo *outparam), yet I did see
> someone upthread praising that it helped you see which values were
> outparams.

As I said above, I'd support introducing a separate prefix for output
parameters. And consequently enforcing the no-a-writes policy in reviews.

>
>> Makes the code cleaner, more readable, and often gives it a clear
>> structure. When reading the code later on, it's easy to spot the
>> parts of a the code that directly depend on external parameters by
>> looking for 'a' and 'm' prefixes.
>
> This, I feel, is an aspiration which is not supported by any of the
> code I work on (which admittedly is heavily COMtaminated). Any
> intuition about a difference between aFoo and foo in terms of "relies
> on arguments" is bound to be wrong.

I agree in general, but there are a number of conventions for special
symbols: prefixes, all-capitals, start-with-capital, etc. Each helps to
structure the code and make it more readable. That doesn't mean that the
other symbols are unimportant. I never claimed that it's possible to
understand code without actually reading it.

>
>
> Given that the aFoo rule is one of the least adhered-to portions of
> our style guide, and has been for as long as I've worked on Mozilla
> code; that the ancillary rule of "don't assign to an argument" has
> also been ignored on quite a few occasions; and that there hasn't been
> any real history of people complaining about the lack of adherence to
> this style guide point, I rather suspect that whatever people might
> say in how useful the 'a' prefix is, they get along quite fine without
> it.
>

In such cases, I always wonder why the coding style is incorrect.

The discussion has a number of good points in favor of using 'a', but I
missed convincing arguments in favor of not using 'a'. Are there any? I
don't consider "I don't get what 'a' is good for" a convincing argument.

Best regards
Thomas

[Benjamin Smedberg]

On 7/8/2015 7:31 AM, Nathan Froyd wrote:
> If somebody is willing to do the formatting, I'm willing to do the
> review. I think the thread has reached the point of people repeating
> ad nauseum what was already said earlier in the thread, so it's time
> for a decision. Benjamin?

Aww, I was avoiding getting into this thread.

I personally have no strong preference, and our existing community is
pretty deeply divided. I doubt we're going to come to consensus here,
and this is a pretty tough decision to make on its own. I do believe
that consistency trumps module/personal preference in terms of coding style.

The argument I am most sympathetic to is that this convention is a
barrier to new contributors. Making new contributors productive, both
employees and volunteers, is a very good reason to choose one style over
another.

Given that premise, we shouldn't just change aArgument; we should adopt
the Google C++ style guide wholesale:

* names_with_underscores
* members_with_trailing_
* no more ns prefix

There is good research that underscore_names are more readable, and many
of these will be more familiar to new contributors. Also we have a fair
bit of shared code with Google.

If there is a decision to be made here, I'd like to make this RFC:

* switch our codebase wholesale to the Google C++ style guide

With the following implementation plan:

* For now, code should continue to be written in the current style with
aFoo, mFoo, and camelCase.
* get our code -Wshadow clean
* Ask poiru to investigate auto-renaming of our variables including
mFoo, aFoo, and camelCase to the google-standard local variable names.
* Do not make any changes to the style guide or standard practice until
we're comfortable that we can do automatic changes.
* Make the automatic changes and change our style guide at roughly the
same time.
* Go back and deal with class names (nsFoo) as a separate/later pass.

--BDS

[Joshua Cranmer 🐧]

On 7/14/2015 10:23 AM, Benjamin Smedberg wrote:
> Given that premise, we shouldn't just change aArgument; we should
> adopt the Google C++ style guide wholesale:
>
> * names_with_underscores

The biggest problem here is that WebIDL and XPIDL codegen are heavily
geared towards camelCase names, as the IDL convention is camelCase.

[David Major]

May I request that the major parts of this not happen until we have a
blame that can "see through" such changes.

Last I checked, gps had some ideas in that space but lacked time to
implement.

On Wed, Jul 15, 2015, at 03:23 AM, Benjamin Smedberg wrote:
>
>
> On 7/8/2015 7:31 AM, Nathan Froyd wrote:
> > If somebody is willing to do the formatting, I'm willing to do the
> > review. I think the thread has reached the point of people repeating
> > ad nauseum what was already said earlier in the thread, so it's time
> > for a decision. Benjamin?
>
> Aww, I was avoiding getting into this thread.
>
> I personally have no strong preference, and our existing community is
> pretty deeply divided. I doubt we're going to come to consensus here,
> and this is a pretty tough decision to make on its own. I do believe
> that consistency trumps module/personal preference in terms of coding
> style.
>
> The argument I am most sympathetic to is that this convention is a
> barrier to new contributors. Making new contributors productive, both
> employees and volunteers, is a very good reason to choose one style over
> another.
>

> Given that premise, we shouldn't just change aArgument; we should adopt
> the Google C++ style guide wholesale:
>
> * names_with_underscores

> * members_with_trailing_
> * no more ns prefix
>
> There is good research that underscore_names are more readable, and many
> of these will be more familiar to new contributors. Also we have a fair
> bit of shared code with Google.
>
> If there is a decision to be made here, I'd like to make this RFC:
>
> * switch our codebase wholesale to the Google C++ style guide
>
> With the following implementation plan:
>
> * For now, code should continue to be written in the current style with
> aFoo, mFoo, and camelCase.
> * get our code -Wshadow clean
> * Ask poiru to investigate auto-renaming of our variables including
> mFoo, aFoo, and camelCase to the google-standard local variable names.
> * Do not make any changes to the style guide or standard practice until
> we're comfortable that we can do automatic changes.
> * Make the automatic changes and change our style guide at roughly the
> same time.
> * Go back and deal with class names (nsFoo) as a separate/later pass.
>
> --BDS
>

[Seth Fowler]

> On Jul 14, 2015, at 8:23 AM, Benjamin Smedberg <benj...@smedbergs.us> wrote:
>
> * no more ns prefix

Are people still creating new classes with an ’ns’ prefix? Surely this is something we can drop right away, at least for new code. Much of the codebase already does not use this style. We have namespaces now, after all.

- Seth

[Gregory Szorc]

On Tue, Jul 14, 2015 at 9:06 AM, David Major <dma...@mozilla.com> wrote:

> May I request that the major parts of this not happen until we have a
> blame that can "see through" such changes.
>
> Last I checked, gps had some ideas in that space but lacked time to
> implement.
>

I spoke to a Mercurial maintainer about some of my ideas and the feature
would be welcome upstream. However, there isn't enough time to land for
Mercurial 3.5 (code freeze this week), so we'd have to wait until Mercurial
3.6 (November 1 release date) at the earliest.

That being said, every other organizations in the world is using the same
or similar tools and is faced with similar challenges. Lack of a
commit-skipping feature doesn't hinder other organizations from performing
major refactorings. So while I'd love to make the tools better, I don't
think waiting on the tools should be a blocker to mass reformatting the
tree.

While I'm here, if anyone has suggestions for quick fixes to
https://hg.mozilla.org/'s HTML output, those are relatively easy to make.
Please file bugs against Developer Services :: hg.mozilla.org if you have
ideas! And, if you want to hack on improvements yourself,
https://mozilla-version-control-tools.readthedocs.org/en/latest/hgmo/contributing.html#hacking-the-theming
tells you how. I reckon we could come up with enough small changes to hold
us over until commit skipping in blame is implemented.

>
>
On Wed, Jul 15, 2015, at 03:23 AM, Benjamin Smedberg wrote:
> >
> >
> > On 7/8/2015 7:31 AM, Nathan Froyd wrote:
> > > If somebody is willing to do the formatting, I'm willing to do the
> > > review. I think the thread has reached the point of people repeating
> > > ad nauseum what was already said earlier in the thread, so it's time
> > > for a decision. Benjamin?
> >
> > Aww, I was avoiding getting into this thread.
> >
> > I personally have no strong preference, and our existing community is
> > pretty deeply divided. I doubt we're going to come to consensus here,
> > and this is a pretty tough decision to make on its own. I do believe
> > that consistency trumps module/personal preference in terms of coding
> > style.
> >
> > The argument I am most sympathetic to is that this convention is a
> > barrier to new contributors. Making new contributors productive, both
> > employees and volunteers, is a very good reason to choose one style over
> > another.
> >
> > Given that premise, we shouldn't just change aArgument; we should adopt
> > the Google C++ style guide wholesale:
> >
> > * names_with_underscores
> > * members_with_trailing_

> > * no more ns prefix
> >

[Martin Thomson]

On Tue, Jul 14, 2015 at 10:06 AM, Gregory Szorc <g...@mozilla.com> wrote:
> That being said, every other organizations in the world is using the same
> or similar tools and is faced with similar challenges. Lack of a
> commit-skipping feature doesn't hinder other organizations from performing
> major refactorings. So while I'd love to make the tools better, I don't
> think waiting on the tools should be a blocker to mass reformatting the
> tree.

This. If blame is the only victim, and a temporary one, then that's a
pretty small price to pay.

[Kyle Huey]

On Tue, Jul 14, 2015 at 8:23 AM, Benjamin Smedberg <benj...@smedbergs.us>

FWIW, I think the Google C++ style is terrible, but if it gets us to a
point where we can run clang-format as part of make check and never worry
about style again I am all for it.

- Kyle

[smaug]

Couple of work days per year for certain devs?
Perhaps it is a small price.

Also, if we just stick with the current coding style, large parts of Gecko doesn't need to be
refactored to new style.



About using Google coding style, there isn't any evidence it would make new contributors more productive, and
it might make old contributors less productive at least for some time.



But whatever we change, if any - since the current coding style is rather sane for C++ -
consistency is what I care about most. It is mystery to me why we've still written new code not using
the coding style we have had for ages. I guess that is where we really need tools, enforce some style.



-Olli