Operators.

Implement support for user-defined operators.
Committed: https://code.google.com/p/dart/source/detail?r=3646

[Lasse Reichstein Nielsen, 2012-01-27 09:42:34]

LGTM

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/lib/core.dart
File frog/leg/lib/core.dart (right):

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/lib/core.da...
frog/leg/lib/core.dart:116: if (JS("bool", @"$0.operator$eq$1", a)) {
I recommend doing the foreign PREOPRTY_CHECK macro now.
Having a literal pre-mangled name is fragile.

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/lib/core.da...
frog/leg/lib/core.dart:132: if (JS("bool", @"$0.operator$eq$1", a)) {
Ditto here.

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/lib/core.da...
frog/leg/lib/core.dart:395: static void isTrue(var actual) {
Add a TODO to add the optional message parameter when we support optional
parameters.

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/resolver.dart
File frog/leg/resolver.dart (right):

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/resolver.da...
frog/leg/resolver.dart:653: constructorName =
Elements.constructConstructorName(className, name);
You are mangling the name here, right?
How about not mangling it until codegen?

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/scanner/par...
File frog/leg/scanner/parser.dart (right):

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/scanner/par...
frog/leg/scanner/parser.dart:571: } else if (isIdentifier(peek)) {
You could move the test for 'operator' to after the isIdentifier test here.

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

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/ssa/builder...
frog/leg/ssa/builder.dart:1083: assert(op.source == const SourceString("="));
Might as well do 
  op.source.stringValue == "="
(or even ===).

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/ssa/builder...
frog/leg/ssa/builder.dart:1100: bool isPrefix = !node.isPostfix;  // Compound
assignments are prefix.
"are prefix." => "are considered as being prefix."
They *are* infix.

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/ssa/builder...
frog/leg/ssa/builder.dart:1126: } else if (const SourceString("=") == op.source
&& !node.isIndex) {
Change that to
 assert(!node.isIndex);
It can't happen with this being an else branch of an if with condition
(node.isIndex).

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/tests/leg/src/R...
File frog/tests/leg/src/ResolverTest.dart (right):

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/tests/leg/src/R...
frog/tests/leg/src/ResolverTest.dart:520: resolveConstructor(script, "A a = new
A.a();", "A", "A\$a", 1,
Consider using raw strings.

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/tests/leg_only/...
File frog/tests/leg_only/src/CompoundOperatorIndexTest.dart (right):

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/tests/leg_only/...
frog/tests/leg_only/src/CompoundOperatorIndexTest.dart:29: a[0] += 2;
Try testing that the object and operand are only evaluated once, and that we
only call the [] and []= operators once each, and in the correct order.

Something like:

List<int> trace = <int>[];
class B {
  int value = 100;
  operator [](index) { 
    trace.add(-3); 
    trace.add(index); 
    trace.add(this.value);
    this.value = this.value + 1; 
    return this; 
  }
  operator []=(index, value) { 
    trace.add(-5);
    trace.add(index);
    trace.add(value.value);
    this.value = this.value + 1;
  }
  operator +(int value) {
    trace.add(-4);
    trace.add(this.value);
    trace.add(value);
    this.value = this.value + 1;
    return this;
  }
}

var b = new B();
B getB() {
  trace.add(-1);
  return new B();
}
int getIndex() {
  trace.add(-2);
  return 42;
}
getB()[getIndex()] += 37;

and then check that trace is 
 [-1, -2, -3, 42, 100, -4, 101, 37, -5, 42, 102]

[ngeoffray, 2012-01-27 10:50:04]

Thank you Lasse! And great test :)

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/lib/core.dart
File frog/leg/lib/core.dart (right):

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/lib/core.da...
frog/leg/lib/core.dart:116: if (JS("bool", @"$0.operator$eq$1", a)) {
On 2012/01/27 09:42:34, Lasse Reichstein Nielsen wrote:
> I recommend doing the foreign PREOPRTY_CHECK macro now.
> Having a literal pre-mangled name is fragile.

As discussed, added a JS_HAS_EQUALS to avoid the mangled hardcoding.

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/lib/core.da...
frog/leg/lib/core.dart:132: if (JS("bool", @"$0.operator$eq$1", a)) {
On 2012/01/27 09:42:34, Lasse Reichstein Nielsen wrote:
> Ditto here.

Done.

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/lib/core.da...
frog/leg/lib/core.dart:395: static void isTrue(var actual) {
On 2012/01/27 09:42:34, Lasse Reichstein Nielsen wrote:
> Add a TODO to add the optional message parameter when we support optional
> parameters.

Done.

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/resolver.dart
File frog/leg/resolver.dart (right):

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/resolver.da...
frog/leg/resolver.dart:653: constructorName =
Elements.constructConstructorName(className, name);
On 2012/01/27 09:42:34, Lasse Reichstein Nielsen wrote:
> You are mangling the name here, right?
> How about not mangling it until codegen?

No I'm not mangling it. I'm making it a single SourceString, because that's the
keys for our maps.

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/scanner/par...
File frog/leg/scanner/parser.dart (right):

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/scanner/par...
frog/leg/scanner/parser.dart:571: } else if (isIdentifier(peek)) {
On 2012/01/27 09:42:34, Lasse Reichstein Nielsen wrote:
> You could move the test for 'operator' to after the isIdentifier test here.

Code has changed since after Peter's recent changes on getset.

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

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/ssa/builder...
frog/leg/ssa/builder.dart:1083: assert(op.source == const SourceString("="));
On 2012/01/27 09:42:34, Lasse Reichstein Nielsen wrote:
> Might as well do 
>   op.source.stringValue == "="
> (or even ===).

Done.

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/ssa/builder...
frog/leg/ssa/builder.dart:1100: bool isPrefix = !node.isPostfix;  // Compound
assignments are prefix.
On 2012/01/27 09:42:34, Lasse Reichstein Nielsen wrote:
> "are prefix." => "are considered as being prefix."
> They *are* infix.

Done.

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/leg/ssa/builder...
frog/leg/ssa/builder.dart:1126: } else if (const SourceString("=") == op.source
&& !node.isIndex) {
On 2012/01/27 09:42:34, Lasse Reichstein Nielsen wrote:
> Change that to
>  assert(!node.isIndex);
> It can't happen with this being an else branch of an if with condition
> (node.isIndex).

You're right, that's a leftover from a previous implementation. Removed (and I
did not add an assert since the first if (node.isIndex) is quite trivial to
read).

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/tests/leg/src/R...
File frog/tests/leg/src/ResolverTest.dart (right):

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/tests/leg/src/R...
frog/tests/leg/src/ResolverTest.dart:520: resolveConstructor(script, "A a = new
A.a();", "A", "A\$a", 1,
On 2012/01/27 09:42:34, Lasse Reichstein Nielsen wrote:
> Consider using raw strings.

Done.

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/tests/leg_only/...
File frog/tests/leg_only/src/CompoundOperatorIndexTest.dart (right):

https://chromiumcodereview.appspot.com/9284022/diff/4008/frog/tests/leg_only/...
frog/tests/leg_only/src/CompoundOperatorIndexTest.dart:29: a[0] += 2;
On 2012/01/27 09:42:34, Lasse Reichstein Nielsen wrote:
> Try testing that the object and operand are only evaluated once, and that we
> only call the [] and []= operators once each, and in the correct order.
> 
> Something like:
> 
> List<int> trace = <int>[];
> class B {
>   int value = 100;
>   operator [](index) { 
>     trace.add(-3); 
>     trace.add(index); 
>     trace.add(this.value);
>     this.value = this.value + 1; 
>     return this; 
>   }
>   operator []=(index, value) { 
>     trace.add(-5);
>     trace.add(index);
>     trace.add(value.value);
>     this.value = this.value + 1;
>   }
>   operator +(int value) {
>     trace.add(-4);
>     trace.add(this.value);
>     trace.add(value);
>     this.value = this.value + 1;
>     return this;
>   }
> }
> 
> var b = new B();
> B getB() {
>   trace.add(-1);
>   return new B();
> }
> int getIndex() {
>   trace.add(-2);
>   return 42;
> }
> getB()[getIndex()] += 37;
> 
> and then check that trace is 
>  [-1, -2, -3, 42, 100, -4, 101, 37, -5, 42, 102]

Done, with some little tweaks :)