Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++
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 tzimmerm...@mozilla.com 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++
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++
On Thu, Jul 16, 2015 at 6:50 AM, Ehsan Akhgari ehsan.akhg...@gmail.com 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++)
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: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)
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: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++
Hi Am 16.07.2015 um 00:47 schrieb Jeff Gilbert: On Tue, Jul 14, 2015 at 7:55 AM, Thomas Zimmermann tzimmerm...@mozilla.com 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++)
On Tue, Jul 14, 2015 at 9:17 PM, Nicholas Nethercote n.netherc...@gmail.com wrote: On Tue, Jul 14, 2015 at 8:06 PM, Bobby Holley bobbyhol...@gmail.com 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++)
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 bobbyhol...@gmail.com 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 benja...@smedbergs.us 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++)
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: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)
On Wed, Jul 15, 2015 at 2:26 PM, Gregory Szorc g...@mozilla.com 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++)
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: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++
On 2015-07-14 9:59 AM, Kyle Huey wrote: On Tue, Jul 14, 2015 at 6:57 AM, Ehsan Akhgari ehsan.akhg...@gmail.com 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 tzimmerm...@mozilla.com 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++
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++
On Tue, Jul 14, 2015 at 10:10 AM, Tom Tromey ttro...@mozilla.com wrote: That assumes that the 'Foo' of aFoo is stable across function boundaries, which is not always the case. Ehsan No, it doesn't. In the scenario above, all you're looking for is when Ehsan a value was computed, so you can quickly see an aDuck, aQuack, Ehsan aFoopyFoo and determine that the value was passed down from the Ehsan caller, until you get to a call site which passes in something that Ehsan doesn't start with an 'a'. It was mentioned elsewhere in this thread that some code assigns to arguments. In these cases going up to the point of origination may miss the spot that actually introduced the value. Admittedly not perfect, but as a first-order approximation: kats@kgupta-air mozilla-git$ find . -name *.cpp | xargs grep ^ *a[A-Z]\w* = | wc -l 5414 That's not a lot considering the size of the codebase. ___ 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++)
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++
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++
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. ___ 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++
On Tue, Jul 14, 2015 at 6:57 AM, Ehsan Akhgari 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 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++
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: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)
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
Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)
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++)
On Jul 14, 2015, at 8:23 AM, Benjamin Smedberg benja...@smedbergs.us wrote: * no more ns prefix Are people still creating new classes with an ’ns’ prefix? Surely this is something we can drop right away, at least for new code. Much of the codebase already does not use this style. We have namespaces now, after all. - Seth ___ 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++
(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 dba...@dbaron.org 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++
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++
Hi Am 13.07.2015 um 21:07 schrieb Jeff Gilbert: On Sun, Jul 12, 2015 at 6:45 PM, Thomas Zimmermann tzimmerm...@mozilla.com 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++
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: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)
On Tue, Jul 14, 2015 at 9:06 AM, David Major dma...@mozilla.com wrote: May I request that the major parts of this not happen until we have a blame that can see through such changes. Last I checked, gps had some ideas in that space but lacked time to implement. I spoke to a Mercurial maintainer about some of my ideas and the feature would be welcome upstream. However, there isn't enough time to land for Mercurial 3.5 (code freeze this week), so we'd have to wait until Mercurial 3.6 (November 1 release date) at the earliest. That being said, every other organizations in the world is using the same or similar tools and is faced with similar challenges. Lack of a commit-skipping feature doesn't hinder other organizations from performing major refactorings. So while I'd love to make the tools better, I don't think waiting on the tools should be a blocker to mass reformatting the tree. While I'm here, if anyone has suggestions for quick fixes to https://hg.mozilla.org/'s HTML output, those are relatively easy to make. Please file bugs against Developer Services :: hg.mozilla.org if you have ideas! And, if you want to hack on improvements yourself, https://mozilla-version-control-tools.readthedocs.org/en/latest/hgmo/contributing.html#hacking-the-theming tells you how. I reckon we could come up with enough small changes to hold us over until commit skipping in blame is implemented. On Wed, Jul 15, 2015, at 03:23 AM, Benjamin Smedberg wrote: On 7/8/2015 7:31 AM, Nathan Froyd wrote: If somebody is willing to do the formatting, I'm willing to do the review. I think the thread has reached the point of people repeating ad nauseum what was already said earlier in the thread, so it's time for a decision. Benjamin? Aww, I was avoiding getting into this thread. I personally have no strong preference, and our existing community is pretty deeply divided. I doubt we're going to come to consensus here, and this is a pretty tough decision to make on its own. I do believe that consistency trumps module/personal preference in terms of coding style. The argument I am most sympathetic to is that this convention is a barrier to new contributors. Making new contributors productive, both employees and volunteers, is a very good reason to choose one style over another. Given that premise, we shouldn't just change aArgument; we should adopt the Google C++ style guide wholesale: * names_with_underscores * members_with_trailing_ * no more ns prefix 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++)
On Tue, Jul 14, 2015 at 10:06 AM, Gregory Szorc g...@mozilla.com wrote: That being said, every other organizations in the world is using the same or similar tools and is faced with similar challenges. Lack of a commit-skipping feature doesn't hinder other organizations from performing major refactorings. So while I'd love to make the tools better, I don't think waiting on the tools should be a blocker to mass reformatting the tree. This. If blame is the only victim, and a temporary one, then that's a pretty small price to pay. ___ 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 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++)
On Wed, Jul 15, 2015 at 3:40 AM, Joshua Cranmer pidgeo...@gmail.com 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: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)
On 14 July 2015 at 08:23, Benjamin Smedberg benja...@smedbergs.us 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++
On Wed, Jul 15, 2015 at 2:55 AM, Thomas Zimmermann 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. 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++)
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 benja...@smedbergs.us 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++)
On Tue, Jul 14, 2015 at 8:06 PM, Bobby Holley bobbyhol...@gmail.com 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++)
On Tue, Jul 14, 2015 at 8:23 AM, Benjamin Smedberg benja...@smedbergs.us 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++)
On 07/14/2015 08:11 PM, Martin Thomson wrote: On Tue, Jul 14, 2015 at 10:06 AM, Gregory Szorc g...@mozilla.com wrote: That being said, every other organizations in the world is using the same or similar tools and is faced with similar challenges. Lack of a commit-skipping feature doesn't hinder other organizations from performing major refactorings. So while I'd love to make the tools better, I don't think waiting on the tools should be a blocker to mass reformatting the tree. This. If blame is the only victim, and a temporary one, then that's a pretty small price to pay. 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: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++
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 jgilb...@mozilla.com wrote: Arguments have the same lifetimes as locals, and the only exceptions to this apply to both args and locals. (references and pointers) Maybe I've misunderstood what you're saying here, but locals are frequently block-scoped which gives them a different lifetime to args. 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++
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? ___ 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++
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++
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++
On Thu, Jul 9, 2015 at 7:01 PM, Jeff Gilbert jgilb...@mozilla.com wrote: Arguments have the same lifetimes as locals, and the only exceptions to this apply to both args and locals. (references and pointers) Maybe I've misunderstood what you're saying here, but locals are frequently block-scoped which gives them a different lifetime to args. Nick ___ 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++
On Thu, Jul 9, 2015 at 7:01 PM, Jeff Gilbert jgilb...@mozilla.com wrote: FWIW, I hacked a python script that does s/aF[Oo]o/foo/ poiru has the tools to do this already, as mentioned above. , and MSVC does has a code analysis mode that supports an error-on-warn for shadowed variable names. I can take a look at how bad the shadowing is in the tree. Probably worth taking a look at bug 563195. bholley ___ 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++
On Wed, Jul 8, 2015 at 7:48 AM, smaug opet...@mozilla.com wrote: Another example where aFoo tends to be rather useful is lifetime management. If I see aFoo being used somewhere in a method after some unsafe method call (layout flush, any script callback handling, event dispatch, observer service notification etc.), I know that I need to check that the _caller_ follows COM-rules and keeps aFoo object alive during the method call. With non-aFoo variables I know the lifetime is controlled within the method. Arguments have the same lifetimes as locals, and the only exceptions to this apply to both args and locals. (references and pointers) There are the same problems with locals as there are with args, as args are just locals passed into the function. FWIW, I hacked a python script that does s/aF[Oo]o/foo/, and MSVC does has a code analysis mode that supports an error-on-warn for shadowed variable names. I can take a look at how bad the shadowing is in the tree. ___ 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++
On Wed, Jul 8, 2015 at 6:52 AM, Gabor Krizsanits gkrizsan...@mozilla.com wrote: On Wed, Jul 8, 2015 at 1:05 AM, Bobby Holley bobbyhol...@gmail.com wrote: The priority is to automatically rewrite our source with a unified style. foo - aFoo is reasonably safe, whereas aFoo-foo is not, at least with the current tools. So we either need to combine the rewrite tools with static analysis, or just go with aFoo. ___ dev-platform mailing list dev-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++
On Wed, Jul 8, 2015 at 1:05 AM, Bobby Holley bobbyhol...@gmail.com wrote: On Tue, Jul 7, 2015 at 3:59 PM, Eric Rahm er...@mozilla.com wrote: I'm not a huge fan of the 'aFoo' style, but I am a huge fan of consistency. So if we want to change the style guide we should update our codebase, and I don't think we can reasonably do that automatically without introducing shadowing issues. This a great point, and perhaps the most useful one to be raised in this thread. The priority is to automatically rewrite our source with a unified style. foo - aFoo is reasonably safe, whereas aFoo-foo is not, at least with the current tools. So we either need to combine the rewrite tools with static analysis, or just go with aFoo. ___ dev-platform mailing list dev-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++
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 mozn...@karlt.net wrote: Jeff Gilbert writes: It can be a burden on the hundreds of devs who have to read and understand the code in order to write more code. Some people find the prefix helps readability, because it makes extra information immediately available in the code being examined, while you are indicating that this is a significant burden on readability. Can you explain why the extra letter is a significant burden? 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++
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++
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 opet...@mozilla.com wrote: On 07/07/2015 11:18 PM, Jeff Gilbert wrote: On Tue, Jul 7, 2015 at 1:03 PM, smaug opet...@mozilla.com wrote: As someone who spends more than 50% of working time doing reviews I'm strongly against this proposal. aFoo helps with readability - reader knows immediately when the code is dealing with arguments. When and why is this useful to know? Most common case in Gecko is to know that one is assigning value to outparam. Another example where aFoo tends to be rather useful is lifetime management. If I see aFoo being used somewhere in a method after some unsafe method call (layout flush, any script callback handling, event dispatch, observer service notification etc.), I know that I need to check that the _caller_ follows COM-rules and keeps aFoo object alive during the method call. With non-aFoo variables I know the lifetime is controlled within the method. ___ 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++
On Wed, Jul 8, 2015 at 3:52 AM, Gabor Krizsanits gkrizsan...@mozilla.com wrote: The priority is to automatically rewrite our source with a unified style. foo - aFoo is reasonably safe, whereas aFoo-foo is not, at least with the current tools. So we either need to combine the rewrite tools with static analysis, or just go with aFoo. ___ dev-platform mailing list dev-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++
On 7/8/2015 3:08, Gregory Szorc wrote: On Tue, Jul 7, 2015 at 5:13 PM, Jeff Gilbert jgilb...@mozilla.com wrote: On Tue, Jul 7, 2015 at 3:59 PM, Eric Rahm er...@mozilla.com wrote: I'm not a huge fan of the 'aFoo' style, but I am a huge fan of consistency. So if we want to change the style guide we should update our codebase, and I don't think we can reasonably do that automatically without introducing shadowing issues. MSVC 2013 (which I believe is our main windows compiler right now) will error during compilation if such a shadowing issue arises. Thus, if the code compiles there, `aFoo`-`foo` is safe. I would be very surprised if GCC or Clang didn't have an equivalent option. Additionally I don't spend 50% of my time reviewing, so I'd say my opinion here (meh to aFoo) is less important. It's not an undue burden for me to include an aPrefix and if we have static analysis to check for it that would make it even less of an issue. It can be a burden on the hundreds of devs who have to read and understand the code in order to write more code. With the exception of a couple people, review is not the bottleneck. The opinions of a few over-harried reviewers should not hold undue sway over the many many devs writing code. I somewhat disagree. There will always be fewer code reviewers than contributors. And, code reviewers tend to be more senior people. The time of a code reviewer thus tends to be more valuable than the time of the average code author. Coupled with the fact that code review is a barrier to landing, this translates to an incentive to make the lives and workflows of code reviewers as frictionless as possible. I feel strongly that the bandwidth limitations of code reviewers does dictate to some extent how code is written. For example, I feel that authors should spend extra effort to write detailed commit messages and split work into multiple, easier-to-review commits, as these can drastically reduce the time it takes for review. How much this reasoning extends to style and things like aFoo, I'm not sure. But if I hear a frequent code reviewer say X makes review easier, I tend to take that opinion more seriously than that of a non-reviewer. +1 Few more cents: A year ago I was not using a in my code. Then I start using it. And I got used to it! And - when used consistently - find it useful from time to time to quickly get the this is an arg variable info, both as a reviewer and a coder. So, year ago I would probably be on the get rid of it side. Now I'm definitely on the use it everywhere side. Cheers -hb- ___ 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++
On Wed, Jul 8, 2015 at 4:45 PM, Karl Tomlinson mozn...@karlt.net wrote: I think we could relax the 'a' prefix requirement to be a convention used when identifying the variable as a parameter is useful. My opinion is that this is useful for most parameters in all non-trivial functions, but others disagree. I don't think that helps. One of the primary benefits of style rules is that they eliminate (or rather, reduce) the uncertainty and churn that comes from different reviewers having different preferences. Suppose I add a new method signature in a patch. If Alice finds a-prefixing useful and Bob does not, my likelihood of receiving a review nit depends on who's reviewing it - i.e. per-reviewer style guides, which are strictly worse than per-module style guides, which are strictly worse than a single style guide. bholley ___ 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 writes: On Wed, Jul 8, 2015 at 4:45 PM, Karl Tomlinson mozn...@karlt.net wrote: I think we could relax the 'a' prefix requirement to be a convention used when identifying the variable as a parameter is useful. My opinion is that this is useful for most parameters in all non-trivial functions, but others disagree. I don't think that helps. One of the primary benefits of style rules is that they eliminate (or rather, reduce) the uncertainty and churn that comes from different reviewers having different preferences. Suppose I add a new method signature in a patch. If Alice finds a-prefixing useful and Bob does not, my likelihood of receiving a review nit depends on who's reviewing it - i.e. per-reviewer style guides, which are strictly worse than per-module style guides, which are strictly worse than a single style guide. 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 writes: On Wed, Jul 8, 2015 at 3:52 AM, Gabor Krizsanits gkrizsan...@mozilla.com wrote: The priority is to automatically rewrite our source with a unified style. foo - aFoo is reasonably safe, whereas aFoo-foo is not, at least with the current tools. So we either need to combine the rewrite tools with static analysis, or just go with aFoo. ___ dev-platform mailing list dev-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++
On Tue, Jul 7, 2015 at 11:49 AM, Mike Conley mcon...@mozilla.com wrote: Aren't there tools for our (admittedly varied) editors / IDEs to make the readability that people are getting from aFoo readily available, but that don't also require us to pack it into the actual name of the variable? I find it useful for reading code in an editor, but I also find it useful for reviewing patches when I don't have full context of the function. Perhaps that will change with ReviewBoard and its ability to easily provide context...or if ReviewBoard learned how to provide this variable is a (local|global|argument), set on line X tooltips or something. -Nathan ___ 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 mailto:mozn...@karlt.net July 7, 2015 at 12:55 AM I find the 'a' prefix useful to tell me that this variable has the value that was provided to the function. (I'm assuming that the prefix is used with this convention.) There's no additional safety enforced, but I find the single letter helps readability. For example, if I want to know where the value of a variable comes from, and it starts with 'a', then I know immediately that I can skip looking at that particular function in the call chain/graph and I need to look at the calling function(s). That advantage does not exist in a declaration that is not a definition; this is only helpful in the definition. ___ dev-platform mailing list dev-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 https://www.mozilla.org/ 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++
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++
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
Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++
On Tue, Jul 7, 2015 at 6:03 AM, Kartikaya Gupta kgu...@mozilla.com wrote: I'd be interested to know: of those people who are in favour of removing the prefix, how many regularly have to deal with functions that are longer than two pages (a page is however much code you can see at a time in your coding environment)? All the time. -Ekr I'd be happy to support removing the prefix if people also commit to splitting any giant functions they touch as part of the prefix removal. 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 hbam...@mozilla.com wrote: I'm strongly against removing the prefix. I got used to this and it has its meaning all the time I inspect code (even my own) and doing reviews. Recognizing a variable is an argument is very very useful. It's important to have it and it's good we enforce it! -hb- 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++
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
Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++
I agree with Karl that, the 'a' prefix sometimes helps me in that way when I read code. Also it is sometimes convenient to have a local variable use the parameter name without the prefix, like: SomeType foo = CastOrUnwrap(aFoo); I don't have strong opinion on this, though. If the majority of the industry disagrees with this style, probably we should change. - Xidorn On Tue, Jul 7, 2015 at 1:12 PM, Jeff Gilbert jgilb...@mozilla.com wrote: I propose that we stop recommending the universal use of an 'a' prefix for arguments to functions in C and C++. If the prefix helps with disambiguation, that's fine. However, use of this prefix should not be prescribed in general. `aFoo` does not provide any additional safety that I know of.[1] As a superfluous prefix, it adds visual noise, reducing immediate readability of all function declarations and subsequent usage of the variables within the function definition. 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
Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++
On Tue, Jul 7, 2015 at 4:54 AM, Honza Bambas hbam...@mozilla.com wrote: I'm strongly against removing the prefix. I got used to this and it has its meaning all the time I inspect code (even my own) and doing reviews. Recognizing a variable is an argument is very very useful. It's important to have it and it's good we enforce it! -hb- Please expand on this. I review a lot of code, and can't remember the last time knowing a var was an arg was useful. The only exception is 'out-vars', which I believe should be handled as a different case. (WebGL code generally uses the out_ prefix) `aFoo` does not discriminate, so it's impossible to tell if assignment to `aFoo` is local or not without more context. ___ 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++
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++
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++
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 bzbar...@mit.edu 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++
On Tue, Jul 7, 2015 at 6:03 AM, Kartikaya Gupta kgu...@mozilla.com wrote: I'd be interested to know: of those people who are in favour of removing the prefix, how many regularly have to deal with functions that are longer than two pages (a page is however much code you can see at a time in your coding environment)? I'd be happy to support removing the prefix if people also commit to splitting any giant functions they touch as part of the prefix removal. I work with a number of these, but after a page or two, why is it at all relevant which vars were args? For information flow? Should we mark locals that purely derive from args as `aFoo` as well? Long functions (which have poor readability anyway) generally have so much going on that the trivia of which vars are args does not seem very useful.. I do not see how `aFoo` helps here, so please expand on this. ___ 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++
On 7/7/2015 21:27, Jeff Gilbert wrote: On Tue, Jul 7, 2015 at 4:54 AM, Honza Bambas hbam...@mozilla.com wrote: I'm strongly against removing the prefix. I got used to this and it has its meaning all the time I inspect code (even my own) and doing reviews. Recognizing a variable is an argument is very very useful. It's important to have it and it's good we enforce it! -hb- Please expand on this. Not sure how. I simply find it useful since I was once forced to obey it strictly in a dom code. It simply has its meaning. It helps to orient. I don't know what more you want from me to hear. I 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++
On Tue, Jul 7, 2015 at 1:03 PM, smaug opet...@mozilla.com wrote: As someone who spends more than 50% of working time doing reviews I'm strongly against this proposal. aFoo helps with readability - reader knows immediately when the code is dealing with arguments. When and why is this useful to know? ___ 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++
On 07/07/2015 10:55 PM, Jeff Gilbert wrote: On Tue, Jul 7, 2015 at 12:36 PM, Honza Bambas hbam...@mozilla.com wrote: On 7/7/2015 21:27, Jeff Gilbert wrote: On Tue, Jul 7, 2015 at 4:54 AM, Honza Bambas hbam...@mozilla.com wrote: I'm strongly against removing the prefix. I got used to this and it has its meaning all the time I inspect code (even my own) and doing reviews. Recognizing a variable is an argument is very very useful. It's important to have it and it's good we enforce it! -hb- Please expand on this. Not sure how. I simply find it useful since I was once forced to obey it strictly in a dom code. It simply has its meaning. It helps to orient. I don't know what more you want from me to hear. I would like to have reasons why 'we' feel it's necessary or helpful when the rest of the industry (and nearly half our own company) appears to do fine without it. If we deviate from widespread standards, we should have reasons to back our deviation. More acutely, my module does not currently use `aFoo`, and our (few) contributors do not use use or like it. `aFoo` gets in the way for us. Recently, there has been pressure to unify the module's style with the rest of Gecko. The main complaint I have with Gecko style is `aFoo` being required. Vague desires for `aFoo` are not compelling. There needs to be solid reasons. If there are no compelling reasons, the requirement should be removed. We have deprecated style before, and we can do it again. 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++
On Mon, Jul 6, 2015 at 8:12 PM, Jeff Gilbert jgilb...@mozilla.com wrote: I propose we strike the `aFoo` recommendation from the Mozilla style guide. Just so the proposal doesn't get lost in the bike shed, Jeff is only proposing a change to the style guide, not a tree-wide find/replace project. I take that to mean that When in Rome still applies to existing C++ code. Do we have consensus on that part? --Jet ___ 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++
On Mon, Jul 6, 2015 at 8:12 PM, Jeff Gilbert jgilb...@mozilla.com wrote: I propose that we stop recommending the universal use of an 'a' prefix for arguments to functions in C and C++. If the prefix helps with disambiguation, that's fine. However, use of this prefix should not be prescribed in general. I don't always respond in threads like this, but when I do, it's to say a big +1 to no more aFoo. ___ 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++
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++
On Tue, Jul 7, 2015 at 1:20 PM, smaug opet...@mozilla.com wrote: readability / easier to follow the dataflow are rather compelling reasons. It hurts readability for me and many others. I don't see how it revolutionizes following dataflow, since we have locals that are pure functions of args, but yet are not marked aFoo. Outvars are a different beast, and in at least WebGL code, are marked as such. (`out_` prefix) `aFoo` is not a good solution for outvars. If outvars are the main reason for `aFoo`, we should stop using `aFoo` for all arguments, and only mark outvars. I selfishly try to get the time I spend on reviewing a patch shorter, and aFoo helps with that. It hinders my patch reviewing. I've been speaking to those around me, and they do not see any value in differentiating args from locals. (args are just locals, anyway) Though even more important is consistent coding style everywhere (per programming language). Why don't we come into consistency with the industry at large, and also the number of internal Mozilla projects which choose not to use `aFoo`. I have found no other style guide that recommends `aFoo`. Why are we different? Why do we accept reduced readability for all external contributors? Why do so many other Mozilla projects not use this alleged readability aid? ___ 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++
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 jgilb...@mozilla.com wrote: ... I have found no other style guide that recommends `aFoo`. Why are we different? Why do we accept reduced readability for all external contributors? Why do so many other Mozilla projects not use this alleged readability aid? ___ 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++
On Tue, Jul 7, 2015 at 3:33 PM, Jeff Gilbert jgilb...@mozilla.com wrote: On Tue, Jul 7, 2015 at 6:03 AM, Kartikaya Gupta kgu...@mozilla.com wrote: I'd be interested to know: of those people who are in favour of removing the prefix, how many regularly have to deal with functions that are longer than two pages (a page is however much code you can see at a time in your coding environment)? I'd be happy to support removing the prefix if people also commit to splitting any giant functions they touch as part of the prefix removal. I work with a number of these, but after a page or two, why is it at all relevant which vars were args? For information flow? Should we mark locals that purely derive from args as `aFoo` as well? Long functions (which have poor readability anyway) generally have so much going on that the trivia of which vars are args does not seem very useful.. I do not see how `aFoo` helps here, so please expand on this. 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++
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 opet...@mozilla.com wrote: On 07/07/2015 11:18 PM, Jeff Gilbert wrote: On Tue, Jul 7, 2015 at 1:03 PM, smaug opet...@mozilla.com wrote: As someone who spends more than 50% of working time doing reviews I'm strongly against this proposal. aFoo helps with readability - reader knows immediately when the code is dealing with arguments. When and why is this useful to know? Most common case in Gecko is to know that one is assigning value to outparam. ___ 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++
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 jgilb...@mozilla.com wrote: ... I have found no other style guide that recommends `aFoo`. Why are we different? Why do we accept reduced readability for all external contributors? Why do so many other Mozilla projects not use this alleged readability aid? ___ 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++
(Posted this reply to the wrong thread, reposting to the right one... _) One more group of defectors within Mozilla. From the DevTools coding standards[0]: - aArguments aAre the aDevil (don't use them please) Although, there are still some files in tree with the legacy style. [0] https://wiki.mozilla.org/DevTools/CodingStandards#Code_style On Tue, Jul 7, 2015 at 6:57 AM, Kartikaya Gupta kgu...@mozilla.com wrote: On Tue, Jul 7, 2015 at 9:18 AM, Honza Bambas hbam...@mozilla.com wrote: I'd be happy to support removing the prefix if people also commit to splitting any giant functions they touch as part of the prefix removal. That's (sorry) non-sense. In almost all cases longer methods/functions cannot be just split. It would probably make the code just much harder to read and maintain (with a lot of new arguments missing the 'a' prefix ;)) and is not necessary. Not an argument IMHO. Can you point me to a couple of examples of long functions that you think cannot be split reasonably? I'm curious to see what it looks like. Obviously functions with giant switch statements and the like are exceptions and should be treated exceptionally but I would like to see some regular functions that can't be split. Cheers, kats ___ 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++
FWIW, I did a quick poll of the people in our Gfx daily. Here are the results: For aArguments: Bas Milan Matt Kats Against aArguments: Me No strong opinion: Sotoro Lee Benoit Nical Mason -Jeff On Tue, Jul 7, 2015 at 11:12 AM, Nick Fitzgerald nfitzger...@mozilla.com wrote: (Posted this reply to the wrong thread, reposting to the right one... _) One more group of defectors within Mozilla. From the DevTools coding standards[0]: - aArguments aAre the aDevil (don't use them please) Although, there are still some files in tree with the legacy style. [0] https://wiki.mozilla.org/DevTools/CodingStandards#Code_style On Tue, Jul 7, 2015 at 6:57 AM, Kartikaya Gupta kgu...@mozilla.com wrote: On Tue, Jul 7, 2015 at 9:18 AM, Honza Bambas hbam...@mozilla.com wrote: I'd be happy to support removing the prefix if people also commit to splitting any giant functions they touch as part of the prefix removal. That's (sorry) non-sense. In almost all cases longer methods/functions cannot be just split. It would probably make the code just much harder to read and maintain (with a lot of new arguments missing the 'a' prefix ;)) and is not necessary. Not an argument IMHO. Can you point me to a couple of examples of long functions that you think cannot be split reasonably? I'm curious to see what it looks like. Obviously functions with giant switch statements and the like are exceptions and should be treated exceptionally but I would like to see some regular functions that can't be split. Cheers, kats ___ 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++
On Tuesday, July 7, 2015 at 3:30:59 PM UTC-7, Birunthan Mohanathas wrote: On 7 July 2015 at 15:02, Mike Hommey m...@glandium.org wrote: On Tue, Jul 07, 2015 at 11:52:12PM +0300, smaug wrote: until the tools (and poiru) are run and make the code follow Mozilla coding style. Assuming you're talking about clang-format, that doesn't take care about anything else than whitespaces. I have a Clang tool to add the 'a' prefix, but it can be easily modified to drop the prefix should we decide to do so. 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++
On Tue, Jul 7, 2015 at 1:12 PM, Jeff Gilbert jgilb...@mozilla.com wrote: Notable works or style guides which *do* recommend `aFoo`: * Mozilla (except for IDL, Java, and Python) * ? Just FYI, Someone in Twitter mentioned that, code generated by Xcode uses this style by default. The languages are Obj-C and Swift, though. Examples like: https://github.com/search?l=objective-cq=aNotificationref=searchresultstype=Codeutf8= - 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++
On Tue, Jul 7, 2015 at 1:03 PM, smaug sm...@welho.com wrote: As someone who spends more than 50% of working time doing reviews I'm strongly against this proposal. aFoo helps with readability - reader knows immediately when the code is dealing with arguments. I'd like to point out that MozReview allows you to expand context when doing code reviews. This means the necessity of extra hints in naming conventions (such as aFoo) loses its importance. The argument that aFoo assists with readability, while historically accurate when applied to review tools such as Splinter that rely on patch/diff context, is thus somewhat undermined by the employment of modern code review tool. ___ 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++
On Tue, Jul 7, 2015 at 3:02 PM, Mike Hommey m...@glandium.org wrote: On Tue, Jul 07, 2015 at 11:52:12PM +0300, smaug wrote: On 07/07/2015 11:45 PM, Milan Sreckovic wrote: Removing the style guide for “prefix function arguments with a” will not preclude people from naming a variable aFoo. At least the current style guide precludes people from naming non-function arguments that way, albeit indirectly. I’m trying to understand the possible outcomes of this particular conversation: a) Nothing happens. We leave a prefix in the style guide, some code ignores it, some follows it. until the tools (and poiru) are run and make the code follow Mozilla coding style. Assuming you're talking about clang-format, that doesn't take care about anything else than whitespaces. poiru built tools to automatically make all the adjustments that we prescribe in the style guide, and has been iteratively running them on various modules (thanks poiru!), so we should assume that whatever style we pick for the style guide will end up applying to all/most of our code in the not-too-distant future. This is a good thing - specifying things in the style guide but keeping the tree inconsistent on the grounds of when in Rome is the wrong approach. This was discussed to death in various threads already, and the conclusion is that we are actively moving towards a unified style in Gecko and SM. bholley ___ 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++
On 7 July 2015 at 15:02, Mike Hommey m...@glandium.org wrote: On Tue, Jul 07, 2015 at 11:52:12PM +0300, smaug wrote: until the tools (and poiru) are run and make the code follow Mozilla coding style. Assuming you're talking about clang-format, that doesn't take care about anything else than whitespaces. I have a Clang tool to add the 'a' prefix, but it can be easily modified to drop the prefix should we decide to do so. ___ 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++
On Tue, Jul 7, 2015 at 3:59 PM, Eric Rahm er...@mozilla.com wrote: I'm not a huge fan of the 'aFoo' style, but I am a huge fan of consistency. So if we want to change the style guide we should update our codebase, and I don't think we can reasonably do that automatically without introducing shadowing issues. MSVC 2013 (which I believe is our main windows compiler right now) will error during compilation if such a shadowing issue arises. Thus, if the code compiles there, `aFoo`-`foo` is safe. I would be very surprised if GCC or Clang didn't have an equivalent option. Additionally I don't spend 50% of my time reviewing, so I'd say my opinion here (meh to aFoo) is less important. It's not an undue burden for me to include an aPrefix and if we have static analysis to check for it that would make it even less of an issue. It can be a burden on the hundreds of devs who have to read and understand the code in order to write more code. With the exception of a couple people, review is not the bottleneck. The opinions of a few over-harried reviewers should not hold undue sway over the many many devs writing code. ___ 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++
On Tue, Jul 07, 2015 at 05:09:57PM -0700, Gregory Szorc wrote: On Tue, Jul 7, 2015 at 1:03 PM, smaug sm...@welho.com wrote: As someone who spends more than 50% of working time doing reviews I'm strongly against this proposal. aFoo helps with readability - reader knows immediately when the code is dealing with arguments. I'd like to point out that MozReview allows you to expand context when doing code reviews. This means the necessity of extra hints in naming conventions (such as aFoo) loses its importance. The argument that aFoo assists with readability, while historically accurate when applied to review tools such as Splinter that rely on patch/diff context, is thus somewhat undermined by the employment of modern code review tool. While mozreview helps to some degree, searching for the function declaration is not really something that is made easier by the context expansion feature it has. Mike ___ 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++
On 07/07/15 07:17, Eric Rescorla wrote: I am in favor of getting rid of aFoo. -Ekr P.S. At the risk of convincing people I am crazy and thus discounting my opinion above, I rather prefer foo_ to mFoo, but this seems like more a matter of taste. I agree that `aFoo` is only useful very marginally, and rather ugly. I also agree with Eric that `foo_` is somewhat nicer to read than `mFoo`, which introduces a weird cAmelCase, but I can live with it. For what it's worth, `this-foo` is also nice. Cheers, David -- David Rajchenbach-Teller, PhD Performance Team, Mozilla ___ 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++
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
Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++
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 mozn...@karlt.net wrote: Jeff Gilbert writes: It can be a burden on the hundreds of devs who have to read and understand the code in order to write more code. Some people find the prefix helps readability, because it makes extra information immediately available in the code being examined, while you are indicating that this is a significant burden on readability. Can you explain why the extra letter is a significant burden? 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++
On Tue, Jul 7, 2015 at 6:06 PM, Karl Tomlinson mozn...@karlt.net 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 writes: On Tue, Jul 7, 2015 at 5:41 PM, Karl Tomlinson mozn...@karlt.net wrote: Some people find the prefix helps readability, because it makes extra information immediately available in the code being examined, while you are indicating that this is a significant burden on readability. Can you explain why the extra letter is a significant burden? Because extra noise is being forced into variable names for minimal benefit. Every declaration is a sea of extra 'a's. Refactoring code means doing a lot of s/aFoo/foo/ and vice-versa. Reading each arg name requires first passing over 'a' before getting to anything relevant. Often this means that short function bodies can have every fifth or sixth letter being 'a'. I wouldn't see a problem with removing the 'a' prefix from parameter names in declarations and inline methods. ___ 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 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++
On Tue, Jul 7, 2015 at 5:13 PM, Jeff Gilbert jgilb...@mozilla.com wrote: On Tue, Jul 7, 2015 at 3:59 PM, Eric Rahm er...@mozilla.com wrote: I'm not a huge fan of the 'aFoo' style, but I am a huge fan of consistency. So if we want to change the style guide we should update our codebase, and I don't think we can reasonably do that automatically without introducing shadowing issues. MSVC 2013 (which I believe is our main windows compiler right now) will error during compilation if such a shadowing issue arises. Thus, if the code compiles there, `aFoo`-`foo` is safe. I would be very surprised if GCC or Clang didn't have an equivalent option. Additionally I don't spend 50% of my time reviewing, so I'd say my opinion here (meh to aFoo) is less important. It's not an undue burden for me to include an aPrefix and if we have static analysis to check for it that would make it even less of an issue. It can be a burden on the hundreds of devs who have to read and understand the code in order to write more code. With the exception of a couple people, review is not the bottleneck. The opinions of a few over-harried reviewers should not hold undue sway over the many many devs writing code. I somewhat disagree. There will always be fewer code reviewers than contributors. And, code reviewers tend to be more senior people. The time of a code reviewer thus tends to be more valuable than the time of the average code author. Coupled with the fact that code review is a barrier to landing, this translates to an incentive to make the lives and workflows of code reviewers as frictionless as possible. I feel strongly that the bandwidth limitations of code reviewers does dictate to some extent how code is written. For example, I feel that authors should spend extra effort to write detailed commit messages and split work into multiple, easier-to-review commits, as these can drastically reduce the time it takes for review. How much this reasoning extends to style and things like aFoo, I'm not sure. But if I hear a frequent code reviewer say X makes review easier, I tend to take that opinion more seriously than that of a non-reviewer. ___ 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++
On Tue, Jul 7, 2015 at 5:13 PM, Jeff Gilbert jgilb...@mozilla.com wrote: MSVC 2013 (which I believe is our main windows compiler right now) will error during compilation if such a shadowing issue arises. Thus, if the code compiles there, `aFoo`-`foo` is safe. I would be very surprised if GCC or Clang didn't have an equivalent option. They do, and dbaron mentioned them earlier. They're currently not on because there are quite a lot of warnings because there is quite a lot of shadowing occurring. (I have some experience with this thanks to bug 800659.) So I'm surprised by your claim that this is a non-issue. It can be a burden on the hundreds of devs who have to read and understand the code in order to write more code. With the exception of a couple people, review is not the bottleneck. The opinions of a few over-harried reviewers should not hold undue sway over the many many devs writing code. Reviewing code and reading code aren't that different, so let's not get hung up on the differences there. Some people like the aFoo style and find that it helps readability. It's largely a matter of personal taste; you can't just dismiss these people as being wrong. Nick ___ 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++
On Mon, Jul 6, 2015 at 8:26 PM, Mike Hommey m...@glandium.org wrote: The existence of aFoo goes along with the existence of mFoo, sFoo, kFoo, and others I might have forgotten. Not that I particularly care about aFoo, but why strike this one and not the others?[1] It's not like they have widespread use in the industry either. That is, the same reasoning could be applied to them, yet, you're stopping at aFoo. Why? mFoo and sFoo have very different scopes compared to locals, so calling them out is useful. kFoo makes it clear that the variable is constant, and has connotations regarding it being a hardcoded limit or value. Note that commonly kFoo is actually a static constant. Immutable (`const`) locals are often not kPrefixed. ___ 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 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++
+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++
+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++
On Tue, Jul 7, 2015 at 5:41 PM, Karl Tomlinson mozn...@karlt.net wrote: Jeff Gilbert writes: It can be a burden on the hundreds of devs who have to read and understand the code in order to write more code. Some people find the prefix helps readability, because it makes extra information immediately available in the code being examined, while you are indicating that this is a significant burden on readability. Can you explain why the extra letter is a significant burden? Because extra noise is being forced into variable names for minimal benefit. Every declaration is a sea of extra 'a's. Refactoring code means doing a lot of s/aFoo/foo/ and vice-versa. Reading each arg name requires first passing over 'a' before getting to anything relevant. Often this means that short function bodies can have every fifth or sixth letter being 'a'. Why does it matter that they're arguments? Arguments are just locals that are passed into functions. (outparams are a different beast already addressed elsewhere) I would like to reiterate that *we are unusual* in our present preference (not to mention requirement) for this in our style guideline. I'm not proposing a change to something bizarre. I'm proposing the removal of an extremely unusual style requirement. If the 'a' prefix is a burden then the 'm' prefix must be also, and so we should be using this-member instead of mMember. No, as this provides huge benefit by indicating when we are referencing a member variable, which differ greatly from local variables in scope. Arguments have the same scope as locals. There is benefit in mFoo, particularly compared to requiring this-foo, which I don't think we even have compiler or linter support for, and would clearly be superfluous in terms of extra characters. The opinions of a few over-harried reviewers should not hold undue sway over the many many devs writing code. unless people want code to be reviewed. `aFoo` is never going to make or break our ability to do code review. Low code-review bandwidth is all but completely orthogonal to this discussion. ___ 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++
On 7/7/2015 15:03, Kartikaya Gupta wrote: I'd be interested to know: of those people who are in favour of removing the prefix, how many regularly have to deal with functions that are longer than two pages (a page is however much code you can see at a time in your coding environment)? All the time? I'd be happy to support removing the prefix if people also commit to splitting any giant functions they touch as part of the prefix removal. That's (sorry) non-sense. In almost all cases longer methods/functions cannot be just split. It would probably make the code just much harder to read and maintain (with a lot of new arguments missing the 'a' prefix ;)) and is not necessary. Not an argument IMHO. -hb- Also FWIW in the current world I do find the prefix useful when debugging in gdb, because I know I can keep going up a frame as long as I'm tracing a variable with the prefix, whereas otherwise I would have to step backwards through each frame to see where the variable is coming from. I'll probably find some way to adapt if we remove the prefix though. kats On Tue, Jul 7, 2015 at 7:54 AM, Honza Bambas hbam...@mozilla.com wrote: I'm strongly against removing the prefix. I got used to this and it has its meaning all the time I inspect code (even my own) and doing reviews. Recognizing a variable is an argument is very very useful. It's important to have it and it's good we enforce it! -hb- 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++
On 07/07/2015 05:12, Jeff Gilbert wrote: Notable works or style guides [2] which do not recommend `aFoo`: [3] [...] To add another internal datapoint the FxOS gaia codebase is mostly devoid of this style. There are some places using the m prefix for pseudo member variables (really just JS attributes) but the a prefix for arguments is quite rare, I could find only a hundred uses of it or so. Gabriele signature.asc Description: OpenPGP digital signature ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
