Re: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

2014-01-15 Thread Zack Weinberg 

On 2014-01-06 7:58 PM, Karl Tomlinson wrote:

smaug sm...@welho.com writes:


Why this deprecation?


NS_ENSURE_ macros hid return paths.


That was exactly why they were a Good Thing!  Normal control flow was 
emphasized.


zw

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

Re: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

2014-01-07 Thread Karl Tomlinson 
Bobby Holley writes:

 Note that there in a explicit stylistic exception that NS_WARN_IF
 statements do not require braces. So it's:

 if (NS_WARN_IF(NS_FAILED(rv)))
   return rv;

I don't see that on the current version of
https://developer.mozilla.org/en-US/docs/Developer_Guide/Coding_Style

I like having entirely blank lines (without braces) after jump
statements because I find it helps make them stand out, but I
don't see any reason why there should be an exception only for
particular macros.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

2014-01-07 Thread smaug 

On 01/07/2014 05:14 PM, smaug wrote:

On 01/07/2014 08:46 AM, Bobby Holley wrote:

On Mon, Jan 6, 2014 at 5:04 PM, smaug sm...@welho.com 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:

No exceptions. always {} with if.





if (NS_WARN_IF(NS_FAILED(rv)))
   return rv;


Also, I agree that we should get NS_WARN_IF_FAILED. Then it becomes:

if (NS_WARN_IF_FAILED(rv))
   return rv;

which is almost as palatable as NS_ENSURE_SUCCESS(rv, rv);

bholley






And looks like whoever updated the coding style made the examples inconsistent, 
since there has pretty much always
(I think since the time coding style was somewhere in www.mozilla.org/*) been the 
following Always brace controlled statements, and
that is still there. No exceptions. Consistency is more important than style.




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

Re: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

2014-01-07 Thread smaug 

On 01/07/2014 08:46 AM, Bobby Holley wrote:

On Mon, Jan 6, 2014 at 5:04 PM, smaug sm...@welho.com 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:

No exceptions. always {} with if.





if (NS_WARN_IF(NS_FAILED(rv)))
   return rv;


Also, I agree that we should get NS_WARN_IF_FAILED. Then it becomes:

if (NS_WARN_IF_FAILED(rv))
   return rv;

which is almost as palatable as NS_ENSURE_SUCCESS(rv, rv);

bholley




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

Re: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

2014-01-07 Thread Andrew McCreight 
I filed bug 957201 for NS_WARN_IF_FAILED.

Andrew

- Original Message -
 On 01/07/2014 02:58 AM, Karl Tomlinson wrote:
  smaug sm...@welho.com 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?
 
 
 
 no, since it is always possible to expand those macros.
 However
 if (NS_WARN_IF(NS_FAILED(rv)) {
return rv;
 }
 is super ugly.
 
 Hopefully something like NS_WARN_IF_FAILED(rv) could be added.
 ___
 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: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

2014-01-07 Thread Bobby Holley 
Hm. It's pretty unfortunate that we now need 4 lines per fallible call, as
opposed to 2 with NS_ENSURE_* macros. The stylistic exception made this 3,
which was slightly more palatable.

bholley


On Tue, Jan 7, 2014 at 7:26 AM, smaug sm...@welho.com wrote:

 On 01/07/2014 05:14 PM, smaug wrote:

 On 01/07/2014 08:46 AM, Bobby Holley wrote:

 On Mon, Jan 6, 2014 at 5:04 PM, smaug sm...@welho.com 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:

 No exceptions. always {} with if.




 if (NS_WARN_IF(NS_FAILED(rv)))
return rv;


 Also, I agree that we should get NS_WARN_IF_FAILED. Then it becomes:

 if (NS_WARN_IF_FAILED(rv))
return rv;

 which is almost as palatable as NS_ENSURE_SUCCESS(rv, rv);

 bholley




 And looks like whoever updated the coding style made the examples
 inconsistent, since there has pretty much always
 (I think since the time coding style was somewhere in www.mozilla.org/*)
 been the following Always brace controlled statements, and
 that is still there. No exceptions. Consistency is more important than
 style.





 ___
 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: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

2014-01-07 Thread Benjamin Smedberg 

On 1/6/2014 7:43 PM, smaug wrote:



Why this deprecation?


Karl is right, we are removing the macros that hide control flow (as 
well as warnings, in this case).


I'm considering the NS_WARN_IF_FAILED(rv) proposal. It's obviously a 
less typing then NS_WARN_IF(NS_FAILED(rv)), but I'm not convinced that 
it's clear just by reading it what the return type would be, especially 
since the signature of NS_WARN_IF returns the same value that is passed in:


bool NS_WARN_IF(bool);

I'm trying to figure out if people would expect the result of 
NS_WARN_IF_FAILED to be an nsresult or a bool.


--BDS

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

Re: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

2014-01-06 Thread smaug 

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(somethingthatshouldbetrue))
   return NS_ERROR_INVALID_ARG;

if (NS_WARN_IF(NS_FAILED(rv))
   return rv;

I am working on a script which can be used to automatically convert most of the 
existing NS_ENSURE_* macros, and I will also be updating the coding
style guide to point to the recommended form.

--BDS




Why this deprecation?
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

2014-01-06 Thread Karl Tomlinson 
smaug sm...@welho.com 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?
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

2014-01-06 Thread smaug 

On 01/07/2014 02:58 AM, Karl Tomlinson wrote:

smaug sm...@welho.com 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?




no, since it is always possible to expand those macros.
However
if (NS_WARN_IF(NS_FAILED(rv)) {
  return rv;
}
is super ugly.

Hopefully something like NS_WARN_IF_FAILED(rv) could be added.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

2013-11-22 Thread Benjamin Smedberg 
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(somethingthatshouldbetrue))
  return NS_ERROR_INVALID_ARG;

if (NS_WARN_IF(NS_FAILED(rv))
  return rv;

I am working on a script which can be used to automatically convert most 
of the existing NS_ENSURE_* macros, and I will also be updating the 
coding style guide to point to the recommended form.


--BDS

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

Re: Please use NS_WARN_IF instead of NS_ENSURE_SUCCESS

2013-11-22 Thread Daniel Holbert 
On 11/22/2013 12:18 PM, Benjamin Smedberg wrote:
 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(somethingthatshouldbetrue))
   return NS_ERROR_INVALID_ARG;
 
 if (NS_WARN_IF(NS_FAILED(rv))
   return rv;

I think you mean somethingthatshouldNOTbetrue in the first example
there, right?

(Since it looks like we're warning  returning an error code, when that
placeholder is truthy.)
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform