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

We should consider adding a method isSubtypeOf in class Type to check if a type instance is a sub type of another instance #23764

Closed
a-siva opened this issue Jul 1, 2015 · 5 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. closed-obsolete Closed as the reported issue is no longer relevant

Comments

@a-siva
Copy link
Contributor

a-siva commented Jul 1, 2015

The widget code in Sky uses mirrors in the following scenario to query if the type of an object is a subtype of another target type.

Widget findAncestor(Type targetType) {
var ancestor = _parent;
while (ancestor != null && !reflectClass(ancestor.runtimeType).isSubtypeOf(reflectClass(targetType)))
ancestor = ancestor._parent;
return ancestor;
}

We would like to eliminate all uses of mirrors in Sky code as we will have issues with it when precompiling and tree shaking for the ios platform

@sgmitrovic @crelier

@a-siva a-siva added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. Accepted labels Jul 1, 2015
@crelier
Copy link
Contributor

crelier commented Jul 1, 2015

We could provide this functionality in the class Type of the SDK. Instead of

abstract class Type {}

we could have

abstract class Type {
bool equals(Type other);
bool isSubtypeOf(Type other);
bool isMoreSpecificThan(Type other);
Type superType();
}

Other methods to inspect function types may be useful too:
bool isFunctionType();
int FunctionTypeParameterCount();
Type FunctionTypeParameterTypeAt(int i);
Type FunctionTypeResultType();

static Type VoidType();
static Type DynamicType();
static Type NullType();

To be complete, we would also need access to optional arguments, since they are considered in function type tests:
int FunctionTypeFixedParameterCount();
int FunctionTypeOptionalPositionalParameterCount();
int FunctionTypeOptionalNamedParameterCount();
String FunctionTypeParameterNameAt(int i);

All the above is already implemented in C++ in the VM and would just have to be exposed on the Dart side. This is an API change, obviously.

@lrhn
Copy link
Member

lrhn commented Jul 7, 2015

I'm sympathetic to adding more functionality on Type (so it has a reason to exist except as an opaque token tightly coupled with the mirror system), but it's not uncontroversial.

Personally I'd want at least:

  class Type<T> {
    bool isInstance(object) => object is T;
    bool isSubtypeOf(Type other) => ...;
 }

I don't think the function inspection is a good idea - that's too close to reflection, and then you can just use mirrors.

In general, passing around Type objects is not a good idea (right now). They are pretty much useless without the mirror system, so it would be better to have some other representation of your widgets than a Type object. For the findAncestor function I would probably go for: findAncestor(bool test(Widget widget)) instead of passing in a Type object. That's far more general. Then the user can do: findAncestor((x) => x is FooWidget) instead of findAncestor(FooWidget), getting the full subtype-test support of "is" without having to implement it manually. If we get generic methods, it can even be: findAncestor<T extends Widget>([bool test(T widget)]) which checks parents with is T and can then also use the optional extra test.

In short: If you use Type objects for anything else than putting them into the mirror syustem, it'll most likely suck, so don't do that.

Adding anything to Type will conflict with the metaclass proposal which wants to treat static members of a class as instance members a corresponding Type object - that would be complicated if some names were shadowed by the Type class itself.

@a-siva
Copy link
Contributor Author

a-siva commented Jul 7, 2015

I don't think we should speculatively add functionality without first having a use case for it. I think a good first step would be to find all the 'dart:mirrors' usage in the Sky framework and get that functionality in through other means.

Regarding passing Type objects around, we can only suggest as a recommended patterns but it is always up to the user as to how they use it.

@sethladd
Copy link
Contributor

sethladd commented Jul 7, 2015

@larsbak had a suggestion for the Sky code in this particular case, in order to eliminate the need for mirrors here.

@kevmoo kevmoo removed the accepted label Feb 29, 2016
@mraleph mraleph added the closed-obsolete Closed as the reported issue is no longer relevant label Mar 6, 2018
@mraleph
Copy link
Member

mraleph commented Mar 6, 2018

I think this issue is stale - and it is not a VM issue but a language/core library one

@mraleph mraleph closed this as completed Mar 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. closed-obsolete Closed as the reported issue is no longer relevant
Projects
None yet
Development

No branches or pull requests

6 participants