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

mirrors Invoke API is unnecessary picky given our provided input API #16745

Closed
efortuna opened this issue Feb 11, 2014 · 9 comments
Closed

mirrors Invoke API is unnecessary picky given our provided input API #16745

efortuna opened this issue Feb 11, 2014 · 9 comments
Assignees
Labels
area-library closed-as-intended Closed as the reported issue is expected behavior library-mirrors

Comments

@efortuna
Copy link
Contributor

To call invoke, you pass in positional arguments and named arguments.

InstanceMirror invoke(Symbol memberName, List positionalArguments, [Map<Symbol, dynamic> namedArguments])

Invoke throws an error if you call invoke on member that has no named arguments and you pass in an empty Map. Same for the positional arguments and an empty list.

The problem is if you get this info from an Invocation object, like from noSuchMethod, and you call invocationObj.namedArguments or invocationObject.positionalArguments they return empty lists if that member doesn't have any named/positional arguments.

As a result, to use invoke and noSuchMethod effectively, you have to individually determine if the invocation is a getter/setter/method/accessor and call invoke on each of those cases differently!

This defeats the purpose of noSuchMethod and invoke. So we probably just need to align our APIs so that either Invocation.namedArguments returns null instead of an empty list if that method/getter/etc doesn't take any arguments, or don't throw an error in invoke if we pass an empty list to a getter.

@gbracha
Copy link
Contributor

gbracha commented Feb 12, 2014

Set owner to @rmacnak-google.
Added Accepted label.

@rmacnak-google
Copy link
Contributor

"Invoke throws an error if you call invoke on member that has no named arguments and you pass in an empty Map."

I do not see this behavior.

import 'dart:mirrors';

class C {
  foo(x) => x;
}

main() {
 var emptyMap = {};
 print(reflect(new C()).invoke(#foo, [3], emptyMap));
}

prints an instance mirror on 3 and does not throw an exception.

Can you provide an example that breaks?

"As a result, to use invoke and noSuchMethod effectively, you have to individually determine if the invocation is a getter/setter/method/accessor and call invoke on each of those cases differently!"

It sounds like you should be using InstanceMirror.delegate, which handles this for you. (It would be nice if Dart didn't distinguish between four/five kinds of invocation, but it is too late to change that.)


Added NeedsInfo label.

@efortuna
Copy link
Contributor Author

The error is easily reproduced if you change foo to be anything other than a method:

import 'dart:mirrors';

class C {
  int get foo => 3;
}

main() {
 print(reflect(new C()).invoke(#foo, [3]));
}

results in:
Unhandled exception:
Class 'C' has no instance method 'foo'.

NoSuchMethodError : method not found: 'foo'
Receiver: Instance of 'C'
Arguments: [3]
#­0 Object.noSuchMethod (dart:core-patch/object_patch.dart:42)
#­1 _LocalInstanceMirrorImpl._invoke (dart:mirrors-patch/mirrors_impl.dart:316)
#­2 _LocalInstanceMirrorImpl.invoke (dart:mirrors-patch/mirrors_impl.dart:309)
#­3 main (file:///Users/efortuna/dart-git2/dart/foo.dart:164:31)
#­4 _startIsolate.isolateStartHandler (dart:isolate-patch/isolate_patch.dart:190)
#­5 _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:93)


Added Triaged label.

@rmacnak-google
Copy link
Contributor

It looks like you're running a VM from before Issue #8 was fixed. You should see

Unhandled exception:
Class 'int' has no instance method 'call'.

NoSuchMethodError : method not found: 'call'
Receiver: 3
Arguments: [3]
#­0 Object.noSuchMethod (dart:core-patch/object_patch.dart:45)
#­1 _LocalInstanceMirror._invoke (dart:mirrors-patch/mirrors_impl.dart:457)
#­2 _LocalInstanceMirror.invoke (dart:mirrors-patch/mirrors_impl.dart:453)
#­3 main (file:///media/SolidState/dart-git/dart/b.dart:8:32)
#­4 _startIsolate.isolateStartHandler (dart:isolate-patch/isolate_patch.dart:216)
#­5 _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:115)

which is what you would see if you wrote 'new C().get(3)'

@gbracha
Copy link
Contributor

gbracha commented Feb 13, 2014

re comment 4: should say new C().foo(3)

@efortuna
Copy link
Contributor Author

Yeah, sorry I my $dart was older than my checkout.
When I run with a fresh build,

class C {
  int get foo => 3;
}

main() {
 print(reflect(new C()).invoke(#foo, []));
}

$ xcodebuild/ReleaseIA32/dart-sdk/bin/dart foo.dart
Unhandled exception:
Class 'int' has no instance method 'call'.

NoSuchMethodError : method not found: 'call'
Receiver: 3
Arguments: []
#­0 Object.noSuchMethod (dart:core-patch/object_patch.dart:45)
#­1 _LocalInstanceMirror._invoke (dart:mirrors-patch/mirrors_impl.dart:454)
#­2 _LocalInstanceMirror.invoke (dart:mirrors-patch/mirrors_impl.dart:450)
#­3 main (file:///Users/efortuna/dart-git2/dart/foo.dart:163:31)
#­4 _startIsolate.isolateStartHandler (dart:isolate-patch/isolate_patch.dart:216)
#­5 _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:115)

(note In my first example I incorrectly still was passing in a 3 in the argument instead of an empty list)

@rmacnak-google
Copy link
Contributor

Okay, this is correct behavior. Did you want this return 3? That wouldn't be Dart.

@DartBot
Copy link

DartBot commented Feb 14, 2014

This comment was originally written by @seaneagan


The problem with delegate though is there is no easy way to create Invocations, which is issue #14776.

@rmacnak-google
Copy link
Contributor

Invocations of regular methods, getters, setters, and constructors are distinct in Dart.


Added AsDesigned label.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-library closed-as-intended Closed as the reported issue is expected behavior library-mirrors
Projects
None yet
Development

No branches or pull requests

5 participants