Fix for issue 2351 - checking assignability on top level getters and setters

Fix for issue 2351 - checking assignability on top level getters and setters

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

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

[Brian Wilkerson, 2012-06-12 16:53:16]

http://codereview.chromium.org/10548002/diff/2001/compiler/java/com/google/da...
File compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java (right):

http://codereview.chromium.org/10548002/diff/2001/compiler/java/com/google/da...
compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java:1436:
//node.setElement(getter);
It isn't clear to me why we would want to stop associating the getter with the
node (or the setter a few lines down). Ideally, every identifier would be
associated with an element unless the name was undefined.

http://codereview.chromium.org/10548002/diff/2001/compiler/java/com/google/da...
compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java:1451: case METHOD:
nit: trailing whitespace

[zundel, 2012-06-12 17:07:45]

PTAL

http://codereview.chromium.org/10548002/diff/2001/compiler/java/com/google/da...
File compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java (right):

http://codereview.chromium.org/10548002/diff/2001/compiler/java/com/google/da...
compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java:1436:
//node.setElement(getter);
On 2012/06/12 16:53:16, Brian Wilkerson wrote:
> It isn't clear to me why we would want to stop associating the getter with the
> node (or the setter a few lines down). Ideally, every identifier would be
> associated with an element unless the name was undefined.

Because getters are method types. the next time you walk the tree, instead of
looking at the value as a field, you think its a method.  When I took of the 'is
top level' constraint, it ended up blowing up several tests.  Now the code is
consistent and works on top level and non top-level types.

It turns out that setting the element didn't help all that much for the switch
expression issue.  The normal way to get the type of an element:
getElement().getType() wouldn't work with a method element.   Konstantin just
added a 'node.setType()' when I merged up.

[zundel, 2012-06-12 17:09:08]

http://codereview.chromium.org/10548002/diff/7001/compiler/java/com/google/da...
File compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java (right):

http://codereview.chromium.org/10548002/diff/7001/compiler/java/com/google/da...
compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java:1443:
node.setType(voidType);
Not sure about this line... bringing in Konstantin

[Brian Wilkerson, 2012-06-12 17:35:14]

LGTM

[scheglov, 2012-06-12 19:39:26]

lgtm

http://codereview.chromium.org/10548002/diff/7001/compiler/java/com/google/da...
File compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java (right):

http://codereview.chromium.org/10548002/diff/7001/compiler/java/com/google/da...
compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java:1443:
node.setType(voidType);
On 2012/06/12 17:09:08, zundel wrote:
> Not sure about this line... bringing in Konstantin

Hm...
Yes, this line is not required.