Closed Bug 1147706 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: seth, Assigned: seth)

References

Details

Attachments

(1 file)

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.
Depends on: 1147707
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.
Attachment #8583525 - Flags: review?(tnikkel)
Assignee: nobody → seth
Status: NEW → ASSIGNED
Depends on: 1137615
Attachment #8583525 - Flags: review?(tnikkel) → review+
Thanks for the review! Pushed to inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/1cf1e2b49d90
https://hg.mozilla.org/mozilla-central/rev/1cf1e2b49d90
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.