Report arity errors on user definable operators.

Report arity errors on user definable operators.


BUG=http://dartbug/7149

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

[ahe, 2013-01-09 10:17:52]

LGTM!

https://codereview.chromium.org/11778039/diff/1001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/11778039/diff/1001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:601: if
(identical(value, 'unary-')) {
Consider this approach:

int requiredParameterCount;
if (identical(value, 'unary-')) {
  requiredParameterCount = 0;
} else if (isMinusOperator(value) {
  requiredParameterCount = 1;
} ... {
  internalErrorOnElement(function);
}
checkArity(function, requiredParameterCount, 0, 0);

We should be able to reuse the checkArity method with other code and generate
really nice messages, such as:

Error: extra parameter "y". "+" must have exactly 1 parameter.
operator+(x, y) { ... }
             ^

Error: missing parameter. "+" must have exactly 1 parameter.
operator+() { ... }
         ^^

https://codereview.chromium.org/11778039/diff/1001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:602: if
(signature.requiredParameterCount != 0) {
You should be able to assert this.

https://codereview.chromium.org/11778039/diff/1001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:636: }
else {
  // Internal error.
}

https://codereview.chromium.org/11778039/diff/1001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:640:
compiler.spanFromElement(function),
There is a more accurate position for the optional parameters.

https://codereview.chromium.org/11778039/diff/1001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:645:
compiler.spanFromElement(function),
Ditto.

[Johnni Winther, 2013-01-09 13:00:47]

https://codereview.chromium.org/11778039/diff/1001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/11778039/diff/1001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:601: if
(identical(value, 'unary-')) {
On 2013/01/09 10:17:52, ahe wrote:
> Consider this approach:
> 
> int requiredParameterCount;
> if (identical(value, 'unary-')) {
>   requiredParameterCount = 0;
> } else if (isMinusOperator(value) {
>   requiredParameterCount = 1;
> } ... {
>   internalErrorOnElement(function);
> }
> checkArity(function, requiredParameterCount, 0, 0);
> 
> We should be able to reuse the checkArity method with other code and generate
> really nice messages, such as:
> 
> Error: extra parameter "y". "+" must have exactly 1 parameter.
> operator+(x, y) { ... }
>              ^
> 
> Error: missing parameter. "+" must have exactly 1 parameter.
> operator+() { ... }
>          ^^

Done.

https://codereview.chromium.org/11778039/diff/1001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:636: }
On 2013/01/09 10:17:52, ahe wrote:
> else {
>   // Internal error.
> }

Done.

https://codereview.chromium.org/11778039/diff/1001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:640:
compiler.spanFromElement(function),
On 2013/01/09 10:17:52, ahe wrote:
> There is a more accurate position for the optional parameters.

Done.

https://codereview.chromium.org/11778039/diff/1001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:645:
compiler.spanFromElement(function),
On 2013/01/09 10:17:52, ahe wrote:
> Ditto.

Done.

[ahe, 2013-01-09 13:35:46]

SLGTM

https://codereview.chromium.org/11778039/diff/6002/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/11778039/diff/6002/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:518:
MessageKind.ILLEGAL_CONSTRUCTOR_MODIFIERS.error([mismatchedFlags]),
Strange newline in review tool here.

https://codereview.chromium.org/11778039/diff/6002/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:661: 
Extra line.

https://codereview.chromium.org/11778039/diff/6002/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/util/link.dart (right):

https://codereview.chromium.org/11778039/diff/6002/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/util/link.dart:52: Link<T> skip(int n)
{
if (n == 0) return this;

https://codereview.chromium.org/11778039/diff/6002/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/util/link_implementation.dart
(right):

https://codereview.chromium.org/11778039/diff/6002/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/util/link_implementation.dart:73:
return tail.skip(n-1);
How about a for loop?

[Johnni Winther, 2013-01-09 14:29:11]

https://codereview.chromium.org/11778039/diff/6002/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/11778039/diff/6002/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:518:
MessageKind.ILLEGAL_CONSTRUCTOR_MODIFIERS.error([mismatchedFlags]),
On 2013/01/09 13:35:46, ahe wrote:
> Strange newline in review tool here.

Done.

https://codereview.chromium.org/11778039/diff/6002/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:661: 
On 2013/01/09 13:35:46, ahe wrote:
> Extra line.

Done.

https://codereview.chromium.org/11778039/diff/6002/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/util/link.dart (right):

https://codereview.chromium.org/11778039/diff/6002/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/util/link.dart:52: Link<T> skip(int n)
{
On 2013/01/09 13:35:46, ahe wrote:
> if (n == 0) return this;

Done.

https://codereview.chromium.org/11778039/diff/6002/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/util/link_implementation.dart
(right):

https://codereview.chromium.org/11778039/diff/6002/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/util/link_implementation.dart:73:
return tail.skip(n-1);
On 2013/01/09 13:35:46, ahe wrote:
> How about a for loop?

Done.