Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.
This repository was archived by the owner on Feb 22, 2018. It is now read-only.

Generate code for recursive generic definitions #253

Closed
@vsmenon

Description

@vsmenon

We currently break on code of the following form:

class ElementInjector extends TreeNode<ElementInjector> {
  ...
}

we generate:

class ElementInjector extends TreeNode$(ElementInjector) {

which generates reference error on ElementInjector.

(edit by @jmesserly) a lot of discussion below is obsolete, see my recent comment for proposed solution.

Activity

jmesserly

jmesserly commented on Jul 10, 2015

@jmesserly
Contributor

yeah we don't support recursively bounded quantification. do we want to?

EDIT: see this comment for a solution that will work.


If yes, we likely need to change how library cycles are compiled, to deal with pathological cases like:
(edit: removed obsolete code example)

without library cycles, it's not too bad. The safest way to handle this is probably, instead of:

class A extends B$(A) { ... }

generate:

class A {} // forward declare
mergeClass(A, class extends B$(A) { ... });

mergeClass copies instance and static methods, as well as fixing what A extends.

We might be able to just fix up "extends", but I think super bindings might only work if we do it as shown above. Not 100% sure though.

None of this works for lib cycles+incremental compile tho

self-assigned this
on Aug 5, 2015
jmesserly

jmesserly commented on Aug 7, 2015

@jmesserly
Contributor

One thing to note: we could take a different approach, and change how reified generics work. We could do a type tagging approach for generic type instances, where each instance points at its runtime type, and from there can get to various generic args. But that brings in a bunch of issues (by un-solving some of the problems the current approach solves).

jmesserly

jmesserly commented on Aug 27, 2015

@jmesserly
Contributor

I think we can fix this by extending the lazy-load mechanism that we already use to deal with library cycles. Two parts to this:

  • the declaration loader needs to understand and track that elements can refer to themselves. (right now, it understands cycles in library imports, but not in top-level Elements, like static fields/classes.)
    lazyClass needs to handle the case where a class is requested while its being defined, and understand how to hand out a forward declaration, then merge in the members.
    verify that references to the class name from inside itself are generated as qualified names (probably works already.)
vsmenon

vsmenon commented on Nov 9, 2015

@vsmenon
ContributorAuthor

@jmesserly

I think I may have asked you this before, and it didn't work out, but I don't recall why. Could we change the prototype after the fact along these lines?:

Change this:

class A extends B$(T) { ... }  // Where T is unavailable

to:

class A extends B$(dart.dynamic) { ... }
...
A.prototype.__proto__ = B$(T).prototype;  // Once T is available - i.e., immediately if A == T
jmesserly

jmesserly commented on Nov 9, 2015

@jmesserly
Contributor

I think the problem is with super being very early bound in JS. So:

// note: slightly simplified compared with real output
let B$ = (T) => class B {
  foo() {
    console.log('B.foo: ' + T.toString());
  }
};
class A extends B$(dart.dynamic) {
  foo() {
    console.log('A.foo: ' + T.toString());
    super.foo();
  }
}
A.prototype.__proto__ = B$(A).prototype; 
 // I think A.foo prints A, but B.foo prints dynamic
new A().foo();

But yeah, if we could mutate the prototype, and it would affect super, then it would work.

We could perhaps avoid super in these cases, but trying to detect when to avoid super and make the dispatch work at runtime is a bit tricky, IIRC. But that's an option.

I guess I was leaning towards something like:

class A {} // forward declare
mergeClass(A, class extends B$(A) { ... });

basically, we have methods that are immutable (at least, w.r.t. super), but we have a class that's mutable, so we initialize the methods in a way that makes them happy, then copy them over to the class.

leafpetersen

leafpetersen commented on Nov 9, 2015

@leafpetersen
Contributor

Forward declaration seems promising. Does it work when both are generic?

class A<T> {
  T f(T x) => x;
}

class B<T> extends A<B<T>> {
}

void main () {
  B<int> x = new B<int>();
  x = x.f(x);
} 
vsmenon

vsmenon commented on Nov 10, 2015

@vsmenon
ContributorAuthor

Ahh, right - super - you told me that before....

jmesserly

jmesserly commented on Nov 10, 2015

@jmesserly
Contributor

Forward declaration seems promising. Does it work when both are generic?

For that example, yes:

let A$ = dart.generic((T) => class A { ... });
let B$ = dart.generic((T) => {
  class B {}
  return mergeClass(B, class extends A$(B));
});
function main() {
  new B$(core.int)();
}

The harder case is more along the lines of:

class B<T> extends A<C<T>> {}
class C<T> extends A<B<T>> {}

To support those, we'd have to let dart.generic handle the forward initialization & merging process itself. Which it should be able to do. (We might get some side benefits from that, like it's easier to compute signatures inside, since referring to B$(T) becomes generally safe. We had considered it before IIRC).

jmesserly

jmesserly commented on Nov 10, 2015

@jmesserly
Contributor

Restating this more fundamentally -- it's hard to implement letrec without mutation :)

We currently have a cycle.

the class -> class methods -> superclass methods -> generic type argument (= the class)

I'm picking on the class -> class method link as it's easy to mutate. However we could consider a design change that makes either the the class methods -> superclass linkage mutable, or a design change that makes methods -> generic type argument mutable.

To break method -> super method link, we'd have to avoid ES6 super. Not my first choice, but it's an option.

To break the method -> generic type arg link, we'd have to make T be mutable somehow. For example

let B$ = dart.generic((types) => class B { method() { print(types.T); });

Now we can provide a way to mutate T later. Just not sure if we want to change all generic types. (we don't know which ones someone will create a cycle with until later.) But maybe it's nice looking enough.

jmesserly

jmesserly commented on Nov 10, 2015

@jmesserly
Contributor

BTW, to complete that example, it would be like:

let B$ = dart.generic((types) => class B { method() { print(types.T); });

// We assume B$() with no args returns a fresh copy,
// with "undefined" type args, so we can initialize them later.
class A extends B$() { ... }
// gets ahold of the "types" object we stored previously and assigns T
// dart.typesArguments is just a Symbol
// A.prototype is our B$() from before.
A.prototype[dart.typesArguments].T = A;

edit x3: make it more obvious what's going on.

jmesserly

jmesserly commented on Nov 10, 2015

@jmesserly
Contributor

also: if dart.generic's function argument takes a "types" array I'm not sure how it would know the type parameter name ("T" in this case). I guess it can figure it out after it makes a type:

let typeArgs = {}; // empty at first
let newType = makeTypeFunction(typeArgs);
// will newType's type info will tell us the name "T", allowing us to set typeArgs['T']?

34 remaining items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @jmesserly@rakudrama@vsmenon@leafpetersen

        Issue actions

          Generate code for recursive generic definitions · Issue #253 · dart-archive/dev_compiler