Better runtime type checks.

Better runtime type checks. The generated code is nicer+faster and they're generated in more places. Since it's reasonably fast now, I enabled it in presubmit.py.
TBR=jimhug

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

[Jennifer Messerly, 2011-11-05 00:45:10]

submitted

http://codereview.chromium.org/8457007/diff/4001/frog/library.dart
File frog/library.dart (right):

http://codereview.chromium.org/8457007/diff/4001/frog/library.dart#newcode117
frog/library.dart:117: // Function types implement the Function interface.
feedback from a previous CR

http://codereview.chromium.org/8457007/diff/4001/frog/member.dart
File frog/member.dart (right):

http://codereview.chromium.org/8457007/diff/4001/frog/member.dart#newcode1479
frog/member.dart:1479: void forEach(void f(Member member)) {
This was a run time type error: we are passing a ConcreteMember to it.
Interestingly, the VM doesn't flag this one as an error at runtime.

http://codereview.chromium.org/8457007/diff/4001/frog/parser.dart
File frog/parser.dart (right):

http://codereview.chromium.org/8457007/diff/4001/frog/parser.dart#newcode948
frog/parser.dart:948: _intTypeRef(SourceSpan span) {
I started trying to make sense of ints vs doubles, but after fixing the parser
side I discovered a lot more issues. For now, there's a TODO in Value.convertTo
that ignores the distinction. It's not too far from working--but we need to
remove the various TODOs throughout the codebase that treat all nums the same.

http://codereview.chromium.org/8457007/diff/4001/frog/presubmit.py
File frog/presubmit.py (right):

http://codereview.chromium.org/8457007/diff/4001/frog/presubmit.py#newcode70
frog/presubmit.py:70: # Selfhost Checked
The speed/size is reasonable enough, so I just turned on --enable_type_checks.

[jimhug, 2011-11-07 16:41:26]

LGTM!

We still got some big issues (like how to implement the checks on a List that is
passed to a method expecting a List<String>) but this is already starting to be
useful and to give me hope that we can keep the costs of these checks reasonably
small.

http://codereview.chromium.org/8457007/diff/2002/frog/frog.dart
File frog/frog.dart (right):

http://codereview.chromium.org/8457007/diff/2002/frog/frog.dart#newcode16
frog/frog.dart:16: var argv = new List.from(process.argv);
TODO: This seems like a bug in the node interop that this would be needed here. 
I think any such shenanigans should be nicely encapsulated in whatever wrappers
we use to expose node.

http://codereview.chromium.org/8457007/diff/2002/frog/lib/core.js
File frog/lib/core.js (right):

http://codereview.chromium.org/8457007/diff/2002/frog/lib/core.js#newcode181
frog/lib/core.js:181: return test === true;
I wonder if this would be better?

(possibly inlined at call sites?)

return test === true ? true : test === false ? false : $bool_error();

It seems like this could also be further optimized for if/else branches by using
the if (test === true) ... else { assert(test === false); ...} pattern.

http://codereview.chromium.org/8457007/diff/2002/frog/member.dart
File frog/member.dart (right):

http://codereview.chromium.org/8457007/diff/2002/frog/member.dart#newcode693
frog/member.dart:693: // TODO(jmesserly): I don't like how this is coupled to
invoke
I agree that this is starting to feel nasty - along with the
needsConversion/convertTo coupling.  Let's see if we can clean this up sometime
this week - after a little bit more thought.

http://codereview.chromium.org/8457007/diff/2002/frog/value.dart
File frog/value.dart (right):

http://codereview.chromium.org/8457007/diff/2002/frog/value.dart#newcode162
frog/value.dart:162: // dynamic method calls. What should we do?
I believe that in checked mode we will have to generate "checking stubs" for
functions that could be used in this situation.  These stubs would perform any
needed type checks and forward on to the actual funciton.

http://codereview.chromium.org/8457007/diff/2002/frog/world.dart
File frog/world.dart (right):

http://codereview.chromium.org/8457007/diff/2002/frog/world.dart#newcode95
frog/world.dart:95: Type intType;
Darn.  I was hoping we could pretend these just didn't exist <smile?>

[Jennifer Messerly, 2011-11-07 22:13:10]

http://codereview.chromium.org/8457007/diff/2002/frog/frog.dart
File frog/frog.dart (right):

http://codereview.chromium.org/8457007/diff/2002/frog/frog.dart#newcode16
frog/frog.dart:16: var argv = new List.from(process.argv);
On 2011/11/07 16:41:26, jimhug wrote:
> TODO: This seems like a bug in the node interop that this would be needed
here. 
> I think any such shenanigans should be nicely encapsulated in whatever
wrappers
> we use to expose node.

It's a bug in our createSandbox. We pass certain objects (require, process,
console) to the child. These objects (by design of Node's sandbox) don't share
prototype with the parent. That part is good. The bad is that we are forcing the
child code to deal with the problem. The only fix I can think of is injecting
some code into the generate JS that patches the prototypes of those 3 objects,
as well as process.argv.

[Jennifer Messerly, 2011-11-08 03:56:16]

http://codereview.chromium.org/8457007/diff/2002/frog/lib/core.js
File frog/lib/core.js (right):

http://codereview.chromium.org/8457007/diff/2002/frog/lib/core.js#newcode181
frog/lib/core.js:181: return test === true;
On 2011/11/07 16:41:26, jimhug wrote:
> I wonder if this would be better?
> 
> (possibly inlined at call sites?)
> 
> return test === true ? true : test === false ? false : $bool_error();
> 
> It seems like this could also be further optimized for if/else branches by
using
> the if (test === true) ... else { assert(test === false); ...} pattern.

I was mostly trying to keep checked mode relatively readable, hence going
through the method. I agree it'd be good to try a few different patterns and see
if we can get it inlined.

However I suspect once we start tracking non-nullable bools (e.g. anything that
is a result of a non-overloaded == or != operator or && || ! operator) we will
be able to eliminate a lot of the checks.