DartType equals converted to operator == and tested.

DartType equals converted to operator == and tested.

equals method replaced by operator == in version 0.11. DartType and Link equals updated accordingly test added.

TEST=tests/compiler/dart2js/type_equals_test.dart

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

[ahe, 2012-09-17 08:19:58]

LGTM, but I'm a little concerned about the test.

https://codereview.chromium.org/10917290/diff/1/lib/compiler/implementation/t...
File lib/compiler/implementation/typechecker.dart (right):

https://codereview.chromium.org/10917290/diff/1/lib/compiler/implementation/t...
lib/compiler/implementation/typechecker.dart:60: return other.element ==
element;
===

https://codereview.chromium.org/10917290/diff/1/lib/compiler/implementation/t...
lib/compiler/implementation/typechecker.dart:147: if (element != other.element)
return false;
!==

https://codereview.chromium.org/10917290/diff/1/lib/compiler/implementation/t...
lib/compiler/implementation/typechecker.dart:235: if (element != other.element)
return false;
!==

https://codereview.chromium.org/10917290/diff/1/tests/compiler/dart2js/type_e...
File tests/compiler/dart2js/type_equals_test.dart (right):

https://codereview.chromium.org/10917290/diff/1/tests/compiler/dart2js/type_e...
tests/compiler/dart2js/type_equals_test.dart:13: Compiler compile(String script)
{
You'd probably be better off using compilerFor in compiler_helper.dart.

https://codereview.chromium.org/10917290/diff/1/tests/compiler/dart2js/type_e...
tests/compiler/dart2js/type_equals_test.dart:19: bool test(compiler, String
name1, String name2, [bool expect]) {
Should use {}, not [].

Since the argument is required, I think you should start the method saying
something like:

Expect.isTrue(?expect, 'required parameter "expect" not given');

https://codereview.chromium.org/10917290/diff/1/tests/compiler/dart2js/type_e...
tests/compiler/dart2js/type_equals_test.dart:20: var clazz =
You can use the word "class".

https://codereview.chromium.org/10917290/diff/1/tests/compiler/dart2js/type_e...
tests/compiler/dart2js/type_equals_test.dart:27: Expect.isTrue(element1 is
FunctionElement);
Test the kind instead, it is more precise.

Also, the is-check wouldn't be necessary if you declared element1 and element2
to have type FuncionType.

https://codereview.chromium.org/10917290/diff/1/tests/compiler/dart2js/type_e...
tests/compiler/dart2js/type_equals_test.dart:29: var signature1 =
element1.computeSignature(compiler);
Use a type here for improved readability.

https://codereview.chromium.org/10917290/diff/1/tests/compiler/dart2js/type_e...
tests/compiler/dart2js/type_equals_test.dart:32: var value1;
A type here would also improve. Especially since the names "value1" and "value2"
convey no information other than these variables hold values, not types.

https://codereview.chromium.org/10917290/diff/1/tests/compiler/dart2js/type_e...
tests/compiler/dart2js/type_equals_test.dart:48: }
I find this test confusing. It seems like you don't test all arguments.

[karlklose, 2012-09-17 10:28:36]

LGTM, thanks Johnni.

[Roman, 2012-09-17 16:53:36]

lgtm

I almost started fixing the same thing today as I stepped on consequences of
Link == and InterfaceType == not working properly. I.e. please submit this :)

[Johnni Winther, 2012-09-18 08:14:30]

https://codereview.chromium.org/10917290/diff/1/lib/compiler/implementation/t...
File lib/compiler/implementation/typechecker.dart (right):

https://codereview.chromium.org/10917290/diff/1/lib/compiler/implementation/t...
lib/compiler/implementation/typechecker.dart:60: return other.element ==
element;
On 2012/09/17 08:19:58, ahe wrote:
> ===

Done.

https://codereview.chromium.org/10917290/diff/1/lib/compiler/implementation/t...
lib/compiler/implementation/typechecker.dart:147: if (element != other.element)
return false;
On 2012/09/17 08:19:58, ahe wrote:
> !==

Done.

https://codereview.chromium.org/10917290/diff/1/lib/compiler/implementation/t...
lib/compiler/implementation/typechecker.dart:235: if (element != other.element)
return false;
On 2012/09/17 08:19:58, ahe wrote:
> !==

Done.

https://codereview.chromium.org/10917290/diff/1/tests/compiler/dart2js/type_e...
File tests/compiler/dart2js/type_equals_test.dart (right):

https://codereview.chromium.org/10917290/diff/1/tests/compiler/dart2js/type_e...
tests/compiler/dart2js/type_equals_test.dart:13: Compiler compile(String script)
{
On 2012/09/17 08:19:58, ahe wrote:
> You'd probably be better off using compilerFor in compiler_helper.dart.

Done.

https://codereview.chromium.org/10917290/diff/1/tests/compiler/dart2js/type_e...
tests/compiler/dart2js/type_equals_test.dart:19: bool test(compiler, String
name1, String name2, [bool expect]) {
On 2012/09/17 08:19:58, ahe wrote:
> Should use {}, not [].
> 
> Since the argument is required, I think you should start the method saying
> something like:
> 
> Expect.isTrue(?expect, 'required parameter "expect" not given');

Done.

https://codereview.chromium.org/10917290/diff/1/tests/compiler/dart2js/type_e...
tests/compiler/dart2js/type_equals_test.dart:20: var clazz =
On 2012/09/17 08:19:58, ahe wrote:
> You can use the word "class".

The VM complains.

https://codereview.chromium.org/10917290/diff/1/tests/compiler/dart2js/type_e...
tests/compiler/dart2js/type_equals_test.dart:27: Expect.isTrue(element1 is
FunctionElement);
On 2012/09/17 08:19:58, ahe wrote:
> Test the kind instead, it is more precise.
> 
> Also, the is-check wouldn't be necessary if you declared element1 and element2
> to have type FuncionType.

Done.

https://codereview.chromium.org/10917290/diff/1/tests/compiler/dart2js/type_e...
tests/compiler/dart2js/type_equals_test.dart:29: var signature1 =
element1.computeSignature(compiler);
On 2012/09/17 08:19:58, ahe wrote:
> Use a type here for improved readability.

Done.

https://codereview.chromium.org/10917290/diff/1/tests/compiler/dart2js/type_e...
tests/compiler/dart2js/type_equals_test.dart:32: var value1;
On 2012/09/17 08:19:58, ahe wrote:
> A type here would also improve. Especially since the names "value1" and
"value2"
> convey no information other than these variables hold values, not types.

Done.

https://codereview.chromium.org/10917290/diff/1/tests/compiler/dart2js/type_e...
tests/compiler/dart2js/type_equals_test.dart:48: }
On 2012/09/17 08:19:58, ahe wrote:
> I find this test confusing. It seems like you don't test all arguments.

Comments added as to why function signatures are used.