Support named arguments for statically resolved calls.

Support named arguments for statically resolved calls.
Committed: https://code.google.com/p/dart/source/detail?r=3735

[ngeoffray, 2012-01-31 14:11:07]

Because builder/emitter/name treat very differently statically resolved
invocations compared to dynamic invocations, it felt easier to start with
supporting statically resolved calls first.

In this CL, I don't change the name of a static method, but I make sure that the
caller puts the right number of arguments, and at their expected location.

[Lasse Reichstein Nielsen, 2012-01-31 15:36:05]

Drive-by comments.

https://chromiumcodereview.appspot.com/9301038/diff/4002/frog/leg/universe.dart
File frog/leg/universe.dart (right):

https://chromiumcodereview.appspot.com/9301038/diff/4002/frog/leg/universe.da...
frog/leg/universe.dart:114: }
Remove the name from the set, so you don't allow duplicate named arguments.

https://chromiumcodereview.appspot.com/9301038/diff/4002/frog/tests/leg_only/...
File frog/tests/leg_only/src/NamedParameterForStaticTest.dart (right):

https://chromiumcodereview.appspot.com/9301038/diff/4002/frog/tests/leg_only/...
frog/tests/leg_only/src/NamedParameterForStaticTest.dart:22:
oneOptionalArgument(1, b: 2);
How about adding a negative test (or more) that tests invalid parameters. The
usual:
  twoOptionalArguments(1, a:2);
  twoOptionalArguments(1, 2, a: 1);
  twoOptionalArguments(a:1, a:1, b:2);
  etc.
(Maybe if you can catch a NoSuchMethodException, you can do it in this file
too).

[floitsch, 2012-01-31 15:52:41]

LGTM. (after Lasse's fix).

https://chromiumcodereview.appspot.com/9301038/diff/4002/frog/leg/ssa/builder...
File frog/leg/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9301038/diff/4002/frog/leg/ssa/builder...
frog/leg/ssa/builder.dart:954: // TODO(ngeoffray): Add the default value.
If it is not too difficult I would prefer checking if there actually is a
default value. Bailing out is still fine, though.
If it is difficult, don't worry.

https://chromiumcodereview.appspot.com/9301038/diff/4002/frog/leg/universe.dart
File frog/leg/universe.dart (right):

https://chromiumcodereview.appspot.com/9301038/diff/4002/frog/leg/universe.da...
frog/leg/universe.dart:96: argumentCount <= requiredParameterCount +
optionalParameterCount;
indent by 4?

[ngeoffray, 2012-01-31 16:46:16]

Thanks for the comments!

https://chromiumcodereview.appspot.com/9301038/diff/4002/frog/leg/ssa/builder...
File frog/leg/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9301038/diff/4002/frog/leg/ssa/builder...
frog/leg/ssa/builder.dart:954: // TODO(ngeoffray): Add the default value.
On 2012/01/31 15:52:41, floitsch wrote:
> If it is not too difficult I would prefer checking if there actually is a
> default value. Bailing out is still fine, though.
> If it is difficult, don't worry.

We don't support default values for parameters yet. We cancel early in the
pipeline.

https://chromiumcodereview.appspot.com/9301038/diff/4002/frog/leg/universe.dart
File frog/leg/universe.dart (right):

https://chromiumcodereview.appspot.com/9301038/diff/4002/frog/leg/universe.da...
frog/leg/universe.dart:96: argumentCount <= requiredParameterCount +
optionalParameterCount;
On 2012/01/31 15:52:41, floitsch wrote:
> indent by 4?

Done.

https://chromiumcodereview.appspot.com/9301038/diff/4002/frog/leg/universe.da...
frog/leg/universe.dart:114: }
On 2012/01/31 15:36:05, Lasse Reichstein Nielsen wrote:
> Remove the name from the set, so you don't allow duplicate named arguments.

Done. I expect another phase to check that though.

https://chromiumcodereview.appspot.com/9301038/diff/4002/frog/tests/leg_only/...
File frog/tests/leg_only/src/NamedParameterForStaticTest.dart (right):

https://chromiumcodereview.appspot.com/9301038/diff/4002/frog/tests/leg_only/...
frog/tests/leg_only/src/NamedParameterForStaticTest.dart:22:
oneOptionalArgument(1, b: 2);
On 2012/01/31 15:36:05, Lasse Reichstein Nielsen wrote:
> How about adding a negative test (or more) that tests invalid parameters. The
> usual:
>   twoOptionalArguments(1, a:2);
>   twoOptionalArguments(1, 2, a: 1);
>   twoOptionalArguments(a:1, a:1, b:2);
>   etc.
> (Maybe if you can catch a NoSuchMethodException, you can do it in this file
> too).

(We don't have try/catch :))

I have put a cancel in the Ssa builder if the call site does not match the
signature of the statically resolved method. We still need to decide what should
we do in this situation. Not generate code at all, or emit a throw in place of
the call.

Note that NoSuchMethodException only is for instance methods, not
static/top-level methods.