Changed the API in DartEntry for invoking dart code from C++ to make it more compatible with the re…

Changed the API in DartEntry for invoking dart code from C++ to make it more compatible with the requirements of the runtime.

Deleted all the code duplication that was added to circumvent the old DartEntry API requirements.
Committed: https://code.google.com/p/dart/source/detail?r=16288

[regis, 2012-12-18 16:21:28]

LGTM

https://chromiumcodereview.appspot.com/11613009/diff/9002/vm/dart_entry.cc
File vm/dart_entry.cc (right):

https://chromiumcodereview.appspot.com/11613009/diff/9002/vm/dart_entry.cc#ne...
vm/dart_entry.cc:83: RawObject* DartEntry::InvokeClosure(const Instance&
closure,
Remove closure as argument.

https://chromiumcodereview.appspot.com/11613009/diff/9002/vm/dart_entry.cc#ne...
vm/dart_entry.cc:92: const Instance& instance,
Remove instance as argument.

https://chromiumcodereview.appspot.com/11613009/diff/9002/vm/dart_entry.h
File vm/dart_entry.h (right):

https://chromiumcodereview.appspot.com/11613009/diff/9002/vm/dart_entry.h#new...
vm/dart_entry.h:104: static RawObject* InvokeDynamicWithNamedArgs(
It's great not to have to pass the arguments descriptor in the shorter form
above. The name of this longer form implies that we pass named arguments, but we
do not have to. I wonder if we could overload the name ("InvokeDynamic" as well)
and pass the arguments_descriptor last. We use overloading for New, for example.

https://chromiumcodereview.appspot.com/11613009/diff/9002/vm/dart_entry.h#new...
vm/dart_entry.h:117: static RawObject* InvokeStaticWithNamedArgs(
Overload "InvokeStatic" and swap arguments?

https://chromiumcodereview.appspot.com/11613009/diff/9002/vm/dart_entry.h#new...
vm/dart_entry.h:125: static RawObject* InvokeClosure(const Instance& closure,
The closure is the first argument and does not need to be passed in. But I can
simplify this later.

https://chromiumcodereview.appspot.com/11613009/diff/9002/vm/dart_entry.h#new...
vm/dart_entry.h:130: static RawObject* InvokeClosureWithNamedArgs(
Overload "InvokeClosure" and swap arguments?

[siva, 2012-12-18 21:18:13]

Thanks.

https://chromiumcodereview.appspot.com/11613009/diff/9002/vm/dart_entry.cc
File vm/dart_entry.cc (right):

https://chromiumcodereview.appspot.com/11613009/diff/9002/vm/dart_entry.cc#ne...
vm/dart_entry.cc:83: RawObject* DartEntry::InvokeClosure(const Instance&
closure,
See comment in dart_entry.h file regarding this.

On 2012/12/18 16:21:28, regis wrote:
> Remove closure as argument.

https://chromiumcodereview.appspot.com/11613009/diff/9002/vm/dart_entry.cc#ne...
vm/dart_entry.cc:92: const Instance& instance,
See comment in dart_entry.h file regarding this.

On 2012/12/18 16:21:28, regis wrote:
> Remove instance as argument.

https://chromiumcodereview.appspot.com/11613009/diff/9002/vm/dart_entry.h
File vm/dart_entry.h (right):

https://chromiumcodereview.appspot.com/11613009/diff/9002/vm/dart_entry.h#new...
vm/dart_entry.h:104: static RawObject* InvokeDynamicWithNamedArgs(
Agree, I have changed the name to just InvokeDynamic.

On 2012/12/18 16:21:28, regis wrote:
> It's great not to have to pass the arguments descriptor in the shorter form
> above. The name of this longer form implies that we pass named arguments, but
we
> do not have to. I wonder if we could overload the name ("InvokeDynamic" as
well)
> and pass the arguments_descriptor last. We use overloading for New, for
example.

https://chromiumcodereview.appspot.com/11613009/diff/9002/vm/dart_entry.h#new...
vm/dart_entry.h:117: static RawObject* InvokeStaticWithNamedArgs(
On 2012/12/18 16:21:28, regis wrote:
> Overload "InvokeStatic" and swap arguments?

Done.

https://chromiumcodereview.appspot.com/11613009/diff/9002/vm/dart_entry.h#new...
vm/dart_entry.h:125: static RawObject* InvokeClosure(const Instance& closure,
I thought about that too, but somehow felt it is more
readable this way it is more readable and consistent
with the way the InvokeDynamic and InvokeStatic are
structured (i.e the first parameter signifies what is being
invoked). We can discuss about this some more. I am going to
leave it as is for now.

On 2012/12/18 16:21:28, regis wrote:
> The closure is the first argument and does not need to be passed in. But I can
> simplify this later.

https://chromiumcodereview.appspot.com/11613009/diff/9002/vm/dart_entry.h#new...
vm/dart_entry.h:130: static RawObject* InvokeClosureWithNamedArgs(
Swapped the arguments but retained the closure parameter per
my comment above.

On 2012/12/18 16:21:28, regis wrote:
> Overload "InvokeClosure" and swap arguments?

[regis, 2012-12-18 21:34:46]

Great. Still LGTM

https://chromiumcodereview.appspot.com/11613009/diff/9002/vm/dart_entry.h
File vm/dart_entry.h (right):

https://chromiumcodereview.appspot.com/11613009/diff/9002/vm/dart_entry.h#new...
vm/dart_entry.h:125: static RawObject* InvokeClosure(const Instance& closure,
On 2012/12/18 21:18:13, siva wrote:
> I thought about that too, but somehow felt it is more
> readable this way it is more readable and consistent
> with the way the InvokeDynamic and InvokeStatic are
> structured (i.e the first parameter signifies what is being
> invoked). We can discuss about this some more. I am going to
> leave it as is for now.
> 
> On 2012/12/18 16:21:28, regis wrote:
> > The closure is the first argument and does not need to be passed in. But I
can
> > simplify this later.
> 

Sure. It is definitely more readable to repeat the closure.

https://chromiumcodereview.appspot.com/11613009/diff/15001/vm/dart_entry.h
File vm/dart_entry.h (right):

https://chromiumcodereview.appspot.com/11613009/diff/15001/vm/dart_entry.h#ne...
vm/dart_entry.h:104: static RawObject* InvokeDynamic(
Restore indentation as for InvokeDynamic above?

https://chromiumcodereview.appspot.com/11613009/diff/15001/vm/dart_entry.h#ne...
vm/dart_entry.h:117: static RawObject* InvokeStatic(
Restore indentation as for InvokeStatic above?

https://chromiumcodereview.appspot.com/11613009/diff/15001/vm/dart_entry.h#ne...
vm/dart_entry.h:130: static RawObject* InvokeClosure(
Restore indentation as for InvokeClosure above?

[siva, 2012-12-18 23:14:36]

https://chromiumcodereview.appspot.com/11613009/diff/15001/vm/dart_entry.h
File vm/dart_entry.h (right):

https://chromiumcodereview.appspot.com/11613009/diff/15001/vm/dart_entry.h#ne...
vm/dart_entry.h:104: static RawObject* InvokeDynamic(
Forgot these in my last submission, will make these changes in the next CL.

On 2012/12/18 21:34:46, regis wrote:
> Restore indentation as for InvokeDynamic above?

https://chromiumcodereview.appspot.com/11613009/diff/15001/vm/dart_entry.h#ne...
vm/dart_entry.h:117: static RawObject* InvokeStatic(
Ditto.

On 2012/12/18 21:34:46, regis wrote:
> Restore indentation as for InvokeStatic above?

https://chromiumcodereview.appspot.com/11613009/diff/15001/vm/dart_entry.h#ne...
vm/dart_entry.h:130: static RawObject* InvokeClosure(
Ditto.

On 2012/12/18 21:34:46, regis wrote:
> Restore indentation as for InvokeClosure above?