Fix missing trampolines for named arguments.

Fix missing trampolines for named arguments.

To elide the trampoline, the named arguments must be in exactly the right positions, and not merely in the correct order to preserve evaluation order semantics.

BUG=
TEST=

Committed: https://code.google.com/p/dart/source/detail?r=2418

[Jennifer Messerly, 2011-12-14 19:15:32]

Thanks for this fix! lgtm

http://codereview.chromium.org/8912017/diff/4001/frog/member.dart
File frog/member.dart (right):

http://codereview.chromium.org/8912017/diff/4001/frog/member.dart#newcode820
frog/member.dart:820: bool namesInHomePositions(Arguments args) {
mind adding a comment to this? I'm not familiar with the term "home positions"

I was wondering if this works for cases like:

class Foo { foo(a,[b=1,c=2,d=3]) { print(d); } }
main() { var f = new Foo(); f.foo(0,b:4,c:5); }

... discovered, it does, because _useDirectCall checks that any trailing default
args are null.

http://codereview.chromium.org/8912017/diff/4001/frog/member.dart#newcode824
frog/member.dart:824: if (i >= parameters.length)
style nit: ifs should be one line, or use { ... }

http://codereview.chromium.org/8912017/diff/4001/frog/var_member.dart
File frog/var_member.dart (right):

http://codereview.chromium.org/8912017/diff/4001/frog/var_member.dart#newcode248
frog/var_member.dart:248: // collide with any other name.
Some examples would be helpful--I'm not sure what kinds of things you're worried
about.

FWIW, there are some things on "world" that might help:
    toJsIdentifier -> currently built assuming ':' appears in only certain
positions, but could be generalized like you did here.
    _addTopName -> tracks top level names to handle collisions

we do prevent user methods from containing a single '$', and our special ones
should only have a leading $. (i'm also moving the special ones to natives.dart,
where they go through better name collision detection, fwiw)

http://codereview.chromium.org/8912017/diff/4001/frog/var_member.dart#newcode255
frog/var_member.dart:255: nameBuilder.add('${name.length}');
do you need length here, or would it be enough to add something like _ before
and after the name? mostly asking out of curiosity -- the length works for me
too.

[sra1, 2011-12-14 22:04:32]

Comments addressed in http://codereview.chromium.org/8915021

http://codereview.chromium.org/8912017/diff/4001/frog/member.dart
File frog/member.dart (right):

http://codereview.chromium.org/8912017/diff/4001/frog/member.dart#newcode820
frog/member.dart:820: bool namesInHomePositions(Arguments args) {
On 2011/12/14 19:15:33, John Messerly wrote:
> mind adding a comment to this? I'm not familiar with the term "home positions"
> 
> I was wondering if this works for cases like:
> 
> class Foo { foo(a,[b=1,c=2,d=3]) { print(d); } }
> main() { var f = new Foo(); f.foo(0,b:4,c:5); }
> 
> ... discovered, it does, because _useDirectCall checks that any trailing
default
> args are null.

Done.

http://codereview.chromium.org/8912017/diff/4001/frog/member.dart#newcode824
frog/member.dart:824: if (i >= parameters.length)
On 2011/12/14 19:15:33, John Messerly wrote:
> style nit: ifs should be one line, or use { ... }

Done.

http://codereview.chromium.org/8912017/diff/4001/frog/var_member.dart
File frog/var_member.dart (right):

http://codereview.chromium.org/8912017/diff/4001/frog/var_member.dart#newcode248
frog/var_member.dart:248: // collide with any other name.
On 2011/12/14 19:15:33, John Messerly wrote:
> Some examples would be helpful--I'm not sure what kinds of things you're
worried
> about.
> 
> FWIW, there are some things on "world" that might help:
>     toJsIdentifier -> currently built assuming ':' appears in only certain
> positions, but could be generalized like you did here.
>     _addTopName -> tracks top level names to handle collisions
> 
> we do prevent user methods from containing a single '$', and our special ones
> should only have a leading $. (i'm also moving the special ones to
natives.dart,
> where they go through better name collision detection, fwiw)

Done.

http://codereview.chromium.org/8912017/diff/4001/frog/var_member.dart#newcode255
frog/var_member.dart:255: nameBuilder.add('${name.length}');
On 2011/12/14 19:15:33, John Messerly wrote:
> do you need length here, or would it be enough to add something like _ before
> and after the name? mostly asking out of curiosity -- the length works for me
> too.

I've kept the length.  If you put '_' in there, how do you tell the added '_'
from pre-existing ones?  A good way to think about this kind of encoding is to
imagine the code for reversing the transformation.