Skip to content

ObjectMirror.invoke ignores typeArguments #33594

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

Closed
thosakwe opened this issue Jun 23, 2018 · 13 comments
Closed

ObjectMirror.invoke ignores typeArguments #33594

thosakwe opened this issue Jun 23, 2018 · 13 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-not-planned Closed as we don't intend to take action on the reported issue library-mirrors

Comments

@thosakwe
Copy link

thosakwe commented Jun 23, 2018

  • Dart SDK Version: 2.0.0-dev.64.1
  • Mac OSX Sierra
  • Browser: N/A

I'm updating the package json_god to work with Dart 2; it's a dart:mirrors-based reflection solution. The actual functionality works, but for the fact that Dart 2's generics are reified, and so to work with typed collections, I need to somehow attach type information to objects.

The solution I am attempting is to reflectively call List.cast with the necessary type argument; however, the ObjectMirror.delegate method forwards to ObjectMirror.invoke, which doesn't use the type arguments. Thus, the type arguments are ignored, and items like List<String> remain List<dynamic>.


My code:

logger.info('Casting list elements to ${typeArguments[0].reflectedType}');
var inv = new Invocation.genericMethod(#cast,
   [typeArguments[0].reflectedType], []);
logger.info('INVOCATION OF ${inv.memberName} with type args: ${inv
    .typeArguments}');
var output = reflect(it.toList()).delegate(inv);
logger.info('Casted list type: ${output.runtimeType}');

The current output:

[INFO] json_god: Casting list elements to String
[INFO] json_god: INVOCATION OF Symbol("cast") with type args: [String]
[INFO] json_god: About to deserialize es2015 to a String
[INFO] json_god: Value es2015 is a primitive
[INFO] json_god: About to deserialize stage-0 to a String
[INFO] json_god: Value stage-0 is a primitive
[INFO] json_god: Casted list type: CastList<dynamic, dynamic>

It's evident that that the type arguments are not actually being applied at all. I can't think of any possible workaround for this, unfortunately.

@thosakwe thosakwe changed the title ObjectMirror.invoke doesn't support typeArguments Instance.invoke doesn't support typeArguments Jun 23, 2018
@thosakwe thosakwe changed the title Instance.invoke doesn't support typeArguments InstanceMirror.invoke doesn't support typeArguments Jun 23, 2018
@thosakwe thosakwe changed the title InstanceMirror.invoke doesn't support typeArguments ObjectMirror.invoke doesn't support typeArguments Jun 23, 2018
@thosakwe thosakwe changed the title ObjectMirror.invoke doesn't support typeArguments ObjectMirror.invoke ignores typeArguments Jun 23, 2018
@lrhn lrhn added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-mirrors labels Jun 25, 2018
@thosakwe
Copy link
Author

thosakwe commented Jul 6, 2018

Is there anything else I can do about this? This bug means that anything that uses dart:mirrors for serialization, etc., will not be able to perform casts, and always produce runtime errors.

@devoncarew
Copy link
Member

cc @rmacnak-google

@matanlurey
Copy link
Contributor

FWIW, you really won't want to use .cast as part of a normal workflow. It creates a forwarding implementation of the underlying collection, and is mostly meant to be a temporary stop-gap. For more data sets, a defensive copy is probably better:

https://www.dartlang.org/guides/language/effective-dart/usage#avoid-using-cast

(Still doesn't help you with mirrors, of course)

@sestegra
Copy link
Contributor

sestegra commented Jul 8, 2018

Did you try List<T>.from ?

@thosakwe
Copy link
Author

Thanks for the tip - I was able to resolve it by using List.from, which works perfectly.

@mdempsky
Copy link
Contributor

I'd like to work on this, if no one else is planning on looking into it soon.

The solution I'm thinking of:

  1. Add an invokeGeneric method to ObjectMirror with signature:

    InstanceMirror invokeGeneric(
        Symbol memberName,
        List<Type> typeArguments,
        List positionalArguments,
        [Map<Symbol, dynamic> namedArguments])
    

    (Analogous to Invocation having method and genericMethod factory methods.)

    Q: Should List<Type> be List<TypeMirror>? Is there a reason List and Map<Symbol, dynamic> aren't List<InstanceMirror> and Map<Symbol, InstanceMirror>, respectively?

  2. Change invoke and delegate to use this method instead.

  3. Plumb the extra parameter down through runtime/lib/mirrors.cc and into runtime/vm/object.cc. I see some TODOs by @crelier about "Support invocation of generic functions with type arguments," so I assume once I get that far down, then the rest of the code below is in place.

Does that sound like a reasonable plan? Or are there other aspects of dart:mirrors that should be updated for generics at the same time?

@crelier
Copy link
Contributor

crelier commented May 24, 2019

When generic functions got implemented in the VM, mirrors were supposed to be removed. Therefore, mirrors were only modified to gracefully ignore type arguments on functions without crashing. But no work was done to support them fully.

You can try to support generic functions in mirrors, but I think you will run into multiple issues. Mirrors would require a major revision for this to work properly. The author of mirrors, @rmacnak-google, can comment further.

@mdempsky
Copy link
Contributor

@crelier Thanks for the background. To clarify, when you say mirrors were supposed to be removed, is that still the plan? Or has that changed?

@crelier
Copy link
Contributor

crelier commented May 24, 2019

Mirrors definitely survived much longer than expected. I am not clear about their future, though. @rmacnak-google may know.

@bobjackman
Copy link

bobjackman commented May 30, 2019

It would also be hugely helpful for ClassMirror.newInstance() to accept generic type arguments as well since I need to reflectively instantiate a class...

reflectClass(StreamController).newGenericInstance(Symbol(""), [<expectedType>], []);
// should yield instance of StreamController<expectedType>

@rmacnak-google
Copy link
Contributor

@kogi That is already possible with

reflectType(StreamController, [<expected type>]).newInstance(Symbol(""), []);

@mdempsky I don't see mirrors being removed. The base language still isn't capable of abstracting over classes, libraries or messages.

@bobjackman
Copy link

@rmacnak-google thank you!

@a-siva
Copy link
Contributor

a-siva commented Aug 5, 2022

At this point we have no plans for enhancing dart:mirrors library.
Please see #44489 which is investigating a path towards discontinuing support for dart:mirrors

@a-siva a-siva added the closed-not-planned Closed as we don't intend to take action on the reported issue label Aug 5, 2022
@a-siva a-siva closed this as completed Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-not-planned Closed as we don't intend to take action on the reported issue library-mirrors
Projects
None yet
Development

No branches or pull requests

10 participants