1147706 - Warn if we don't use the result of const methods on BaseRect and its subclasses

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Seth Fowler [:seth] [:s2h]]

Many methods on BaseRect, nsRect, nsIntRect, and gfxRect have confusingly ambiguous names that make it unclear whether the methods change the existing rect or return a new one.

It'd be great if we could make the names less ambiguous, but that's a hard problem. What's less hard is ensuring that we warn if the result of const methods isn't used, so at least it becomes obvious that |rect.Union(anotherRect)| does nothing in isolation.

In this bug I'll add MOZ_WARN_UNUSED_RESULT to every const method that seems likely to cause confusion. I won't add it to operators (which have clearly defined semantics that we conform to) and I won't add it to methods that return some non-rect type, because generally those don't cause confusion.

Bug 1144951 is already handling the ConvertAppUnits* case, so I also won't touch those methods.

Comment 1

[Seth Fowler [:seth] [:s2h]]

Attachment: Warn if we don't use the result of const methods on BaseRect and its subclasses

Here's the patch. This already uncovered one bug, which I filed as bug 1147707.

Note that I checked gfxRect.h, but we don't define any new problematic methods
there, so this patch doesn't touch it.

Comment 2

[Seth Fowler [:seth] [:s2h]]

Thanks for the review! Pushed to inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/1cf1e2b49d90

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1cf1e2b49d90