Allow for instance checking of methods and function aliases.

Allow for instance checking of methods and function aliases.

Note: Please beat this one up.  It doesn't pass all the tests (maps) yet,
but if you see something feel free to comment.

BUG=
TEST=

[zundel, 2011-11-08 15:28:42]

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
File
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java
(right):

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java:189:
public Void visitFunctionTypeAlias(DartFunctionTypeAlias node) {
you could set the return value to Boolean instead of keeping an instance
variable.

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java:192:
}
missing n/l

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java:196:
aliasFound = true;
Efficiency: if you find an alias, do you really want to continue the traversal?

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java:1129:
Element element = x.getSymbol();
I think you can be pretty sure that element is of kind ElementKind.METHOD
because getSymbol() is typed as MethodElement.

But, if you still want to do it, it is common to test with
 
ElementKind.of(element) == ElementKind.METHOD so that you catch the null case.

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java:3565:
// TODO: If we're never in an instanceOf check, we don't need to generate this
I'm not sure what you mean by instanceOf, do you mean the Dart 'is' keyword?

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java:3567:
JsName classJsName = getJsName(node.getSymbol());
node.getSymbol() == classElement

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java:3664:
mangler.mangleLookupMethod(methodElement.getName(), unitLibrary));
out of curiousity, why didn't you use the overload
mangleLookupMethod(MethodElement, LibraryElement)?

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java:3734:
if (node.getParent().getParent() instanceof DartBinaryExpression) {
Not sure what's going on here - could you add some kind of comment?

On the face of it, getParent().getParent() looks dangerous - is it guaranteed
not to be null?  (what if its a top level expression?) Why do it twice - you
could save it in a temp variable.

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/backend/js/JsNameProvider.java
(right):

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/JsNameProvider.java:64: //   
assert ElementKind.of(symbol).equals(ElementKind.CLASS)
Are you now allowing FUNCTION kind through?  Maybe you should just expand the
assertion to include CLASS and FUNCTION

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/backend/js/RuntimeTypeInjector.java
(right):

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/RuntimeTypeInjector.java:246:
boolean hasTypeParams = hasTypeParameters(classElement);
looks like you aren't using hasTypeParams() anymore.

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/RuntimeTypeInjector.java:472:
elementExpr = generateRTTLookup((ClassElement) elementType.getElement(),
(InterfaceType) elementType, classElement);
this and other lines > 100chars

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/resolver/ResolveVisitor.java
(right):

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/resolver/ResolveVisitor.java:91: for
(DartTypeNode typeNode : returnType.getTypeArguments()) {
It looks to me like these two sections of this method are essentially the same
and could be refactored out into one method to set the bounds for return type
and type parameters for the parameters.

http://codereview.chromium.org/8479049/diff/1/compiler/lib/implementation/rtt.js
File compiler/lib/implementation/rtt.js (right):

http://codereview.chromium.org/8479049/diff/1/compiler/lib/implementation/rtt...
compiler/lib/implementation/rtt.js:104: for(var i = this.typeArgs.length - 1; i
>= 0; i--) {
you are re-using 'i' here, its already used as an iterator in the outer loop.

http://codereview.chromium.org/8479049/diff/1/compiler/lib/implementation/rtt...
compiler/lib/implementation/rtt.js:107: break; // fall through and search the
next property
So, one of the type args doesn't match.  From what I can see, its just going to
skip over this type argument.  If the second type argument matches, and the
first one doesn't, the type is deemed to match?

[codefu, 2011-11-14 19:33:40]

Thanks for the comments.  This turned out to be much bigger of an issue and I
decided to incorporate your comments and roll a new CL.

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
File
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java
(right):

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java:189:
public Void visitFunctionTypeAlias(DartFunctionTypeAlias node) {
On 2011/11/08 15:28:42, zundel wrote:
> you could set the return value to Boolean instead of keeping an instance
> variable.

I'll have to override some other methods that by default return null.  One other
cleanup is to move the test logic into a "isFunctionAlias(DartExpression)"
utility.

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java:192:
}
On 2011/11/08 15:28:42, zundel wrote:
> missing n/l

Done.

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java:196:
aliasFound = true;
On 2011/11/08 15:28:42, zundel wrote:
> Efficiency: if you find an alias, do you really want to continue the
traversal?

Done.

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java:1129:
Element element = x.getSymbol();
On 2011/11/08 15:28:42, zundel wrote:
> I think you can be pretty sure that element is of kind ElementKind.METHOD
> because getSymbol() is typed as MethodElement.
> 
> But, if you still want to do it, it is common to test with
>  
> ElementKind.of(element) == ElementKind.METHOD so that you catch the null case.

Done.

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java:3565:
// TODO: If we're never in an instanceOf check, we don't need to generate this
On 2011/11/08 15:28:42, zundel wrote:
> I'm not sure what you mean by instanceOf, do you mean the Dart 'is' keyword?

Yep.  I was using the function name from rtt.generateInstanceOfComparison().  I
will update the comment to make this clearer.

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java:3567:
JsName classJsName = getJsName(node.getSymbol());
On 2011/11/08 15:28:42, zundel wrote:
> node.getSymbol() == classElement

Done.

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java:3664:
mangler.mangleLookupMethod(methodElement.getName(), unitLibrary));
On 2011/11/08 15:28:42, zundel wrote:
> out of curiousity, why didn't you use the overload
> mangleLookupMethod(MethodElement, LibraryElement)?

Done.

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/backend/js/JsNameProvider.java
(right):

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/JsNameProvider.java:64: //   
assert ElementKind.of(symbol).equals(ElementKind.CLASS)
On 2011/11/08 15:28:42, zundel wrote:
> Are you now allowing FUNCTION kind through?  Maybe you should just expand the
> assertion to include CLASS and FUNCTION

Done.

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/backend/js/RuntimeTypeInjector.java
(right):

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/RuntimeTypeInjector.java:246:
boolean hasTypeParams = hasTypeParameters(classElement);
On 2011/11/08 15:28:42, zundel wrote:
> looks like you aren't using hasTypeParams() anymore.

For now, yes.  I need to look into a generic rtt creation method for functions
and not rely on the class's method.

http://codereview.chromium.org/8479049/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/RuntimeTypeInjector.java:472:
elementExpr = generateRTTLookup((ClassElement) elementType.getElement(),
(InterfaceType) elementType, classElement);
On 2011/11/08 15:28:42, zundel wrote:
> this and other lines > 100chars

Done.

http://codereview.chromium.org/8479049/diff/1/compiler/lib/implementation/rtt.js
File compiler/lib/implementation/rtt.js (right):

http://codereview.chromium.org/8479049/diff/1/compiler/lib/implementation/rtt...
compiler/lib/implementation/rtt.js:104: for(var i = this.typeArgs.length - 1; i
>= 0; i--) {
On 2011/11/08 15:28:42, zundel wrote:
> you are re-using 'i' here, its already used as an iterator in the outer loop.

Done.

http://codereview.chromium.org/8479049/diff/1/compiler/lib/implementation/rtt...
compiler/lib/implementation/rtt.js:107: break; // fall through and search the
next property
On 2011/11/08 15:28:42, zundel wrote:
> So, one of the type args doesn't match.  From what I can see, its just going
to
> skip over this type argument.  If the second type argument matches, and the
> first one doesn't, the type is deemed to match?

I've changed this around a little in the new patch I'm preparing to upload. 
There should only be one typeargs for a function, so it would break out of the
loop and there would be no others.  'This' should be the RHS of an 'is'
expression, otherType is whatever is returned through a series of RTT lookups.