Closed Bug 608759 Opened 9 years ago Closed 9 years ago

Stop calling ContentEnumFunc through a function pointer

Categories

(Core :: CSS Parsing and Computation, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla5

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Details

Attachments

(1 file)

EnumerateAllRules takes a function pointer, but it's always to the same function.

At this point I've got a patch to fix that in two different trees (independently written each time as part of other work).

Once we ship fx2 and I have time to remerge stuff to this change, I'll just do it.
Attached patch FixSplinter Review
Whiteboard: [need review]
I think if you actually want the compiler to inline ContentEnumFunc, you should put it before its first use.
Comment on attachment 510050 [details] [diff] [review]
Fix

So two things.  First, that's doable but a bit of a hassle.  In particular, ContentEnumFunc needs to come after the class decls of both TreeMatchContext and NodeMatchContext, and needs to know about the signatures of SelectorMatches and SelectorMatchesTree.  I could move the former and forward-declare the latter, I guess...  The other option is to move the ContentEnumFunc consumers down lower in the file.  They're class methods, so that's safe enough.  It's a bunch of code movement, though.  Let me know if you prefer one of those options?

Second, at some point I verified that at least gcc on Mac did inline it, iirc.  And I'm not quite sure how much I care about the inlining...
Attachment #510050 - Flags: review?(dbaron)
Comment on attachment 510050 [details] [diff] [review]
Fix

r=dbaron
Attachment #510050 - Flags: review?(dbaron) → review+
Pushed http://hg.mozilla.org/projects/birch/rev/feee7408eacd
Whiteboard: [need review] → fixed-in-birch
https://hg.mozilla.org/mozilla-central/rev/feee7408eacd
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: fixed-in-birch
Target Milestone: --- → mozilla2.2
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.