795721 - FunctionBox should inherit from ObjectBox, not forward to it

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Eddy Bruel [:ejpbruel]]

Right now we have this rather weird organisation where if FunctionBox contains an Objectbox, the ObjectBox contains the FunctionBox it is contained in.

For Harmony modules, we will likely also end up with ModuleBoxes. Conceptually, both ModuleBox and FunctionBox are just a specialisation of ObjectBox. It would be much nicer to inherit both from ObjectBox, and make ObjectBox a tagged union to avoid the need for virtual functions.

Note that ModuleBox and FunctionBox also inherit from SharedContext, but from the perspective of ObjectBox, we don't care about this. All we care about is that the traceable stuff in both is visible in ObjectBox's union.

Comment 1

[Eddy Bruel [:ejpbruel]]

Attachment: Patch to be reviewed

Here's a patch that cleans up ObjectBox. Better encapsulation, and the relationship between FunctionBox and ObjectBox is much clearer this way. Didn't need any unions or type tags; the type of the underlying Object is enough to determine the type of the box.

Comment 2

[Nicholas Nethercote [inactive]]

Comment on attachment 666361 [details] [diff] [review]
Patch to be reviewed

Review of attachment 666361 [details] [diff] [review]:
-----------------------------------------------------------------

I did it the current way because I was wary of having multiple inheritance, but there's no real reason not to have it in this case and it does make things clearer.  Thanks.

Comment 3

[Eddy Bruel [:ejpbruel]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef321673c843

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ef321673c843