Re: A proposal to reduce the number of styles in Mozilla code

Sounds good, and I'd include also js/* so that we had consistent style everywhere. It is rather painful to hack various non-js/* and js/* (xpconnect in my case) in the same patch. (I also happen to think that Mozilla coding style is inherently better than js style, since it has clear rules for n

PSA: Please stop using NULL in C++ code

With Birunthan's restless efforts in bug 784739, we have finally removed the usage of NULL in our C++ code. Please stop using NULL in new C++ code, and use nullptr instead! Cheers, -- Ehsan ___ dev-platform mailing list dev-pl

Re: A proposal to reduce the number of styles in Mozilla code

A concise summary of the changes you're proposing would be useful - here's my attempt at one. >From what I gather, the changes you're proposing to the style guide are: * remove implicit discouragement of changing code to conform to "Mozilla style" ** style changes should never be combined with fu

Re: PSA: Please stop using NULL in C++ code

On 1/6/2014, 11:19 AM, Gavin Sharp wrote: Seems like it would be a good idea to update https://developer.mozilla.org/en-US/docs/Developer_Guide/Coding_Style#C.2FC.2B.2B_practices accordingly. Good point, done! Cheers, Ehsan ___ dev-platform mailing

Re: A proposal to reduce the number of styles in Mozilla code

On 1/6/14 09:50, Gavin Sharp wrote: A concise summary of the changes you're proposing would be useful - here's my attempt at one. From what I gather, the changes you're proposing to the style guide are: * remove implicit discouragement of changing code to conform to "Mozilla style" ** style ch

Re: PSA: Please stop using NULL in C++ code

Seems like it would be a good idea to update https://developer.mozilla.org/en-US/docs/Developer_Guide/Coding_Style#C.2FC.2B.2B_practices accordingly. Gavin On Mon, Jan 6, 2014 at 10:12 AM, Ehsan Akhgari wrote: > With Birunthan's restless efforts in bug 784739, we have finally removed > the usage

Re: A proposal to reduce the number of styles in Mozilla code

I was listing only the policy changes, and would have lumped that one into "(other specifics about how this should be accomplished and with what exceptions may need elaborating)", but yes, it's certainly worth calling out explicitly. Gavin On Mon, Jan 6, 2014 at 11:07 AM, Adam Roach wrote: > O

Re: PSA: Please stop using NULL in C++ code

We also use NULL when generating JNI-interfacing code; I don't know if the patches in bug 784739 already cover these cases. [1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/jni-generator.py [2] http://mxr.mozilla.org/mozilla-central/source/build/annotationProcessors/CodeGenera

Re: A proposal to reduce the number of styles in Mozilla code

On 1/5/2014 8:34 PM, Nicholas Nethercote wrote: With these ideas in mind, a goal is clear: convert non-Mozilla-style code to Mozilla-style code, within reason. One thing that is worth pointing out is that there are three distinct classes of style rules, which have different implications for co

Re: A proposal to reduce the number of styles in Mozilla code

I think that this is a good start, but it doesn’t go quite far enough. Part of the problem with a policy that requires people to avoid reformatting of stuff they don’t touch is that it propagates formatting divergence. Sometimes because it’s better to conform to the style immediately adjacent t

Re: PSA: Please stop using NULL in C++ code

On 1/6/2014, 12:16 PM, Jim Chen wrote: We also use NULL when generating JNI-interfacing code; I don't know if the patches in bug 784739 already cover these cases. [1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/jni-generator.py [2] http://mxr.mozilla.org/mozilla-central/sou

Re: A proposal to reduce the number of styles in Mozilla code

I think trying to change SpiderMonkey style to Gecko isn't a great use of effort: * There are a lot of SpiderMonkey hackers who don't hack on anything else, and don't consider themselves Gecko hackers. Many of them don't read dev-platform, and would probably revolt if this were to occur. * SpiderMo

Re: PSA: Please stop using NULL in C++ code

On 1/6/14, 7:12 AM, Ehsan Akhgari wrote: With Birunthan's restless efforts in bug 784739, we have finally removed the usage of NULL in our C++ code. Please stop using NULL in new C++ code, and use nullptr instead! Can we update the Clang plugin to emit an error (and turn the tree red) if a NU

Re: A proposal to reduce the number of styles in Mozilla code

Hi, two points of caution: In the little version control archaeology I do, I hit "breaks blame for no good reason" pretty often already. I'd not underestimate the cost for the project of doing changes just for the sake of changes. Tools don't get code right. I've seen various changes where t

Re: A proposal to reduce the number of styles in Mozilla code

On 1/6/2014, 1:22 PM, Axel Hecht wrote: Hi, two points of caution: In the little version control archaeology I do, I hit "breaks blame for no good reason" pretty often already. I'd not underestimate the cost for the project of doing changes just for the sake of changes. I have very rarely run

Re: A proposal to reduce the number of styles in Mozilla code

On 01/06/2014 08:06 PM, Bobby Holley wrote: I think trying to change SpiderMonkey style to Gecko isn't a great use of effort: * There are a lot of SpiderMonkey hackers who don't hack on anything else, and don't consider themselves Gecko hackers. Many of them don't read dev-platform, and would pro

Re: PSA: Please stop using NULL in C++ code

On 1/6/2014, 1:20 PM, Gregory Szorc wrote: On 1/6/14, 7:12 AM, Ehsan Akhgari wrote: With Birunthan's restless efforts in bug 784739, we have finally removed the usage of NULL in our C++ code. Please stop using NULL in new C++ code, and use nullptr instead! Can we update the Clang plugin to em

Re: PSA: Please stop using NULL in C++ code

On Mon, Jan 06, 2014 at 01:35:38PM -0500, Ehsan Akhgari wrote: > On 1/6/2014, 1:20 PM, Gregory Szorc wrote: > >On 1/6/14, 7:12 AM, Ehsan Akhgari wrote: > >>With Birunthan's restless efforts in bug 784739, we have finally removed > >>the usage of NULL in our C++ code. Please stop using NULL in new

Re: A proposal to reduce the number of styles in Mozilla code

On 1/6/14 12:22, Axel Hecht wrote: In the little version control archaeology I do, I hit "breaks blame for no good reason" pretty often already. I'd not underestimate the cost for the project of doing changes just for the sake of changes. Do you have a concrete reason to believe that Martin's

Re: PSA: Please stop using NULL in C++ code

On 1/6/14, 10:54 AM, Trevor Saunders wrote: On Mon, Jan 06, 2014 at 01:35:38PM -0500, Ehsan Akhgari wrote: On 1/6/2014, 1:20 PM, Gregory Szorc wrote: On 1/6/14, 7:12 AM, Ehsan Akhgari wrote: With Birunthan's restless efforts in bug 784739, we have finally removed the usage of NULL in our C++ c

Re: PSA: Please stop using NULL in C++ code

On 2014-01-06, at 11:25, Gregory Szorc wrote: > Clang provides access to the low-level token stream [1]. This is available > via libclang (the C API) and the Python bindings and I'm pretty sure you can > access this info from plugins. > > Each token is associated with a source location (file,

Re: A proposal to reduce the number of styles in Mozilla code

I was asked, in the previous discussion about code formatting, to provide some research on the subject. I'm sorry I didn't get back to that fast enough, but here we are. We have a lot of data on this: Michael Hansen at Indiana University has done a series of eye-tracking experiments regarding

Re: PSA: Please stop using NULL in C++ code

On 1/6/14, 11:32 AM, Martin Thomson wrote: On 2014-01-06, at 11:25, Gregory Szorc wrote: Clang provides access to the low-level token stream [1]. This is available via libclang (the C API) and the Python bindings and I'm pretty sure you can access this info from plugins. Each token is asso

Re: PSA: Please stop using NULL in C++ code

On 1/6/14, 11:37 AM, Gregory Szorc wrote: Both are probably good clang-analyser plugins to write. http://clang-analyzer.llvm.org/ True. Although I don't believe we run clang-analyzer in automation... yet. I don't think the benefit of replacing NULL with nullptr is worth the developer and inf

Re: PSA: Please stop using NULL in C++ code

On 1/6/2014, 2:37 PM, Gregory Szorc wrote: On 1/6/14, 11:32 AM, Martin Thomson wrote: On 2014-01-06, at 11:25, Gregory Szorc wrote: Clang provides access to the low-level token stream [1]. This is available via libclang (the C API) and the Python bindings and I'm pretty sure you can access t

Re: A proposal to reduce the number of styles in Mozilla code

On 1/6/14, 9:55 AM, Martin Thomson wrote: 2. create some tools These tools should help people conform to the style. Primarily, what is needed is a tool with appropriate configuration that runs on the command line — e.g., mach reformat … clang-format is looking like a good candidate for C/C

Re: PSA: Please stop using NULL in C++ code

On 1/6/2014, 2:56 PM, Chris Peterson wrote: On 1/6/14, 11:37 AM, Gregory Szorc wrote: Both are probably good clang-analyser plugins to write. http://clang-analyzer.llvm.org/ True. Although I don't believe we run clang-analyzer in automation... yet. I don't think the benefit of replacing NULL

Re: A proposal to reduce the number of styles in Mozilla code

Nicholas Nethercote writes: > We've had some recent discussions about code style. I have a propasal I'm sceptical about the value of such changes out-weighing the cost here. I know there will be a cost now and in the future, but I don't recall seeing any entire files so badly formatted that it t

Re: A proposal to reduce the number of styles in Mozilla code

On Sun, Jan 5, 2014 at 6:34 PM, Nicholas Nethercote wrote: > - There is an semi-official policy that the owner of a module can dictate its > style. Examples: SpiderMonkey, Storage, MFBT. AFAICT, there are not many rules that module owners are bound by. The reason module owners can dictate style

Re: PSA: Please stop using NULL in C++ code

On 1/6/2014 12:20 PM, Gregory Szorc wrote: On 1/6/14, 7:12 AM, Ehsan Akhgari wrote: With Birunthan's restless efforts in bug 784739, we have finally removed the usage of NULL in our C++ code. Please stop using NULL in new C++ code, and use nullptr instead! Can we update the Clang plugin to

Re: A proposal to reduce the number of styles in Mozilla code

On 2014-01-06, at 12:36, Karl Tomlinson wrote: > I'm sceptical about the value of such changes out-weighing the > cost here. I know there will be a cost now and in the future, but > I don't recall seeing any entire files so badly formatted that it > took me more time to understand the code. If

Re: A proposal to reduce the number of styles in Mozilla code

On 2014-01-06, at 13:27, Brian Smith wrote: > As far as PSM is concerned, my main ask is that such reformatting of > security/manager/ssl/src/* be done in February or later, so that the > current urgently-needed big refactoring in that code is not disrupted. For one, I don’t see this being done

Re: A proposal to reduce the number of styles in Mozilla code

On Tue, Jan 7, 2014 at 7:06 AM, Bobby Holley wrote: > * There are a lot of SpiderMonkey hackers who don't hack on anything else, > and don't consider themselves Gecko hackers. > This has always bothered me a lot. It needs to change. Rob -- Jtehsauts tshaei dS,o n" Wohfy Mdaon yhoaus eanutt

Re: A proposal to reduce the number of styles in Mozilla code

On Tue, Jan 7, 2014 at 10:27 AM, Brian Smith wrote: > AFAICT, there are not many rules that module owners are bound by. There are lots of tree-wide or Gecko-wide rules that module owners are bound by. > The > reason module owners can dictate style is because module owners can > dictate everyt

Re: A proposal to reduce the number of styles in Mozilla code

FWIW I should mention explicitly that I support this proposal. The only big thing that I wish to see changed here is to remove the exception for js/* but I can live with that exception being lifted in the future. Cheers, Ehsan On 1/5/2014, 9:34 PM, Nicholas Nethercote wrote: We've had some r

Re: A proposal to reduce the number of styles in Mozilla code

On 1/6/14 3:27 PM, Brian Smith wrote: > On Sun, Jan 5, 2014 at 6:34 PM, Nicholas Nethercote > wrote: >> - There is an semi-official policy that the owner of a module can dictate its >> style. Examples: SpiderMonkey, Storage, MFBT. > AFAICT, there are not many rules that module owners are bound b

Re: A proposal to reduce the number of styles in Mozilla code

Martin Thomson writes: > I would like to think that adding (or removing) braces from block statements > should be acceptable. I would argue that braces should not be added with automation. When debugging code, it is important to understand the intent of the author. Adding braces at parser-deter

Re: A proposal to reduce the number of styles in Mozilla code

On 01/06/2014 03:10 PM, Karl Tomlinson wrote: > Martin Thomson writes: > >> I would like to think that adding (or removing) braces from block statements >> should be acceptable. > > I would argue that braces should not be added with automation. > > When debugging code, it is important to underst

List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]

On 1/6/2014 4:27 PM, Robert O'Callahan wrote: That's just not true, sorry. If some module owner decides to keep using NULL or PRUnichar, or invent their own string class, they will be corrected. Maybe. But we also have a very large number of deprecated or effectively-deprecated constructs wh

Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]

On 1/6/2014, 6:38 PM, Joshua Cranmer 🐧 wrote: On 1/6/2014 4:27 PM, Robert O'Callahan wrote: That's just not true, sorry. If some module owner decides to keep using NULL or PRUnichar, or invent their own string class, they will be corrected. Maybe. But we also have a very large number of deprec

Re: A proposal to reduce the number of styles in Mozilla code

On 1/6/14, 3:35 PM, Daniel Holbert wrote: On 01/06/2014 03:10 PM, Karl Tomlinson wrote: Martin Thomson writes: I would like to think that adding (or removing) braces from block statements should be acceptable. I would argue that braces should not be added with automation. When debugging cod

Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]

On 01/07/2014 01:38 AM, Joshua Cranmer 🐧 wrote: On 1/6/2014 4:27 PM, Robert O'Callahan wrote: That's just not true, sorry. If some module owner decides to keep using NULL or PRUnichar, or invent their own string class, they will be corrected. Maybe. But we also have a very large number of dep

Re: A proposal to reduce the number of styles in Mozilla code

On 01/06/2014 04:27 PM, Robert O'Callahan wrote: > That's just not true, sorry. If some module owner decides to keep using > NULL or PRUnichar, or invent their own string class, they will be corrected. At least the former isn't true, as far as I remember. It took convincing to get JS to move ove

Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]

On 1/6/2014 6:06 PM, smaug wrote: On 01/07/2014 01:38 AM, Joshua Cranmer 🐧 wrote: On 1/6/2014 4:27 PM, Robert O'Callahan wrote: That's just not true, sorry. If some module owner decides to keep using NULL or PRUnichar, or invent their own string class, they will be corrected. Maybe. But we a

Re: A proposal to reduce the number of styles in Mozilla code

On 01/06/2014 04:46 PM, Jason Orendorff wrote: > I'm fine with changing SpiderMonkey in whatever way is necessary to stop > these threads forever. Pretty sure I've said the same thing in other places, that we should have something everyone hates and use it everywhere. Definitely I've said it to

Re: A proposal to reduce the number of styles in Mozilla code

I'm fine with enforcing a gecko wide coding style as long as it comes with cross platform tools to act as arbiter.. it is something that needs to be automated and isn't worth the effort of trying to get everybody on the same page by best effort. On Mon, Jan 6, 2014 at 5:41 PM, Ehsan Akhgari wrot

Re: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

On 11/22/2013 10:18 PM, Benjamin Smedberg wrote: With the landing of bug 672843, the NS_ENSURE_* macros are now considered deprecated. If you are writing code that wants to issue warnings when methods fail, please either use NS_WARNING directly or use the new NS_WARN_IF macro. if (NS_WARN_IF(s

Mozilla style guide issues, from a JS point of view

I'm writing this list, so obviously I'm choosing what I think is on it. But I think there's rough consensus on most of these among JS hackers. JS widely uses 99ch line lengths (allows a line-wrap character in 100ch terminals). Given C++ symbol names, especially with templates, get pretty long

Re: Mozilla style guide issues, from a JS point of view

On 1/6/14, 4:46 PM, Jeff Walden wrote: JS widely uses 99ch line lengths (allows a line-wrap character in 100ch terminals). Given C++ symbol names, especially with templates, get pretty long, it's a huge loss to revert to 80ch because of how much has to wrap. Is there a reason Mozilla couldn'

Re: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

smaug writes: > Why this deprecation? NS_ENSURE_ macros hid return paths. Also many people didn't understand that they issued warnings, and so used the macros for expected return paths. Was there some useful functionality that is not provided by the replacements? ___

Re: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

On 01/07/2014 02:58 AM, Karl Tomlinson wrote: smaug writes: Why this deprecation? NS_ENSURE_ macros hid return paths. Also many people didn't understand that they issued warnings, and so used the macros for expected return paths. Was there some useful functionality that is not provided by t

Re: A proposal to reduce the number of styles in Mozilla code

Gregory Szorc writes: > I think you just gave an example of why our tooling needs to > identify poorly formatted code better. An "if" without a brace > followed by multiple indented lines containing compiler > expressions is a bug at worst or unreadable/unclear code at > best. We could, of course,

Re: Mozilla style guide issues, from a JS point of view

On Tue, Jan 7, 2014 at 1:46 PM, Jeff Walden wrote: > JS widely uses 99ch line lengths (allows a line-wrap character in 100ch > terminals). Given C++ symbol names, especially with templates, get pretty > long, it's a huge loss to revert to 80ch because of how much has to wrap. > Is there a reaso

Re: Mozilla style guide issues, from a JS point of view

On 01/07/2014 02:46 AM, Jeff Walden wrote: I'm writing this list, so obviously I'm choosing what I think is on it. But I think there's rough consensus on most of these among JS hackers. JS widely uses 99ch line lengths (allows a line-wrap character in 100ch terminals). Given C++ symbol names

Re: Mozilla style guide issues, from a JS point of view

On 1/6/14 7:46 PM, Jeff Walden wrote: Any style checker that checks both indentation and bracing (of course we'll have one, right?) If we have such a checker, and if it turns the tree orange on violations, then I can see relaxing the requirement for braces, probably. That requirement is the

Re: Mozilla style guide issues, from a JS point of view

On Monday 2014-01-06 18:46 -0600, Jeff Walden wrote: > I'm writing this list, so obviously I'm choosing what I think is on it. But > I think there's rough consensus on most of these among JS hackers. > > JS widely uses 99ch line lengths (allows a line-wrap character in 100ch > terminals). Give

Re: A proposal to reduce the number of styles in Mozilla code

On Jan 6, 2014, at 17:10, Karl Tomlinson wrote: > Gregory Szorc writes: > >> I think you just gave an example of why our tooling needs to >> identify poorly formatted code better. An "if" without a brace >> followed by multiple indented lines containing compiler >> expressions is a bug at worst

Re: A proposal to reduce the number of styles in Mozilla code

On 1/6/14, 5:05 PM, Karl Tomlinson wrote: If we have a tool to skip the style change on any such unclear situations, then perhaps we can proceed more safely. I would replace "skip" with "abort loudly", so a human can review the unclear code. :) chris

Re: Mozilla style guide issues, from a JS point of view

On Mon, Jan 6, 2014 at 5:38 PM, L. David Baron wrote: >> > So if we're switching to 99 or 100, I'd like to understand how you > picked that number and have confidence that it's not just going to > keep going up. SpiderMonkey's used 99 for years and years without anyone (AFAIK) arguing that it be

Re: Mozilla style guide issues, from a JS point of view

L. David Baron writes: > I tend to think that we should either: > * stick to 80 > * require no wrapping, meaning that comments must be one paragraph >per line, boolean conditions must all be single line, and assume >that people will deal, using an editor that handles such code >usefu

Re: Mozilla style guide issues, from a JS point of view

On 1/6/2014 6:46 PM, Jeff Walden wrote: I'm writing this list, so obviously I'm choosing what I think is on it. But I think there's rough consensus on most of these among JS hackers. JS widely uses 99ch line lengths (allows a line-wrap character in 100ch terminals). Given C++ symbol names, e

Re: Mozilla style guide issues, from a JS point of view

I strongly prefer at least a 100 character per line limit. Technology marches on. On Mon, Jan 6, 2014 at 9:23 PM, Karl Tomlinson wrote: > L. David Baron writes: > > > I tend to think that we should either: > > * stick to 80 > > * require no wrapping, meaning that comments must be one paragrap

Re: Mozilla style guide issues, from a JS point of view

On 01/06/2014 08:35 PM, Joshua Cranmer 🐧 wrote: > And a '_' at the end of member names requires less typing than 'm' + capital > letter? This started, and still largely is, an Ion convention. Lots of existing code doesn't use it, and I haven't much worked on code that does. But as I said, I'm

Re: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

On Mon, Jan 6, 2014 at 5:04 PM, smaug wrote: > no, since it is always possible to expand those macros. > However > if (NS_WARN_IF(NS_FAILED(rv)) { > return rv; > } > is super ugly. > Note that there in a explicit stylistic exception that NS_WARN_IF statements do not require braces. So it's: i