Adds typechecking of return values

Adds typechecking of return values
Also fixes places in frog where we were returning things from "void" methods.

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

[Jennifer Messerly, 2011-11-11 02:36:01]

tbr. Pretty straightforward.

http://codereview.chromium.org/8534001/diff/2001/frog/lib/corelib_impl.dart
File frog/lib/corelib_impl.dart (right):

http://codereview.chromium.org/8534001/diff/2001/frog/lib/corelib_impl.dart#n...
frog/lib/corelib_impl.dart:238: const JSSyntaxRegExp(String pattern, [bool
multiLine, bool ignoreCase]):
this is an unrelated fix I noticed in co19 checked mode (which isn't running
yet, but anyway...)

"null" is allowed as the default arg & shouldn't cause a type error.

[Siggi Cherem (dart-lang), 2011-11-11 08:23:53]

http://codereview.chromium.org/8534001/diff/3010/frog/frogsh
File frog/frogsh (right):

http://codereview.chromium.org/8534001/diff/3010/frog/frogsh#newcode112
frog/frogsh:112: return typeof(test) == 'boolean' ? test : test.is$bool();
I couldn't find, where is 'is$bool()' generated?

http://codereview.chromium.org/8534001/diff/3010/frog/lib/corelib_impl.dart
File frog/lib/corelib_impl.dart (left):

http://codereview.chromium.org/8534001/diff/3010/frog/lib/corelib_impl.dart#o...
frog/lib/corelib_impl.dart:239: [bool multiLine = false, bool ignoreCase =
false]):
why is this change needed?

http://codereview.chromium.org/8534001/diff/3010/frog/value.dart
File frog/value.dart (right):

http://codereview.chromium.org/8534001/diff/3010/frog/value.dart#newcode44
frog/value.dart:44: String get canonicalCode() => null;
maybe this should throw an error in this case?

[jimhug, 2011-11-11 15:02:02]

LGTM!

One line change to enable return type checks - followed by a lot more changes to
fix type issues.  I like it!

http://codereview.chromium.org/8534001/diff/3010/frog/corejs.dart
File frog/corejs.dart (right):

http://codereview.chromium.org/8534001/diff/3010/frog/corejs.dart#newcode290
frog/corejs.dart:290: // Some testing showed that this patterned fared well
across browsers.
Thanks for the comment - I fear that we will have more of these heursitics in
the future and its good to at least have some information on which ones are wild
guesses and which ones had some level of testing behind them.

http://codereview.chromium.org/8534001/diff/3010/frog/gen.dart
File frog/gen.dart (right):

http://codereview.chromium.org/8534001/diff/3010/frog/gen.dart#newcode1107
frog/gen.dart:1107: // It can't issue a warning because every type is nullable.
Isn't there a TODO here to fix the language <smile>

http://codereview.chromium.org/8534001/diff/3010/frog/gen.dart#newcode1113
frog/gen.dart:1113: var value = visitTypedValue(node.value, method.returnType);
I love the fact that this is essentially your entire real change and all the
rest is just fixing errors caught by the better checking!

http://codereview.chromium.org/8534001/diff/3010/frog/lib/corelib_impl.dart
File frog/lib/corelib_impl.dart (right):

http://codereview.chromium.org/8534001/diff/3010/frog/lib/corelib_impl.dart#n...
frog/lib/corelib_impl.dart:239: this._create(pattern,
I agree with Siggi that the previous version of the code looks better to me - am
I missing something?

http://codereview.chromium.org/8534001/diff/3010/frog/type.dart
File frog/type.dart (right):

http://codereview.chromium.org/8534001/diff/3010/frog/type.dart#newcode313
frog/type.dart:313: Map<String, MethodMember> get constructors() {
Is this required or just good style?

http://codereview.chromium.org/8534001/diff/3010/frog/type.dart#newcode532
frog/type.dart:532: void set parent(Type p) { _parent = p; }
Same question here - can we use => on void methods?

http://codereview.chromium.org/8534001/diff/3010/frog/value.dart
File frog/value.dart (right):

http://codereview.chromium.org/8534001/diff/3010/frog/value.dart#newcode339
frog/value.dart:339: if (toType.typeCheckCode == null) {
Hmm, I'd rather not see this code sitting in value.dart.  I'll try to clean this
up with my next round of value changes - please let me do the cleanup here to
minimize conflicts <smile>.

http://codereview.chromium.org/8534001/diff/3010/tests/corelib/src/SortHelper...
File tests/corelib/src/SortHelper.dart (right):

http://codereview.chromium.org/8534001/diff/3010/tests/corelib/src/SortHelper...
tests/corelib/src/SortHelper.dart:23: bool isSorted(List a) {
Do you know why frog is the first one catching these type issues?  They look
pretty egregious to me.

http://codereview.chromium.org/8534001/diff/3010/tests/language/src/ContextAr...
File tests/language/src/ContextArgsWithDefaultsTest.dart (right):

http://codereview.chromium.org/8534001/diff/3010/tests/language/src/ContextAr...
tests/language/src/ContextArgsWithDefaultsTest.dart:10: static crasher(int
fixed, [String optional = '']) {
Oh my, that was an ugly test bug!

[Jennifer Messerly, 2011-11-11 18:23:55]

http://codereview.chromium.org/8534001/diff/3010/frog/lib/corelib_impl.dart
File frog/lib/corelib_impl.dart (right):

http://codereview.chromium.org/8534001/diff/3010/frog/lib/corelib_impl.dart#n...
frog/lib/corelib_impl.dart:239: this._create(pattern,
On 2011/11/11 15:02:02, jimhug wrote:
> I agree with Siggi that the previous version of the code looks better to me -
am
> I missing something?

I commented on this here:
http://codereview.chromium.org/8534001/diff/2001/frog/lib/corelib_impl.dart#n...

[Jennifer Messerly, 2011-11-11 18:38:05]

http://codereview.chromium.org/8534001/diff/3010/frog/type.dart
File frog/type.dart (right):

http://codereview.chromium.org/8534001/diff/3010/frog/type.dart#newcode313
frog/type.dart:313: Map<String, MethodMember> get constructors() {
On 2011/11/11 15:02:02, jimhug wrote:
> Is this required or just good style?

internalError has a return type of "void". If we changed its return type to
"var" I could keep the =>. Let me know which you prefer.

http://codereview.chromium.org/8534001/diff/3010/frog/type.dart#newcode532
frog/type.dart:532: void set parent(Type p) { _parent = p; }
On 2011/11/11 15:02:02, jimhug wrote:
> Same question here - can we use => on void methods?

Yup, you can use => on void methods if the RHS type is also void or var.

"_parent = p" returns "Type" which is not a subtype of "void". Looking at this
again though, maybe the bug is in the setter's return type--should it be Type?
Do we rely on setters returning the correct value to make things like "o.a = o.b
= 42" work?