Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]
On 01/07/2014 01:11 AM, Joshua Cranmer wrote: 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 also have a very large number of deprecated or effectively-deprecated constructs whose deprecation module owners may not be aware of because their use is somewhat prevalent in code. For example, the NS_ENSURE_* macros are apparently now considered officially deprecated. Since when? NS_ENSURE_ macros are very useful for debugging. (When something is going wrong, the warnings in the terminal tend to give strong hints what/where that something is. Reduces debugging time significantly.) Since Benjamin's message of November 22: news://news.mozilla.org/mailman.11861.1385151580.23840.dev-platf...@lists.mozilla.org (search for Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS if you don't have an NNTP client). Although there wasn't any prior discussion of the wisdom of this change, whether or not to use NS_ENSURE_SUCCESS-like patterns has been the subject of very acrimonious debates in the past and given the voluminous discussion on style guides in recent times, I'm not particularly inclined to start yet another one. FWIW, I've never seen much support for this change from anyone else than Benjamin, and only in his modules the NS_ENSURE_* macros have been effectively deprecated. HTH Ms2ger ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]
On Tuesday 2014-01-07 09:13 +0100, Ms2ger wrote: On 01/07/2014 01:11 AM, Joshua Cranmer wrote: Since Benjamin's message of November 22: news://news.mozilla.org/mailman.11861.1385151580.23840.dev-platf...@lists.mozilla.org (search for Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS if you don't have an NNTP client). Although there wasn't any prior discussion of the wisdom of this change, whether or not to use NS_ENSURE_SUCCESS-like patterns has been the subject of very acrimonious debates in the past and given the voluminous discussion on style guides in recent times, I'm not particularly inclined to start yet another one. FWIW, I've never seen much support for this change from anyone else than Benjamin, and only in his modules the NS_ENSURE_* macros have been effectively deprecated. I'm happy about getting rid of NS_ENSURE_*. -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: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]
2014/1/7 L. David Baron dba...@dbaron.org On Tuesday 2014-01-07 09:13 +0100, Ms2ger wrote: On 01/07/2014 01:11 AM, Joshua Cranmer wrote: Since Benjamin's message of November 22: news:// news.mozilla.org/mailman.11861.1385151580.23840.dev-platf...@lists.mozilla.org (search for Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS if you don't have an NNTP client). Although there wasn't any prior discussion of the wisdom of this change, whether or not to use NS_ENSURE_SUCCESS-like patterns has been the subject of very acrimonious debates in the past and given the voluminous discussion on style guides in recent times, I'm not particularly inclined to start yet another one. FWIW, I've never seen much support for this change from anyone else than Benjamin, and only in his modules the NS_ENSURE_* macros have been effectively deprecated. I'm happy about getting rid of NS_ENSURE_*. I would like a random voice in favor of deprecating NS_ENSURE_* for the reason that hiding control flow behind macros is IMO one of the most evil usage patterns of macros. Benoit -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 ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]
I support getting rid of NS_ENSURE_*. Gavin On Tue, Jan 7, 2014 at 3:13 AM, Ms2ger ms2...@gmail.com wrote: On 01/07/2014 01:11 AM, Joshua Cranmer wrote: 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 also have a very large number of deprecated or effectively-deprecated constructs whose deprecation module owners may not be aware of because their use is somewhat prevalent in code. For example, the NS_ENSURE_* macros are apparently now considered officially deprecated. Since when? NS_ENSURE_ macros are very useful for debugging. (When something is going wrong, the warnings in the terminal tend to give strong hints what/where that something is. Reduces debugging time significantly.) Since Benjamin's message of November 22: news://news.mozilla.org/mailman.11861.1385151580.23840.dev-platf...@lists.mozilla.org (search for Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS if you don't have an NNTP client). Although there wasn't any prior discussion of the wisdom of this change, whether or not to use NS_ENSURE_SUCCESS-like patterns has been the subject of very acrimonious debates in the past and given the voluminous discussion on style guides in recent times, I'm not particularly inclined to start yet another one. FWIW, I've never seen much support for this change from anyone else than Benjamin, and only in his modules the NS_ENSURE_* macros have been effectively deprecated. HTH Ms2ger ___ 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: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]
Benoit Jacob wrote:
I would like a random voice in favor of deprecating NS_ENSURE_* for the reason
that hiding control flow behind macros is IMO one of the most evil usage
patterns of macros.
So you're saying that
nsresult rv = Foo();
NS_ENSURE_SUCCESS(rv, rv);
is hiding the control flow of the equivalent JavaScript
try {
Foo();
} catch (e) {
throw e;
}
except of course that nobody writes JavaScript like that...
--
Warning: May contain traces of nuts.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]
2014/1/7 Neil n...@parkwaycc.co.uk
Benoit Jacob wrote:
I would like a random voice in favor of deprecating NS_ENSURE_* for the
reason that hiding control flow behind macros is IMO one of the most evil
usage patterns of macros.
So you're saying that
nsresult rv = Foo();
NS_ENSURE_SUCCESS(rv, rv);
is hiding the control flow of the equivalent JavaScript
try {
Foo();
} catch (e) {
throw e;
}
except of course that nobody writes JavaScript like that...
All I mean is that NS_ENSURE_SUCCESS hides a 'return' statement.
#define NS_ENSURE_SUCCESS(res, ret)
\ do {
\nsresult __rv = res; /* Don't evaluate |res| more than
once */\if (NS_FAILED(__rv)) {
\ NS_ENSURE_SUCCESS_BODY(res, ret)
\ return ret;
\}
\ } while(0)
For example, if I'm scanning a function for possible early returns (say I'm
debugging a bug where we're forgetting to close or delete a thing before
returning), I now need to scan for NS_ENSURE_SUCCESS in addition to
scanning for return. That's why hiding control flow in macros is, in my
opinion, never acceptable.
Benoit
--
Warning: May contain traces of nuts.
___
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: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]
On 1/7/2014 12:58 PM, Benoit Jacob wrote: I would like a random voice in favor of deprecating NS_ENSURE_* for the reason that hiding control flow behind macros is IMO one of the most evil usage patterns of macros. In general, I would agree. I have no problems with killing, say, NS_ENSURE_TRUE or similar macros. However, NS_ENSURE_SUCCESS is a special case: it is the equivalent in JS of not providing a try/catch statement around a JS expression. In code that makes extremely heavy use of XPCOM requirements, NS_ENSURE_SUCCESS brings the amount of boilerplate needed to one line per function; to not use it requires significantly more boilerplate (three lines, if you insist on following the always-brace-ifs convention). -- 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: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]
Benoit Jacob wrote: I'm scanning a function for possible early returns Good thing you don't have to deal with C++ exceptions then. -- Warning: May contain traces of nuts. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]
On Tue, Jan 7, 2014 at 11:29 AM, Benoit Jacob jacob.benoi...@gmail.comwrote: For example, if I'm scanning a function for possible early returns (say I'm debugging a bug where we're forgetting to close or delete a thing before returning), I now need to scan for NS_ENSURE_SUCCESS in addition to scanning for return. That's why hiding control flow in macros is, in my opinion, never acceptable. If you care about that 9 times out of 10 you are failing to use an RAII class when you should be. Since we seem to be voting now, I am moderately opposed to making XPCOM method calls more boilerplate-y, and very opposed to removing NS_ENSURE_SUCCESS without some sort of easy shorthand to test an nsresult and print to the console if it is a failure. I know for sure that some of the other DOM peers (smaug and bz come to mind) feel similarly about the latter. - Kyle ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]
On 14-01-07 08:04 PM, Kyle Huey wrote: On Tue, Jan 7, 2014 at 11:29 AM, Benoit Jacob jacob.benoi...@gmail.comwrote: For example, if I'm scanning a function for possible early returns (say I'm debugging a bug where we're forgetting to close or delete a thing before returning), I now need to scan for NS_ENSURE_SUCCESS in addition to scanning for return. That's why hiding control flow in macros is, in my opinion, never acceptable. If you care about that 9 times out of 10 you are failing to use an RAII class when you should be. Since we seem to be voting now, I am moderately opposed to making XPCOM method calls more boilerplate-y, and very opposed to removing NS_ENSURE_SUCCESS without some sort of easy shorthand to test an nsresult and print to the console if it is a failure. I know for sure that some of the other DOM peers (smaug and bz come to mind) feel similarly about the latter. Would a macro starting with RETURN_ be an improvement over NS_ENSURE_? e.g. nsresult rv = Foo(); RETURN_AND_WARN_IF_FAIL(rv); It's a mouthful (handful?) to type, but it's a single line and makes explicit what's going on. --m. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: List of deprecated constructs [was Re: A proposal to reduce the number of styles in Mozilla code]
2014/1/7 Kyle Huey m...@kylehuey.com On Tue, Jan 7, 2014 at 11:29 AM, Benoit Jacob jacob.benoi...@gmail.comwrote: For example, if I'm scanning a function for possible early returns (say I'm debugging a bug where we're forgetting to close or delete a thing before returning), I now need to scan for NS_ENSURE_SUCCESS in addition to scanning for return. That's why hiding control flow in macros is, in my opinion, never acceptable. If you care about that 9 times out of 10 you are failing to use an RAII class when you should be. I was talking about reading code, not writing code. I spend more time reading code that I didn't write, than writing code. Of course I do use RAII helpers when I write this kind of code myself, in fact just today I landed two more such helpers in gfx/gl/ScopedGLHelpers.* ... Benoit Since we seem to be voting now, I am moderately opposed to making XPCOM method calls more boilerplate-y, and very opposed to removing NS_ENSURE_SUCCESS without some sort of easy shorthand to test an nsresult and print to the console if it is a failure. I know for sure that some of the other DOM peers (smaug and bz come to mind) feel similarly about the latter. - Kyle ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
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 whose deprecation module owners may not be aware of because their use is somewhat prevalent in code. For example, the NS_ENSURE_* macros are apparently now considered officially deprecated. Our track record of removing these quickly is poor (nsISupportsArray and nsVoidArray, anyone?), and many of the deprecated constructs are macros or things defined in external project headers (like, say, prtypes.h), which makes using __declspec(deprecated) or __attribute__((deprecated)) unfeasible. Is there any support for setting up a wiki page that lists these deprecated, obsolete constructs and provides tracking bugs for actually eliminating them? -- 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: 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 deprecated or effectively-deprecated constructs whose deprecation module owners may not be aware of because their use is somewhat prevalent in code. For example, the NS_ENSURE_* macros are apparently now considered officially deprecated. Our track record of removing these quickly is poor (nsISupportsArray and nsVoidArray, anyone?), and many of the deprecated constructs are macros or things defined in external project headers (like, say, prtypes.h), which makes using __declspec(deprecated) or __attribute__((deprecated)) unfeasible. FWIW a while ago I got Wan-Teh's approval for us to take local Mozilla specific patches to NSPR. Is there any support for setting up a wiki page that lists these deprecated, obsolete constructs and provides tracking bugs for actually eliminating them? Yes please! Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
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 deprecated or effectively-deprecated constructs whose deprecation module owners may not be aware of because their use is somewhat prevalent in code. For example, the NS_ENSURE_* macros are apparently now considered officially deprecated. Since when? NS_ENSURE_ macros are very useful for debugging. (When something is going wrong, the warnings in the terminal tend to give strong hints what/where that something is. Reduces debugging time significantly.) Our track record of removing these quickly is poor (nsISupportsArray and nsVoidArray, anyone?), and many of the deprecated constructs are macros or things defined in external project headers (like, say, prtypes.h), which makes using __declspec(deprecated) or __attribute__((deprecated)) unfeasible. Is there any support for setting up a wiki page that lists these deprecated, obsolete constructs and provides tracking bugs for actually eliminating them? ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
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 also have a very large number of deprecated or effectively-deprecated constructs whose deprecation module owners may not be aware of because their use is somewhat prevalent in code. For example, the NS_ENSURE_* macros are apparently now considered officially deprecated. Since when? NS_ENSURE_ macros are very useful for debugging. (When something is going wrong, the warnings in the terminal tend to give strong hints what/where that something is. Reduces debugging time significantly.) Since Benjamin's message of November 22: news://news.mozilla.org/mailman.11861.1385151580.23840.dev-platf...@lists.mozilla.org (search for Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS if you don't have an NNTP client). Although there wasn't any prior discussion of the wisdom of this change, whether or not to use NS_ENSURE_SUCCESS-like patterns has been the subject of very acrimonious debates in the past and given the voluminous discussion on style guides in recent times, I'm not particularly inclined to start yet another one. -- 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
