Re: MOZ_WARN_UNUSED_RESULT has been renamed as MOZ_MUST_USE

[Kyle Huey]

On Fri, Apr 29, 2016 at 1:46 PM, Chris Peterson 
wrote:

> On 4/29/16 5:53 AM, Nathan Froyd wrote:
>
>> This is a noble goal, but there is an enormous amount of code that
>> would need to be modified to make this feasible.  Perhaps if you
>> exempted nsresult from MOZ_MUST_USE types.
>>
>
> In theory, nsresult seems like an important type to check.
>
> That said, I once tried building Gecko with `#define NS_IMETHOD
> MOZ_WARN_UNUSED_RESULT NS_IMETHOD_(nsresult)`. There were over 10,000
> warnings for XPCOM method callers not checking nsresult return values, so
> this approach does not seem practical. :(


nsresult is required in a lot of method signatures (thanks XPIDL!) which
makes it often not an important type to check.

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

Re: MOZ_WARN_UNUSED_RESULT has been renamed as MOZ_MUST_USE

[Chris Peterson]

On 4/29/16 5:53 AM, Nathan Froyd wrote:

This is a noble goal, but there is an enormous amount of code that
would need to be modified to make this feasible.  Perhaps if you
exempted nsresult from MOZ_MUST_USE types.


In theory, nsresult seems like an important type to check.

That said, I once tried building Gecko with `#define NS_IMETHOD 
MOZ_WARN_UNUSED_RESULT NS_IMETHOD_(nsresult)`. There were over 10,000 
warnings for XPCOM method callers not checking nsresult return values, 
so this approach does not seem practical. :(

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

Re: MOZ_WARN_UNUSED_RESULT has been renamed as MOZ_MUST_USE

[Nathan Froyd]

On Fri, Apr 29, 2016 at 7:54 AM, Gerald Squelart  wrote:
> Now, for maximum defensiveness, shouldn't we go even further?
>
> How about: Make 'MOZ_MUST_USE' implicit for all functions/methods (except 
> void of course, probably methods returning T&, and maybe more as they come 
> up).
> When a result is not needed somewhere, use the 'Unused << foo()' idiom.
> And if a function's return is really not important, then mark it with 
> MOZ_MAY_IGNORE_RESULT (or similar).

This is a noble goal, but there is an enormous amount of code that
would need to be modified to make this feasible.  Perhaps if you
exempted nsresult from MOZ_MUST_USE types.

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

Re: MOZ_WARN_UNUSED_RESULT has been renamed as MOZ_MUST_USE

[Gerald Squelart]

On Friday, April 29, 2016 at 8:04:46 PM UTC+10, Nicholas Nethercote wrote:
> On Fri, Apr 29, 2016 at 10:25 AM, Nicholas Nethercote
>  wrote:
> >
> > I just landed on inbound the patches in bug 1267550, which renamed
> > MOZ_WARN_UNUSED_RESULT as MOZ_MUST_USE.
> >
> > A shorter name was in order because it's an attribute that is already
> > used widely, and should be used even more, because it catches real
> > problems. In fact, any function that is fallible and returns a
> > non-pointer value (usually a bool or nsresult) is a candidate for a
> > MOZ_MUST_USE annotation.
> 
> I've created bug 1268766 as a meta-bug for tracking the addition of
> more uses of MOZ_MUST_USE. I've started working on this in a few
> places -- and already found a bunch of missing failure checks -- but
> we have more than enough code for the task to be shared around, if
> anybody is interested.
> 
> Also note that we have another attribute called MOZ_MUST_USE_TYPE,
> which can be applied to a type, and then any function that returns
> that type will need to be checked. (In other words, it implicitly adds
> MOZ_MUST_USE to any function that returns that type.) Bug 1209780 is a
> nice example of its use. (Note that it used to be called MOZ_MUST_USE,
> and was renamed MOZ_MUST_USE_TYPE to allow MOZ_WARN_UNUSED_RESULT to
> be renamed as MOZ_MUST_USE.) However, it only works in static analysis
> builds, whereas MOZ_MUST_USE also works in GCC and clang, so although
> it clutters up code less, it also results in less immediate error
> messages.
> 
> Nick

Thank you for that.


Now, for maximum defensiveness, shouldn't we go even further?

How about: Make 'MOZ_MUST_USE' implicit for all functions/methods (except void 
of course, probably methods returning T&, and maybe more as they come up).
When a result is not needed somewhere, use the 'Unused << foo()' idiom.
And if a function's return is really not important, then mark it with 
MOZ_MAY_IGNORE_RESULT (or similar).

I admit I haven't thoroughly thought through all the implications, maybe it's 
just too hard to put in place and then fix all the errors that would 
immediately pop up, but it'd be interesting to try I think. Or dispiriting.


In the meantime, would you have some suggestions/guidelines as to where we 
should add MOZ_MUST_USE?
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

MOZ_WARN_UNUSED_RESULT has been renamed as MOZ_MUST_USE

[Nicholas Nethercote]

Hi,

I just landed on inbound the patches in bug 1267550, which renamed
MOZ_WARN_UNUSED_RESULT as MOZ_MUST_USE.

A shorter name was in order because it's an attribute that is already
used widely, and should be used even more, because it catches real
problems. In fact, any function that is fallible and returns a
non-pointer value (usually a bool or nsresult) is a candidate for a
MOZ_MUST_USE annotation.

(Unfortunately MOZ_MUST_USE is no help with functions that return
pointers and use nullptr to indicate failure, because even if you
forget to null-check such a pointer you will still use it and the
compiler won't complain.)

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

Re: MOZ_WARN_UNUSED_RESULT has been renamed as MOZ_MUST_USE

[Nicholas Nethercote]

On Fri, Apr 29, 2016 at 10:25 AM, Nicholas Nethercote
 wrote:
>
> I just landed on inbound the patches in bug 1267550, which renamed
> MOZ_WARN_UNUSED_RESULT as MOZ_MUST_USE.
>
> A shorter name was in order because it's an attribute that is already
> used widely, and should be used even more, because it catches real
> problems. In fact, any function that is fallible and returns a
> non-pointer value (usually a bool or nsresult) is a candidate for a
> MOZ_MUST_USE annotation.

I've created bug 1268766 as a meta-bug for tracking the addition of
more uses of MOZ_MUST_USE. I've started working on this in a few
places -- and already found a bunch of missing failure checks -- but
we have more than enough code for the task to be shared around, if
anybody is interested.

Also note that we have another attribute called MOZ_MUST_USE_TYPE,
which can be applied to a type, and then any function that returns
that type will need to be checked. (In other words, it implicitly adds
MOZ_MUST_USE to any function that returns that type.) Bug 1209780 is a
nice example of its use. (Note that it used to be called MOZ_MUST_USE,
and was renamed MOZ_MUST_USE_TYPE to allow MOZ_WARN_UNUSED_RESULT to
be renamed as MOZ_MUST_USE.) However, it only works in static analysis
builds, whereas MOZ_MUST_USE also works in GCC and clang, so although
it clutters up code less, it also results in less immediate error
messages.

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