Class definition order in generated JavaScript wrt. inheritance hierarchy

[DartBot]

This issue was originally filed by f.jo...@gmail.com

---

The $inherits function uses one of two methods: 1) rerouting the proto property of the child's prototype, and 2) replacing the child's prototype with a new object whose prototype is the parent's prototype.

Method 1 is used when the proto property is available, method 2 otherwise.

Method 2 has known ordering issues (see issue #168, fixed), but there is at least one other issue that is not fixed yet:

Let C extend B, and B extend A.

• If these classes are defined in the order: A, B, and C, then inheritance works correctly (now that issue Isolate method ordering in generated JS #168 is fixed).
• However if they are defined in the order: C, A, B, then inheritance does not work correctly: when B is made to inherit from A, its prototype is replaced, and the old prototype (which was used to make C inherit from B) is not "connected" anymore. Therefore C is no longer correctly inheriting from B.

Note that this issue does not occur with just two levels of class inheritance. This is the reason why the example above uses three classes.

One solution to this issue involves properly ordering class definitions in generated JavaScript code so that superclasses come before their child classes (e.g., making sure that A, B, and C are defined in this order).

This issue depends on three things: a) how the $inherits method implements inheritance when the proto property is not available, b) how the compiler orders class definitions in generated JavaScript code, and c) that the generated code is executed on a JavaScript engine which does not offer the proto property.

Therefore, this issue does not occur as such if no JavaScript code is generated. Moreover, it can only be seen when there are at least three levels of class inheritance in the code to compile, and when the compiler randomly generates an improper order.

This issue has been identified in the frog compiler r1848, but may exist in any other dart to JavaScript compiler that uses a similar $inherits function, and does not order class definitions properly.

Two additional remarks:

• This issue currently prevents the frog compiler (r1848) itself to correctly run when the proto property is not available: PartialFunctionElement extends FunctionElement, which extends Element, but they are defined in the following order: PartialFunctionElement, Element, and FunctionElement (note that there may be other occurrences of this issue as well). The error messages are not easy to link to this issue.
• I would advise that the tests on the frog compiler be run on at least two different JavaScript engines: one supporting the proto property, as well as another that does not.

[DartBot]

This comment was originally written by drfibonacci@google.com

---

Added Area-Frog, Triaged labels.

[jmesserly]

Set owner to @efortuna.
Added Started label.

[DartBot]

This comment was originally written by f.jouau...@gmail.com

---

Here are some additional notes:

• In addition to reordering the pieces of code generated for each class, it is also necessary to place the initialization of the prototypes of additional constructors (i.e., suffixed $ctor in the generated Javascript) after the call to $inherits. This is #­168, which was about methods, extended to additional constructors.
• It should also be possible to simply put all calls to $inherits at the beginning of the generated code in the proper order (i.e., it is not strictly necessary to reorder everything). They do not need to come after main constructor definition since functions are added to the variable object before code execution.
• It is currently possible to make minfrog (or even archived versions of frogsh) work without actually reordering much code. However, this involves replacing $inherits with a function that remembers all inheritance relationships, and applies them in proper order at the end (taking care to copy old children prototype properties to new children prototype). Moreover, a few other things have to be taken care of (like additional constructors, and calls to $inheritsMembers.
• Other possibilites might also work, like: adding code to $inherits to copy properties of old prototype to new prototype (but this still requires placing additional constructor prototype initialization after calls to $inherits). However, my personal preference still goes to ordering classes properly.

[efortuna]

Thanks for all the additional info. I have a fix already implemented that should fix all of these issues, I am just working on adding tests before I check in, so that this doesn't get broken again!

[efortuna]

Added Fixed label.