Dynamically dispatch getters and setters on dynamically-typed variables.

Dynamically dispatch getters and setters on dynamically-typed variables.

This breaks MethodBindingTest, but only because first-class methods aren't
really supported by Frog in the first place. The fundamental issue is that Frog
doesn't know to compile a property-syntax getter for a method (e.g.
"get$methodName"). Teaching it to do so is outside the scope of this change.

This would technically fix ConstObjectsAreImmutableTest, but only because it
causes what used to be "a.x = 499" to become "a.set$x(499)", which raises an
error not because a is const but because A doesn't have a "set$x" method. Thus
this change adds a type declaration to the test so it continues to use "a.x =
499".

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

[nweiz, 2011-11-15 00:04:08]

http://codereview.chromium.org/8567010/diff/1/frog/member.dart
File frog/member.dart (right):

http://codereview.chromium.org/8567010/diff/1/frog/member.dart#newcode1402
frog/member.dart:1402: targets.forEach((m) => m._get(context, node, target,
isDynamic: true));
This line seems to be slowing down the compilation by about two orders of
magnitude when using the release build of the VM and frogc.dart. This slowdown
doesn't occur on Node with frog.dart, and I'm at a bit of a loss as to why it's
happening. Could it be something relating to the VM's memory limit?

[Jennifer Messerly, 2011-11-15 04:44:34]

http://codereview.chromium.org/8567010/diff/1/frog/member.dart
File frog/member.dart (right):

http://codereview.chromium.org/8567010/diff/1/frog/member.dart#newcode1402
frog/member.dart:1402: targets.forEach((m) => m._get(context, node, target,
isDynamic: true));
On 2011/11/15 00:04:08, nweiz wrote:
> This line seems to be slowing down the compilation by about two orders of
> magnitude when using the release build of the VM and frogc.dart. This slowdown
> doesn't occur on Node with frog.dart, and I'm at a bit of a loss as to why
it's
> happening. Could it be something relating to the VM's memory limit?

Sounds like it. I'd try passing: --new_gen_heap_size=64

http://codereview.chromium.org/8567010/diff/1/frog/member.dart#newcode1403
frog/member.dart:1403: returnValue = new Value(world.varType, null, node.span);
I think you want to use a more precise type here, when possible. Something like:
final returnType = reduce(map(targets, (t) => t.returnType), Type.union)

This same line is used in "invokeOnVar", probably can just factor it out. (I've
come close to factoring it out a few times :) )

same comment for set_

http://codereview.chromium.org/8567010/diff/1/frog/member.dart#newcode1475
frog/member.dart:1475: members.filter((m) => m.canInvoke(context,
args)).forEach((m) =>
why did you need this line?

invokeOnVar already takes care of it (via VarMember).

http://codereview.chromium.org/8567010/diff/1/tests/language/src/ConstObjects...
File tests/language/src/ConstObjectsAreImmutableTest.dart (right):

http://codereview.chromium.org/8567010/diff/1/tests/language/src/ConstObjects...
tests/language/src/ConstObjectsAreImmutableTest.dart:20: A a = const A(1, 2);
I wouldn't change the test. It's fine to mark something as "Pass # not really
passing because of XXX" in the status file.

Or even better: add another case here to capture both the var and non-var cases.

[nweiz, 2011-11-15 20:59:25]

http://codereview.chromium.org/8567010/diff/1/frog/member.dart
File frog/member.dart (right):

http://codereview.chromium.org/8567010/diff/1/frog/member.dart#newcode1402
frog/member.dart:1402: targets.forEach((m) => m._get(context, node, target,
isDynamic: true));
On 2011/11/15 04:44:34, John Messerly wrote:
> On 2011/11/15 00:04:08, nweiz wrote:
> > This line seems to be slowing down the compilation by about two orders of
> > magnitude when using the release build of the VM and frogc.dart. This
slowdown
> > doesn't occur on Node with frog.dart, and I'm at a bit of a loss as to why
> it's
> > happening. Could it be something relating to the VM's memory limit?
> 
> Sounds like it. I'd try passing: --new_gen_heap_size=64

Increasing the heap size seems to do the trick. Is it safe to commit with the
slowdown for the default heap size? Should I increase the heap size in
presubmit.py?

http://codereview.chromium.org/8567010/diff/1/frog/member.dart#newcode1403
frog/member.dart:1403: returnValue = new Value(world.varType, null, node.span);
On 2011/11/15 04:44:34, John Messerly wrote:
> I think you want to use a more precise type here, when possible. Something
like:
> final returnType = reduce(map(targets, (t) => t.returnType), Type.union)
> 
> This same line is used in "invokeOnVar", probably can just factor it out.
(I've
> come close to factoring it out a few times :) )
> 
> same comment for set_

Done. I guess I was too much in the mindset of --force-dynamic.

http://codereview.chromium.org/8567010/diff/1/frog/member.dart#newcode1475
frog/member.dart:1475: members.filter((m) => m.canInvoke(context,
args)).forEach((m) =>
On 2011/11/15 04:44:34, John Messerly wrote:
> why did you need this line?
> 
> invokeOnVar already takes care of it (via VarMember).

Oh, I suppose it does. I had assumed that it was necessary because the methods
were getting invoked below in the non-var case, but I guess that's just for
taking the union of the return values.

http://codereview.chromium.org/8567010/diff/1/tests/language/src/ConstObjects...
File tests/language/src/ConstObjectsAreImmutableTest.dart (right):

http://codereview.chromium.org/8567010/diff/1/tests/language/src/ConstObjects...
tests/language/src/ConstObjectsAreImmutableTest.dart:20: A a = const A(1, 2);
On 2011/11/15 04:44:34, John Messerly wrote:
> I wouldn't change the test. It's fine to mark something as "Pass # not really
> passing because of XXX" in the status file.
> 
> Or even better: add another case here to capture both the var and non-var
cases.

Added both var and non-var cases.

[Jennifer Messerly, 2011-11-16 23:42:35]

lgtm

http://codereview.chromium.org/8567010/diff/1/frog/member.dart
File frog/member.dart (right):

http://codereview.chromium.org/8567010/diff/1/frog/member.dart#newcode1402
frog/member.dart:1402: targets.forEach((m) => m._get(context, node, target,
isDynamic: true));
On 2011/11/15 20:59:25, nweiz wrote:
> On 2011/11/15 04:44:34, John Messerly wrote:
> > On 2011/11/15 00:04:08, nweiz wrote:
> > > This line seems to be slowing down the compilation by about two orders of
> > > magnitude when using the release build of the VM and frogc.dart. This
> slowdown
> > > doesn't occur on Node with frog.dart, and I'm at a bit of a loss as to why
> > it's
> > > happening. Could it be something relating to the VM's memory limit?
> > 
> > Sounds like it. I'd try passing: --new_gen_heap_size=64
> 
> Increasing the heap size seems to do the trick. Is it safe to commit with the
> slowdown for the default heap size? Should I increase the heap size in
> presubmit.py?

Yes, definitely increase it there too. Maybe frog.py as well.