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

[Jeff Gilbert]

On Thu, Jul 16, 2015 at 6:50 AM, Ehsan Akhgari 
wrote:

> On 2015-07-15 6:47 PM, Jeff Gilbert wrote:
>
>> "Arg warts improve backtracking for debugging"
>> Regardless of the validity of  "Arg warts help illustrate information
>> flow", the use-case of backtracking to 'origin' of aFoo is also
>> unconvincing, since you'd only need to go back once more than previously
>> before moving once back 'up'. Either way sounds eminently automateable.
>> -> No practical change, thus irrelevant.
>>
>
> I don't understand what you're saying here at all.  It seems that you're
> saying that this use case is not interesting to you, and that's OK, but
> that doesn't make it irrelevant.  :-)
>

Not at all! Rather, that either way it would be workable. The tiny extra
bit of effort it would take without aFoo is completely solved by
automation, and if this is a common enough use-case for this tiny extra bit
of effort to matter, it should *already* have been automated. Thus I don't
have a ton of sympathy for making a readily-automateable process require
differentially more effort without automation.


>
>  "It's hard to implement"
>> This is irrelevant for new code, and bulk-change is feasible once we have
>> -Wshadow. (which isn't too hard. We shouldn't have shadowing in our code
>> anyway, so we want -Wshadow regardless of this discussion) Besides, a
>> number of our internal projects and modules already don't have it, so it's
>> free for them.
>> -> Non-trivial, but not too hard to implement.
>>
>
> The "isn't too hard" part of the above is factually incorrect.  See bug
> 563195 for why.
>

I have read it, and it doesn't seem insurmountable. Just because it's not
free doesn't make it 'too hard'. I am naturally willing to work on this
myself.


>
>  So, when you say "I like aFoo because it lets me know which vars are
>> args",
>> is there any un-touched-on aspect of the information "this var is an
>> argument" that is useful to know? How does every other project survive
>> without it?
>>
>
> Given Benjamin's response, there is really no point in arguing over this
> any more, but just as a thought exercise: is there anything that would
> convince you that this is useful for some people?


There's only no point here if Benjamin's proposal carries! Besides, this
discussion is certainly a subset of the boil-the-ocean discussion. If aFoo
is truly valuable, then removing it via Google Style would hurt us just as
removing it a la carte would.

What could convince me? Reasoned and consistent arguments, perhaps examples
or anecdotes where aFoo really came in handy that wouldn't have been
covered under general good code style. Maybe someone might have done a
study on this style. Maybe there are other large projects which use this
style. Tautologically: Convincing arguments!


> I would expect having those people tell you that it's useful for them
> should be a good enough reason to assume that it is!
>

Especially as part of standards body, I'm perfectly fine with dismissing
requests for things people claim to want, but can't provide sufficient
reasoning for, [1] or likewise being unable to argue my own case
successfully. I'm well-acquainted with being convinced to change my
position through discussion and debate, and also with convincing others.
(or not!)

This is why we have these discussions! Claims are not substantiated simply
because people claim them to be true. We need to lay the reasons on the
table and then weigh them, as without reasons, choices are arbitrary.
Sometimes we even need to do it more than once, as time changes things.
(even if people hate doing this!) We may find no need for change, or we may
instead find we can change for the better.

[1]: To be clear, "sufficient reasoning" can definitely include "culture",
"harsh realities", "pragmatism", and other such things. They are not all
granted the same gravity, but they all contribute.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[R Kent James]

On 7/14/2015 8:23 AM, Benjamin Smedberg wrote:
> Aww, I was avoiding getting into this thread.
>
...
> 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.

I have also avoided getting into this thread, as the whole premise seems 
to me to be pretty clueless about what makes Mozilla code difficult for 
newcomers.


I think I'm a pretty good authority on experience of newcomers, as I 
spend a pretty good part of my Mozilla life tracing out Thunderbird 
issues in core Mozilla code that I know very little about. Earlier in 
the week it was the addon manager, today it is certificate handling. I 
find the same thing over and over again that makes Mozilla code really 
difficult to approach when you are not already an expert. And it has 
nothing to do with whether you include the "a" in front of method 
variables or not.


What is missing? The most basic description of what major functions do, 
and how they are supposed to interact with the rest of code. If you 
really to be "Making new contributors productive" then require that!


Examples:

1) Earlier this week, it was the addon code. Checkout XPIProvider.jsm No 
description anywhere of what this is supposed to do or how it interacts 
with other code. Yes there is detailed documentation of some of the 
function calls, but nowhere can I understand the relationship of this to 
AddonManager, which seems to pretty much do he same thing just from the 
titles. Only by hours and hours of tracing out code can I figure out 
their relationship (see bug 1183733 for the final result). Documentation 
of function calls does not really help when, as a newcomer, you don't 
understand the big picture.


2) Currently, looking at some sort of regression in certificate 
management with STARTTLS. Look at TCPSocketChild.cpp for example, no 
clue anywhere in that file what it is about. Or nsNSSCertificate.cpp no 
clue what that really does, and the code does not give any hints.


So if you want to make things easier for newcomers, require that modules 
have some sort of overview of what they are supposed to do, and how they 
interact with other code. Don't waste time on code churn with style of 
existing code.


:rkent


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[Chris Pearce]

On 7/15/2015 3: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
* members_with_trailing_
* no more ns prefix



I used this style in a personal project, and I quickly came to regret it.

Distinguishing whether a variable is a member verses being a local var 
by a single '_' character I found was harder, as '_' is a very small 
character, and is hard to see, especially when syntax highlighting is 
underlining the word. Whereas the difference between a leading 'a' and 
'm' is very obvious, even when syntax highlighting is underlining the word.


I think adopting Google style is a bad idea.

cpearce.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Ehsan Akhgari]

On 2015-07-15 6:47 PM, Jeff Gilbert wrote:

"Arg warts improve backtracking for debugging"
Regardless of the validity of  "Arg warts help illustrate information
flow", the use-case of backtracking to 'origin' of aFoo is also
unconvincing, since you'd only need to go back once more than previously
before moving once back 'up'. Either way sounds eminently automateable.
-> No practical change, thus irrelevant.


I don't understand what you're saying here at all.  It seems that you're 
saying that this use case is not interesting to you, and that's OK, but 
that doesn't make it irrelevant.  :-)



"It's hard to implement"
This is irrelevant for new code, and bulk-change is feasible once we have
-Wshadow. (which isn't too hard. We shouldn't have shadowing in our code
anyway, so we want -Wshadow regardless of this discussion) Besides, a
number of our internal projects and modules already don't have it, so it's
free for them.
-> Non-trivial, but not too hard to implement.


The "isn't too hard" part of the above is factually incorrect.  See bug 
563195 for why.



So, when you say "I like aFoo because it lets me know which vars are args",
is there any un-touched-on aspect of the information "this var is an
argument" that is useful to know? How does every other project survive
without it?


Given Benjamin's response, there is really no point in arguing over this 
any more, but just as a thought exercise: is there anything that would 
convince you that this is useful for some people?  I would expect having 
those people tell you that it's useful for them should be a good enough 
reason to assume that it is!

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[WaltS48]

On 07/16/2015 01:16 AM, Thomas Zimmermann wrote:

Hi

Am 16.07.2015 um 00:47 schrieb Jeff Gilbert:

On Tue, Jul 14, 2015 at 7:55 AM, Thomas Zimmermann
mailto:tzimmerm...@mozilla.com>> wrote:

 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.


On the other hand, I'm still unconvinced by the pro-'a' arguments I've
read here. Besides roc's point about refactoring, the argument against
aFoo is mainly about whether the information added is worth the noise.
Adding information is not always worth it, since useless information
is noise.


One man's noise is another man's information. ;) Your arguments here and
below are of the "I don't need it so it's useless" type.

The core question is: How does removing this prefix help us in producing
better code? To me, 'producing' includes 'writing' and
'reviewing/reading'. Using 'a' seems helpful to at least some reviewers.
If we remove the prefix, does this improve the writing part
significantly enough to make it worthwhile? My answer is "No".

Best regards
Thomas




To a user that doesn't know any coding, it appears to me that all of 
these style guides are wrong, and you need to start a campaign to change 
their thinking.


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.

I don't know what a style guide is, but if it is just a guide then 
remove aFoo, and those that use it go on using it.


Those coming from coding one of the above won't have to go WTH, when 
they see the Mozilla style guide.


Cheers!
--
Kubuntu 14.10 | KDE 4.14.1 | Thunderbird 42.0a1(Daily) Go Bucs!
[Coexist · Understanding Across Divides](https://www.coexist.org/)
[Visit Pittsburgh](http://www.visitpittsburgh.com)
[Pittsburgh Vintage Grand Prix -](http://www.pvgp.org/) July 10-19,2015
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Thomas Zimmermann]

Hi

Am 16.07.2015 um 00:47 schrieb Jeff Gilbert:
> On Tue, Jul 14, 2015 at 7:55 AM, Thomas Zimmermann
> mailto:tzimmerm...@mozilla.com>> wrote:
>
> 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.
>
>
> On the other hand, I'm still unconvinced by the pro-'a' arguments I've
> read here. Besides roc's point about refactoring, the argument against
> aFoo is mainly about whether the information added is worth the noise.
> Adding information is not always worth it, since useless information
> is noise.

One man's noise is another man's information. ;) Your arguments here and
below are of the "I don't need it so it's useless" type.

The core question is: How does removing this prefix help us in producing
better code? To me, 'producing' includes 'writing' and
'reviewing/reading'. Using 'a' seems helpful to at least some reviewers.
If we remove the prefix, does this improve the writing part
significantly enough to make it worthwhile? My answer is "No".

Best regards
Thomas

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[Benjamin Smedberg]

On 7/15/2015 5:47 PM, Andrew Sutherland wrote:

Would it be crazy for us to resort to a poll on these things?


A poll will not be useful for informing this decision.

--BDS

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[smaug]

On 07/16/2015 01:47 AM, Jeff Gilbert wrote:

On Tue, Jul 14, 2015 at 7:55 AM, Thomas Zimmermann 
wrote:


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.



On the other hand, I'm still unconvinced by the pro-'a' arguments I've read
here. Besides roc's point about refactoring, the argument against aFoo is
mainly about whether the information added is worth the noise. Adding
information is not always worth it, since useless information is noise.
(and increases cognitive load) The extremes are "variables are always bare"
(no mFoo/gFoo/sFoo/kFoo) and "extreme Hungarian notation". (where each
variable name references its scope, type, and favorite color) The former is
missing very valuable information (implicit member vars!), whereas the
latter definitely gets in the way.

The primary argument here is whether the information aFoo adds is worth the
increased cognitive load. (both of holding the information and parsing the
'real name' out of the annotated variable name)

I hold that aFoo does not add useful information, since outparams should be
standardized to something obvious and distinct from aFoo, and I otherwise
treat arguments as locals within a function.


Let's divide things up for clarity:

"Args should be immutable"
Clearly other people here treat arguments differently, in one instance
where the recommendation is that we should not to assign to them. (in which
case they should really all just be const) This seems strange to me though,
since I don't see why arguments should be immutable moreso than all
variables should be as immutable as possible. Unilaterally requiring that
arguments be const seems at odds with acknowledging that mutability useful.
(And regardless, if we want immutable args, we should enforce immutable
args with const, not just name them differently)
-> Wrong solution for the illustrated (and not-agreed-upon) problem.

"Arg warts help illustrate information flow"
If we instead don't require that args must be const, then args seems little
different than intermediary locals derived from args. A subsequent call or
result of a function may in fact be (eventually) purely derived from args,
yet not visibly pure in that it references non-arguments. If we're tracing
information flow, shouldn't the a-prefix be contagious for any local
derived purely from existing a-prefixed vars? This seems like a distraction
to me. Can't we just read the functions instead of using an unreliable
heuristic?


For reviewing aFoo clearly helps when following the information flow - 
especially
when you first time skim through some method.

I'd probably care less about this matter if my code-written/code-reviewed ratio 
was different
than what it is now.

I believe, but have zero proof to this, that clearly indicating what the 
arguments are also leads
to easier to understand code, since that hints to the code author to think what 
to do with those variables
and not treat them as random local variables.


-> Does an unreliable heuristic warrant having a-warts? I don't think so,
but this is likely the most contentious aspect.

"Arg warts improve backtracking for debugging"
Regardless of the validity of  "Arg warts help illustrate information
flow", the use-case of backtracking to 'origin' of aFoo is also
unconvincing, since you'd only need to go back once more than previously
before moving once back 'up'. Either way sounds eminently automateable.
-> No practical change, thus irrelevant.

"It's hard to implement"
This is irrelevant for new code, and bulk-change is feasible once we have
-Wshadow. (which isn't too hard. We shouldn't have shadowing in our code
anyway, so we want -Wshadow regardless of this discussion) Besides, a
number of our internal projects and modules already don't have it, so it's
free for them.
-> Non-trivial, but not too hard to implement.


So, when you say "I like aFoo because it lets me know which vars are args",
is there any un-touched-on aspect of the information "this var is an
argument" that is useful to know? How does every other project survive
without it?


It is not about surviving. You get used to some style, and lots of Gecko devs 
have got used to
aFoo. And Milan's comments for example hint that devs coming to the project may 
actually think it is
a better, easier to understand coding style than non-prefixed-arguments.





A convincing argument is one that would have been a good reason to switch
to aFoo in the first place. Without this, we don't have reasons, we just
have momentum.


There hasn't really been convincing arguments to switch away from aFoo,
and I don't see a point for the change because of the change itself.





-Olli
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Jeff Gilbert]

On Tue, Jul 14, 2015 at 7:55 AM, Thomas Zimmermann 
wrote:

> 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.
>

On the other hand, I'm still unconvinced by the pro-'a' arguments I've read
here. Besides roc's point about refactoring, the argument against aFoo is
mainly about whether the information added is worth the noise. Adding
information is not always worth it, since useless information is noise.
(and increases cognitive load) The extremes are "variables are always bare"
(no mFoo/gFoo/sFoo/kFoo) and "extreme Hungarian notation". (where each
variable name references its scope, type, and favorite color) The former is
missing very valuable information (implicit member vars!), whereas the
latter definitely gets in the way.

The primary argument here is whether the information aFoo adds is worth the
increased cognitive load. (both of holding the information and parsing the
'real name' out of the annotated variable name)

I hold that aFoo does not add useful information, since outparams should be
standardized to something obvious and distinct from aFoo, and I otherwise
treat arguments as locals within a function.


Let's divide things up for clarity:

"Args should be immutable"
Clearly other people here treat arguments differently, in one instance
where the recommendation is that we should not to assign to them. (in which
case they should really all just be const) This seems strange to me though,
since I don't see why arguments should be immutable moreso than all
variables should be as immutable as possible. Unilaterally requiring that
arguments be const seems at odds with acknowledging that mutability useful.
(And regardless, if we want immutable args, we should enforce immutable
args with const, not just name them differently)
-> Wrong solution for the illustrated (and not-agreed-upon) problem.

"Arg warts help illustrate information flow"
If we instead don't require that args must be const, then args seems little
different than intermediary locals derived from args. A subsequent call or
result of a function may in fact be (eventually) purely derived from args,
yet not visibly pure in that it references non-arguments. If we're tracing
information flow, shouldn't the a-prefix be contagious for any local
derived purely from existing a-prefixed vars? This seems like a distraction
to me. Can't we just read the functions instead of using an unreliable
heuristic?
-> Does an unreliable heuristic warrant having a-warts? I don't think so,
but this is likely the most contentious aspect.

"Arg warts improve backtracking for debugging"
Regardless of the validity of  "Arg warts help illustrate information
flow", the use-case of backtracking to 'origin' of aFoo is also
unconvincing, since you'd only need to go back once more than previously
before moving once back 'up'. Either way sounds eminently automateable.
-> No practical change, thus irrelevant.

"It's hard to implement"
This is irrelevant for new code, and bulk-change is feasible once we have
-Wshadow. (which isn't too hard. We shouldn't have shadowing in our code
anyway, so we want -Wshadow regardless of this discussion) Besides, a
number of our internal projects and modules already don't have it, so it's
free for them.
-> Non-trivial, but not too hard to implement.


So, when you say "I like aFoo because it lets me know which vars are args",
is there any un-touched-on aspect of the information "this var is an
argument" that is useful to know? How does every other project survive
without it?

A convincing argument is one that would have been a good reason to switch
to aFoo in the first place. Without this, we don't have reasons, we just
have momentum.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[Andrew Sutherland]

Would it be crazy for us to resort to a poll on these things?  I propose
abusing the mozillans.org "skills" field in profiles.

For example, I have created the following sets of skills on
mozillians.org by question, and which should autocomplete if you go to
the edit page for your profile at
https://mozillians.org/en-US/user/edit/

"a" prefixing of arguments:
* style-args-no-a
* style-args-yes-a
* style-args-dont-care

Switching wholesale to the google code style:
* style-google-no
* style-google-yes
* style-google-dont-care

My rationale is:
* Everyone should have a mozillians.org account and if you don't and
this provides the motivation... hooray!
* This avoids vote stuffing, more or less
* I guess someone could easily filter it down to valid committer
accounts?
* This requires no work on my part after this point.
* The autocomplete logic should let people add other options if they're
quick on their feet.

Andrew
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[Bobby Holley]

On Wed, Jul 15, 2015 at 2:26 PM, Gregory Szorc  wrote:

> The public source code for Firefox has existed for 17+ years (since ~April
> 1998). We can only assume it will be around for another 10+ years.
>
> I believe you have to take the long view on the cost benefit analysis and
> realize that a lot of pain in the short term (e.g. switching styles
> entirely) will be a fraction of the cost for tolerating inconsistent styles
> for years more. Yes, it will be painful to transition. But for software
> with a history measured in decades as opposed to months, being
> short-sighted will only burden us with various forms of debt in the years
> to come.
>

There are two uses of "consistency" being thrown around.

One is internal consistency within the project (with our style guide), the
other is consistency with Google C++ style (and lack of consistency with
other things, like JS). I don't believe at all that the lack of the latter
is a "burden" or a "debt" that will hamper our ability to effectively
evolve Gecko, and we can get the former much more cheaply. Why make it
harder for ourselves?
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[Gregory Szorc]

The public source code for Firefox has existed for 17+ years (since ~April
1998). We can only assume it will be around for another 10+ years.

I believe you have to take the long view on the cost benefit analysis and
realize that a lot of pain in the short term (e.g. switching styles
entirely) will be a fraction of the cost for tolerating inconsistent styles
for years more. Yes, it will be painful to transition. But for software
with a history measured in decades as opposed to months, being
short-sighted will only burden us with various forms of debt in the years
to come.

On Tue, Jul 14, 2015 at 8:06 PM, Bobby Holley  wrote:

> I'm not wild about this idea. Switching styles entirely would be several
> times more churn and work than just making our existing codebase conform to
> our existing style guide. Consistency with Google's style might be a nice
> bonus, and there might be subjective arguments for one or the other, but
> none of that seems worth the churn and disruption this would cause, IMO.
>
> On Tue, Jul 14, 2015 at 11: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
> >
> > ___
> > dev-platform mailing list
> > dev-platform@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-platform
> >
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[Jeff Gilbert]

On Tue, Jul 14, 2015 at 9:17 PM, Nicholas Nethercote  wrote:

> On Tue, Jul 14, 2015 at 8:06 PM, Bobby Holley 
> wrote:
> > I'm not wild about this idea.
>
> It's such a boil-the-ocean solution I honestly thought bsmedberg was
> joking at first...
>

Well consistency is a major concern, so as long as the oceans are well and
truly boiled...

I'm certainly no fan of snake_case, but the literature does say it's more
readable, and their data is better than my anecdotes.

This Modest Proposal has my vote.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[Nicholas Nethercote]

On Tue, Jul 14, 2015 at 8:06 PM, Bobby Holley  wrote:
> I'm not wild about this idea.

It's such a boil-the-ocean solution I honestly thought bsmedberg was
joking at first...

Nick
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[Bobby Holley]

I'm not wild about this idea. Switching styles entirely would be several
times more churn and work than just making our existing codebase conform to
our existing style guide. Consistency with Google's style might be a nice
bonus, and there might be subjective arguments for one or the other, but
none of that seems worth the churn and disruption this would cause, IMO.

On Tue, Jul 14, 2015 at 11: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
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[Ehsan Akhgari]

On 2015-07-14 6:53 PM, Birunthan Mohanathas wrote:

On 14 July 2015 at 08:23, Benjamin Smedberg  wrote:

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.


The C++ world is a huge mishmash of different styles. I don't think
Google style is necessarily any better than our style. And, to be
fair, our style is actually fairly close to Google style with a few
exceptions like variable naming.


I think the Google C++ style is definitely more closely maintained than 
ours, but given that they both follow the same set of goals (make 
dealing with large C++ code bases sane) the fact that we are similar 
should not be a surprise.



The real issue is that we have vastly different code styles throughout
our codebase. We use 2 spaces for indentation in some modules and 4
spaces in others. Some places defiantly use tabs and a few files have
compromised on 3 space indentation (eww!). Some modules place the
pointer adjacent to the type, others put it beside the name. Some
modules always brace control statements, others do not.


That is mostly a cultural problem.  I can't count the number of times 
that I have seen people consciously not following the coding style. 
That is not a problem that you can fix with switching coding styles, 
since that would merely change the set of people who don't like the style.



It's not our style that is the problem for new (and old!)
contributors; it's the huge inconsistency. I'm making Clang-Format
compatible with our style over in bug 961541. Once Clang-Format is
good enough, I will rewrite the entire tree on a
directory-by-directory basis.


Yes, please!

FWIW, if you hit prohibitively complex issue that you couldn't solve in 
clang-format, I think we may want to consider changing our coding style 
to be enforceable through clang-format.


Once we have this set up, we can force people to conform.  ;-)


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

* names_with_underscores
* members_with_trailing_


If we were to do this, some names would become shorter and other
longer. That might break the 80 column limit in places and break
alignment in others. (Also keep in mind that Google style mandates
`foo_bar()` for getters and `set_foo_bar()` for setters.) This is a
general problem with rewriting. In order to perform them effectively,
we need to have a consistent style and tool to automatically reformat
diffs to that style.

I personally don't think switching to Google style naming is worth it.
It would require far more changes than just sticking with either
aFooBar or fooBar for no apparent benefit.


FWIW, given the fact that we are far far away from having the magical 
rewriting tools that can take a code base the size of ours and switch it 
from Mozilla style to Google style, and do so correctly, I don't think 
there is a lot of value to be gained from nitpicking the Google style. 
We can save that conversation for when the magical tools exist.  :-)

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[Birunthan Mohanathas]

On 14 July 2015 at 08:23, Benjamin Smedberg  wrote:
> 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.

The C++ world is a huge mishmash of different styles. I don't think
Google style is necessarily any better than our style. And, to be
fair, our style is actually fairly close to Google style with a few
exceptions like variable naming.

The real issue is that we have vastly different code styles throughout
our codebase. We use 2 spaces for indentation in some modules and 4
spaces in others. Some places defiantly use tabs and a few files have
compromised on 3 space indentation (eww!). Some modules place the
pointer adjacent to the type, others put it beside the name. Some
modules always brace control statements, others do not.

It's not our style that is the problem for new (and old!)
contributors; it's the huge inconsistency. I'm making Clang-Format
compatible with our style over in bug 961541. Once Clang-Format is
good enough, I will rewrite the entire tree on a
directory-by-directory basis.

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

If we were to do this, some names would become shorter and other
longer. That might break the 80 column limit in places and break
alignment in others. (Also keep in mind that Google style mandates
`foo_bar()` for getters and `set_foo_bar()` for setters.) This is a
general problem with rewriting. In order to perform them effectively,
we need to have a consistent style and tool to automatically reformat
diffs to that style.

I personally don't think switching to Google style naming is worth it.
It would require far more changes than just sticking with either
aFooBar or fooBar for no apparent benefit.

Cheers,
Biru
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Robert O'Callahan]

On Wed, Jul 15, 2015 at 2:55 AM, Thomas Zimmermann 
wrote:

> 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.
>

It increases the overhead of extracting part of a large function into a
helper function, because typically some local variables become parameters
of the helper function, so they have to be renamed.

Admittedly, C++ already inhibits this refactoring by (usually) requiring
helper method declarations to be repeated in the class declaration ...
which is one of the things I most dislike about C++. But every increase in
overhead reduces the likelihood that the refactoring will be done when it
should.

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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[Robert O'Callahan]

On Wed, Jul 15, 2015 at 3:40 AM, Joshua Cranmer 🐧 
wrote:

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


C++ names matching WebIDL (or spec names in general) has value. We've
gotten quite close to that over the last several years. Maybe renaming
tools could understand and enforce the relationship with WebIDL, but is _
identifiers everywhere except where we're matching specs the optimal
long-term state?

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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Karl Tomlinson]

Tom Tromey writes:

> It was mentioned elsewhere in this thread that some code assigns to
> arguments.

The style guide should clarify that parameters named aFoo should
not be assigned to.  Otherwise that defeats the purpose.

Non-const references are the exception.  If these are really
needed, then something in the name would be helpful.
Something specific to non-const references would be nice, but 'a'
is better than nothing.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[smaug]

On 07/14/2015 08:11 PM, Martin Thomson wrote:

On Tue, Jul 14, 2015 at 10:06 AM, Gregory Szorc  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.




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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[Kyle Huey]

On Tue, Jul 14, 2015 at 8: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
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>

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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[Martin Thomson]

On Tue, Jul 14, 2015 at 10:06 AM, Gregory Szorc  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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[Gregory Szorc]

On Tue, Jul 14, 2015 at 9:06 AM, David Major  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
> >
> > 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
> >
> > ___
> > dev-platform mailing list
> > dev-platform@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-platform
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[Seth Fowler]

> On Jul 14, 2015, at 8:23 AM, Benjamin Smedberg  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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[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
> 
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[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.


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

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[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

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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)


signature.asc
Description: Digital signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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

On Tue, Jul 14, 2015 at 10:30 AM, L. David Baron  wrote:
> 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)
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Kartikaya Gupta]

On Tue, Jul 14, 2015 at 10:10 AM, Tom Tromey  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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Ehsan Akhgari]

On 2015-07-14 10:10 AM, Tom Tromey 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.


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.


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Ehsan Akhgari]

On 2015-07-14 9:59 AM, Kyle Huey wrote:

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

On 2015-07-13 3:07 PM, Jeff Gilbert wrote:

On Sun, Jul 12, 2015 at 6:45 PM, Thomas Zimmermann
mailto:tzimmerm...@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'.


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Kyle Huey]

On Tue, Jul 14, 2015 at 6:57 AM, Ehsan Akhgari 
wrote:

> On 2015-07-13 3:07 PM, Jeff Gilbert wrote:
>
>> On Sun, Jul 12, 2015 at 6:45 PM, Thomas Zimmermann <
>> tzimmerm...@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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Ehsan Akhgari]

On 2015-07-13 3:07 PM, Jeff Gilbert wrote:

On Sun, Jul 12, 2015 at 6:45 PM, Thomas Zimmermann 
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.

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Thomas Zimmermann]

Hi

Am 13.07.2015 um 21:07 schrieb Jeff Gilbert:
> On Sun, Jul 12, 2015 at 6:45 PM, Thomas Zimmermann
> mailto:tzimmerm...@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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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.
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Jeff Gilbert]

On Sun, Jul 12, 2015 at 6:45 PM, Thomas Zimmermann 
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?
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Jeff Gilbert]

On Sun, Jul 12, 2015 at 10:45 PM, Nicholas Nethercote <
n.netherc...@gmail.com> wrote:

> On Thu, Jul 9, 2015 at 7:01 PM, Jeff Gilbert  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.
>

Right, args have lifetimes of function-block-scope locals.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Nicholas Nethercote]

On Thu, Jul 9, 2015 at 7:01 PM, Jeff Gilbert  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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Bobby Holley]

On Thu, Jul 9, 2015 at 7:01 PM, Jeff Gilbert  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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Jeff Gilbert]

On Wed, Jul 8, 2015 at 7:48 AM, smaug  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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Karl Tomlinson]

Bobby Holley writes:

> On Wed, Jul 8, 2015 at 4:45 PM, Karl Tomlinson  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.

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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Bobby Holley]

On Wed, Jul 8, 2015 at 4:45 PM, Karl Tomlinson  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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Karl Tomlinson]

Bobby Holley writes:

> On Wed, Jul 8, 2015 at 3:52 AM, Gabor Krizsanits 
> 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-platform@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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Honza Bambas]

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

On Tue, Jul 7, 2015 at 5:13 PM, Jeff Gilbert  wrote:


On Tue, Jul 7, 2015 at 3:59 PM, Eric Rahm  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-



___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform



___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Bobby Holley]

On Wed, Jul 8, 2015 at 3:52 AM, Gabor Krizsanits 
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-platform@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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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  wrote:


On 07/07/2015 11:18 PM, Jeff Gilbert wrote:


On Tue, Jul 7, 2015 at 1:03 PM, smaug  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.




___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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



___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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.





Blame Jeff for the above.

— - Milan



On Jul 7, 2015, at 20:41 , Karl Tomlinson  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?

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. 
___ dev-platform mailing list 
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform




___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Nathan Froyd]

On Wed, Jul 8, 2015 at 6:52 AM, Gabor Krizsanits 
wrote:

> On Wed, Jul 8, 2015 at 1:05 AM, Bobby Holley 
> 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-platform@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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Gabor Krizsanits]

On Wed, Jul 8, 2015 at 1:05 AM, Bobby Holley  wrote:

> On Tue, Jul 7, 2015 at 3:59 PM, Eric Rahm  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-platform@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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Jeff Gilbert]

On Mon, Jul 6, 2015 at 8:26 PM, Mike Hommey  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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Karl Tomlinson]

Jeff Gilbert writes:

> On Tue, Jul 7, 2015 at 5:41 PM, Karl Tomlinson  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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Jeff Gilbert]

On Tue, Jul 7, 2015 at 6:06 PM, Karl Tomlinson  wrote:

> 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 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)
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Jeff Gilbert]

On Tue, Jul 7, 2015 at 5:41 PM, Karl Tomlinson  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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Gregory Szorc]

On Tue, Jul 7, 2015 at 5:13 PM, Jeff Gilbert  wrote:

> On Tue, Jul 7, 2015 at 3:59 PM, Eric Rahm  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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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



On Jul 7, 2015, at 20:41 , Karl Tomlinson  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?
> 
> 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.
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Nicholas Nethercote]

On Tue, Jul 7, 2015 at 5:13 PM, Jeff Gilbert  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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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  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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Jeff Gilbert]

On Tue, Jul 7, 2015 at 3:59 PM, Eric Rahm  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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Gregory Szorc]

On Tue, Jul 7, 2015 at 1:03 PM, smaug  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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Xidorn Quan]

On Tue, Jul 7, 2015 at 1:12 PM, Jeff Gilbert  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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Bobby Holley]

On Tue, Jul 7, 2015 at 3:59 PM, Eric Rahm  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-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Eric Rahm]

On Tuesday, July 7, 2015 at 3:30:59 PM UTC-7, Birunthan Mohanathas wrote:
> On 7 July 2015 at 15:02, Mike Hommey  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.

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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Birunthan Mohanathas]

On 7 July 2015 at 15:02, Mike Hommey  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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Bobby Holley]

On Tue, Jul 7, 2015 at 3:02 PM, Mike Hommey  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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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.


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  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?
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform




___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Kartikaya Gupta]

On Tue, Jul 7, 2015 at 3:33 PM, Jeff Gilbert  wrote:
> On Tue, Jul 7, 2015 at 6:03 AM, 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)? 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.

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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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  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?
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Nicholas Hurley]

On Mon, Jul 6, 2015 at 8: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.
>

I don't always respond in threads like this, but when I do, it's to say a
big +1 to no more aFoo.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Jeff Gilbert]

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  wrote:

> On 07/07/2015 11:18 PM, Jeff Gilbert wrote:
>
>> On Tue, Jul 7, 2015 at 1:03 PM, smaug  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.
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Jeff Gilbert]

On Tue, Jul 7, 2015 at 1:20 PM, smaug  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?
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Jet Villegas]

On Mon, Jul 6, 2015 at 8:12 PM, Jeff Gilbert  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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[smaug]

On 07/07/2015 11:18 PM, Jeff Gilbert wrote:

On Tue, Jul 7, 2015 at 1:03 PM, smaug  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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[smaug]

On 07/07/2015 10:55 PM, Jeff Gilbert wrote:

On Tue, Jul 7, 2015 at 12:36 PM, Honza Bambas  wrote:


On 7/7/2015 21:27, Jeff Gilbert wrote:


On Tue, Jul 7, 2015 at 4:54 AM, Honza Bambas  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.



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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Jeff Gilbert]

On Tue, Jul 7, 2015 at 1:03 PM, smaug  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?
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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


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



___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Jeff Gilbert]

On Tue, Jul 7, 2015 at 12:36 PM, Honza Bambas  wrote:

> On 7/7/2015 21:27, Jeff Gilbert wrote:
>
>> On Tue, Jul 7, 2015 at 4:54 AM, Honza Bambas  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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Honza Bambas]

On 7/7/2015 21:27, Jeff Gilbert wrote:

On Tue, Jul 7, 2015 at 4:54 AM, Honza Bambas  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 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.



___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Jeff Gilbert]

On Tue, Jul 7, 2015 at 6:03 AM, 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)? 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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Jeff Gilbert]

On Tue, Jul 7, 2015 at 4:54 AM, Honza Bambas  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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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.)

On Tue, Jul 7, 2015 at 7:38 AM, Boris Zbarsky  wrote:

> 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
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Honza Bambas]

On 7/7/2015 19:38, Boris Zbarsky wrote:

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?


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

-hb-



-Boris
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform



___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Eric Shepherd]

> Karl Tomlinson 
> 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-platform@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 
Blog: http://www.bitstampede.com/
Twitter: http://twitter.com/sheppy
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Gregory Szorc]

On Tue, Jul 7, 2015 at 9:29 AM, Nathan Froyd  wrote:

> On Tue, Jul 7, 2015 at 11:49 AM, Mike Conley  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.


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.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Eric Rescorla]

On Tue, Jul 7, 2015 at 6:03 AM, 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.

-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.
>
> 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
>
>
> On Tue, Jul 7, 2015 at 7:54 AM, Honza Bambas  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-
> >
> >
> > On 7/7/2015 5:12, 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
> >> ___
> >> dev-platform mailing list
> >> dev-platform@lists.mozilla.org
> >> https://lists.mozilla.org/listinfo/dev-platform
> >>
> >
> > ___
> > dev-platform mailing list
> > dev-platform@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-platform
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Nathan Froyd]

On Tue, Jul 7, 2015 at 11:49 AM, Mike Conley  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
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[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?

On 07/07/2015 11:44 AM, Boris Zbarsky wrote:
> 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
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform