On Tue, Aug 23, 2016 at 3:22 PM, R Kent James <k...@caspia.com> wrote:
>
> A common programming pattern that I would use, when I don't really care
> about why something failed, is:
>
> nsCOMPtr<nsIAsyncInputStream> inputStream;
>
> pipe.GetInputStream(getter_AddRefs(inputStream));
>
> if (!inputStream) {
>   ... take action
> }
>
> If I understand the must_use correctly, you are specifically not allowing
> that pattern. Why?

It is a common idiom. With a function like that (a getter for a
pointer) you have two ways of detecting failure -- looking at the
return value, or checking if the pointer is null. I prefer looking at
the return value, because:

- Consistency: that's what you do with other nsresult-returning
functions that don't match this idiom.

- Checkability: you can make "check the return value" unforgettable
with MOZ_MUST_USE, as I have done. It's harder to make "check if the
pointer is null" unforgettable. Maybe with a separate static analysis
tool, but that's much less convenient than the compiler telling you
immediately.

Unfortunately, it is sometimes unclear whether "return NS_OK and set
the pointer to null" is a possibility, and also whether "return a
failure code and set the pointer to non-null", which does complicate
things.

In general, some functions should obviously have [must_use], some
obviously should not, and then some are in a grey area and people
could reasonably disagree. I tend to err on the side of safety -- my
rule of thumb of late has been "what would Rust do?" -- because an
unnecessary check is less harmful than a missing check. But if you
feel strongly about that case, please file a bug and a patch, get r+,
and change it.

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

Reply via email to