Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Super classes and interfaces of annotated classes should also be reflectable. #11

Closed
jakemac53 opened this issue Jul 28, 2015 · 3 comments

Comments

@jakemac53
Copy link
Contributor

This is needed in order to walk the super chain to get all members of a class. It is not possible to add an annotation to all super classes since many of them live in the sdk (Object for example).

For example, currently this will fail:

class MyReflectable extends Reflectable {
  const MyReflectable() : super(instanceInvokeCapability);
}
const myReflectable = const MyReflectable()

@myReflectable
class MyClass {}

main() {
  var typeMirror = myReflectable.reflectType(MyClass);

  // Throws exception because `Object` is not reflectable.
  var superclass = typeMirror.superclass;
}

This could be hidden possibly under a capability, such as superclassCapability or something along those lines to avoid code bloat in cases that don't need it?

@eernstg
Copy link
Collaborator

eernstg commented Jul 29, 2015

I think that it is reasonable to say that it is already covered. The inclusion of otherwise-not-reflectable superclasses is implied by typeRelationsCapability, because that capability subsumes the superClassCapability (which never existed, but it is implicitly present on p.7 in Fig.1 in the capability design document, and is then later unified with others into typeRelationsCapability).

Granted, it's not the same thing to promise that superclass can be used on a ClassMirror, and promising that the set of reflectable classes will be upwards closed (i.e., that all direct and indirect superclasses of reflectable classes will also be considered reflectable). However, it has essentially a zero cost to support the use of the superclass method among class mirrors for a given set of reflectable classes, whereas the upward-closure may have a substantial cost (because it can make many additional classes reflectable, and each of them has many features).

Based on this, I'll go for a model where upward-closure of the set of reflectable classes is part of the semantics of typeRelations. If needed, we can still split capabilities into more fine-grained ones, and reinterpret the more coarse-grained ones as 'grouping tokens' (p10 in the design document).

I'm currently implementing this. Note that many other parts of typeRelations are still not implemented.

@jakemac53
Copy link
Contributor Author

ok, I filed a separate bug about the current error that is thrown and linked it here

@eernstg
Copy link
Collaborator

eernstg commented Jul 30, 2015

FYI: Now sending a CL to review that includes code for upwards-closing the set of reflectable classes in case there is a typeRelationsCapability on the given reflector.

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

No branches or pull requests

2 participants