Add additional runtime type checks.

Add additional runtime type checks.

[jat, 2011-10-25 20:39:28]

In addition to adding bool type checks, this fixes
http://code.google.com/p/dart/issues/detail?id=6

[fabiomfv, 2011-10-26 14:43:25]

LGTM. few comments (inline).

could run benchmarks in optimized to double check none of the additional checks
'escapes'?

I am assuming this is a surgical change to address bool type and void, function
will be addressed in the future.

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

http://codereview.chromium.org/8393015/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java:1674:
JsExpression expr = rtt.addTypeCheck(getCurrentClass(), jsParam, paramType,
null, param);
assuming this change is basically refactoring?

http://codereview.chromium.org/8393015/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java:2284:
case NOT:
we probably dont need NOT since we dont call getRequiredType() from visit
unaryexpr.

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

http://codereview.chromium.org/8393015/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/RuntimeTypeInjector.java:944:
// TODO: do we need a more detailed RTT than just a generic function?
I wonder if we should use the generic function. what will happen when one has
'String f()' and pass 'int g()' for instance?

[jat, 2011-10-31 20:16:14]

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

http://codereview.chromium.org/8393015/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java:1674:
JsExpression expr = rtt.addTypeCheck(getCurrentClass(), jsParam, paramType,
null, param);
On 2011/10/26 14:43:25, fabiomfv wrote:
> assuming this change is basically refactoring?

No, this is what actually fixes issue 6.  The problem is that the parameter's
type node has the return type of a function type, but if I go through the symbol
first I get the correct type.  This is another example of what I have mentioned
before, where type is stored in several places and it isn't clear which one is
the right one to use (or even which one will actually be populated in which
circumstances).

http://codereview.chromium.org/8393015/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java:2284:
case NOT:
On 2011/10/26 14:43:25, fabiomfv wrote:
> we probably dont need NOT since we dont call getRequiredType() from visit
> unaryexpr.

Done.

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

http://codereview.chromium.org/8393015/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/RuntimeTypeInjector.java:944:
// TODO: do we need a more detailed RTT than just a generic function?
On 2011/10/26 14:43:25, fabiomfv wrote:
> I wonder if we should use the generic function. what will happen when one has
> 'String f()' and pass 'int g()' for instance?

Currently, it won't be caught.

However, adding more detailed RTT will be a lot of work which realistically
isn't going to happen at this point.  Still, it is better to catch passing a
non-function when a function is expected and vice-versa.