Open Bug 1804319 Opened 2 years ago Updated 2 years ago

Remove callsites of "do not use" `operator==` methods on rect-typed classes (replace with `IsEqualEdges` or `IsEqualInterior`)

Categories

(Core :: Layout, task)

task

Tracking

()

People

(Reporter: dholbert, Unassigned)

Details

We have three copies of this comment, for our geometric "rect" classes' operator== methods:

  // This is here only to keep IPDL-generated code happy. DO NOT USE.

https://searchfox.org/mozilla-central/search?q=to%20keep%20IPDL-generated%20code%20happy.%20DO%20NOT%20USE&path=

Despite this comment, we do in fact use these methods in a few places. Though fortunately fewer than 10 times per method.

The comment dates back to bug 641426 comment 45, which was for "Part 5: Avoid operator== where possible to distinguish between 'equal edges' and 'equal areas' for rectangles"

I'm not sure it was explicitly stated there, but it seems like basically the intent was that some code that checks for equality will want to treat empty-rects as a special case, considering them to be equal despite potentially having different x/y positions. Hence we have IsEqualEdges (which treats empty rects at different locations as not-equal) vs. IsEqualInterior (which treats all empty rects as equal).

In the spirit of this "DO NOT USE" comment, we should audit the existing usages and replace them with invocations of IsEqualEdges or IsEqualInterior, to keep the comment reflecting reality and to be sure we're handling the empty-rect edge-case in a context-appropriate way at each callsite.

(followup material / food-for-thought: after we've removed the callers from human-authored code, maybe (?) we can mark these methods as private, and add friend annotations to include their IPDL-generated callsites. Searchfox seems to think that each of these methods only has 1-2 callers in generated code, and each caller is in the context of a method on a class that could be declared to be a friend class, I think.)

You need to log in before you can comment on or make changes to this bug.