fix some errors in our SDK, mostly around numbers

fix some errors in our SDK, mostly around numbers, see issue #103
also improved codegen of JSNumber's methods

R=leafp@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/9baaa466b48180a74da839727c7f050ba8a05544

[Jennifer Messerly, 2015-09-18 21:26:31]

jmesserly@google.com changed reviewers:
+ leafp@google.com

[Leaf, 2015-09-18 22:37:49]

https://codereview.chromium.org/1348453004/diff/60001/lib/src/codegen/js_code...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1348453004/diff/60001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:321: // TODO(jmesserly): explicit check from num
to int or double to int?
I think this changes the semantics a little, since previously casting a
non-integral double to an int would have failed, whereas I think now this will
allow it to pass, yes?

https://codereview.chromium.org/1348453004/diff/60001/tool/input_sdk/private/...
File tool/input_sdk/private/js_number.dart (right):

https://codereview.chromium.org/1348453004/diff/60001/tool/input_sdk/private/...
tool/input_sdk/private/js_number.dart:246: if (other is !num) throw new
ArgumentError(other);
I don't know how much we want to assume DDC strong mode in our version of the
SDK, but I think these argument checks aren't needed for us.

[Jennifer Messerly, 2015-09-18 22:42:27]

https://codereview.chromium.org/1348453004/diff/60001/lib/src/codegen/js_code...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1348453004/diff/60001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:321: // TODO(jmesserly): explicit check from num
to int or double to int?
On 2015/09/18 22:37:49, Leaf wrote:
> I think this changes the semantics a little, since previously casting a
> non-integral double to an int would have failed, whereas I think now this will
> allow it to pass, yes?  

I think previously double -> int was a cast. However, num -> int was *not* a
cast. So it was kind of inconsistent :\

IMO, if we do want a check for int, we should special case it so it's not going
through `dart.as` ...

https://codereview.chromium.org/1348453004/diff/60001/tool/input_sdk/private/...
File tool/input_sdk/private/js_number.dart (right):

https://codereview.chromium.org/1348453004/diff/60001/tool/input_sdk/private/...
tool/input_sdk/private/js_number.dart:246: if (other is !num) throw new
ArgumentError(other);
On 2015/09/18 22:37:49, Leaf wrote:
> I don't know how much we want to assume DDC strong mode in our version of the
> SDK, but I think these argument checks aren't needed for us.

ah, good point. Interesting that isn't a hint (dead code) . I'll try and track
them down.

[Jennifer Messerly, 2015-09-18 22:44:36]

https://codereview.chromium.org/1348453004/diff/60001/lib/src/codegen/js_code...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1348453004/diff/60001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:321: // TODO(jmesserly): explicit check from num
to int or double to int?
On 2015/09/18 22:42:27, John Messerly wrote:
> On 2015/09/18 22:37:49, Leaf wrote:
> > I think this changes the semantics a little, since previously casting a
> > non-integral double to an int would have failed, whereas I think now this
will
> > allow it to pass, yes?  
> 
> I think previously double -> int was a cast. However, num -> int was *not* a
> cast. So it was kind of inconsistent :\
> 
> IMO, if we do want a check for int, we should special case it so it's not
going
> through `dart.as` ...

so from a type system perspective, if JSNumber can't be treated as implementing
`int` -- I think I'm back to my original problem of not being able to implement
JSNumber in a way that passes override checks ...

[Leaf, 2015-09-18 22:57:48]

https://codereview.chromium.org/1348453004/diff/60001/lib/src/codegen/js_code...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1348453004/diff/60001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:321: // TODO(jmesserly): explicit check from num
to int or double to int?
On 2015/09/18 22:44:36, John Messerly wrote:
> On 2015/09/18 22:42:27, John Messerly wrote:
> > On 2015/09/18 22:37:49, Leaf wrote:
> > > I think this changes the semantics a little, since previously casting a
> > > non-integral double to an int would have failed, whereas I think now this
> will
> > > allow it to pass, yes?  
> > 
> > I think previously double -> int was a cast. However, num -> int was *not* a
> > cast. So it was kind of inconsistent :\
> > 
> > IMO, if we do want a check for int, we should special case it so it's not
> going
> > through `dart.as` ...
> 
> so from a type system perspective, if JSNumber can't be treated as
implementing
> `int` -- I think I'm back to my original problem of not being able to
implement
> JSNumber in a way that passes override checks ...

I'm not hard set on this, but I'm just a little worried that there may be code
that relies on is/as checks to distinguish between integral and non-integral
values (since dart2js does this).

I think this kind of works out ok if we think of JSNumber as a private
implementation type which users don't refer to directly.  The int and double
type then become kind of assertions about the subset of JSNumber values which
might inhabit them.  You can subvert that by moving a JSNumber directly into an
int or double variable, but in user code they should always be moving between
int, double, and num.  So if we insert the appropriate casts between those, then
user code at least is safe, I think?

Alternatively we could just give up on the difference.

Alternatively, are there other alternatives?  Could JSNumber maybe be a generic
mixin (JSNumber<T extends num>, with int mixing in JSNumber<int> and double
mixing in JSNumber<double>?

[Jennifer Messerly, 2015-09-18 23:53:06]

PTAL, int checks are now fixed, I think.

https://codereview.chromium.org/1348453004/diff/60001/tool/input_sdk/private/...
File tool/input_sdk/private/js_number.dart (right):

https://codereview.chromium.org/1348453004/diff/60001/tool/input_sdk/private/...
tool/input_sdk/private/js_number.dart:246: if (other is !num) throw new
ArgumentError(other);
On 2015/09/18 22:42:27, John Messerly wrote:
> On 2015/09/18 22:37:49, Leaf wrote:
> > I don't know how much we want to assume DDC strong mode in our version of
the
> > SDK, but I think these argument checks aren't needed for us.
> 
> ah, good point. Interesting that isn't a hint (dead code) . I'll try and track
> them down.

fixed kind of ... we still need the null checks though. so most of them are
still around as a checkNull.

[Leaf, 2015-09-19 00:04:22]

lgtm

[Jennifer Messerly, 2015-09-19 00:10:08]

Committed patchset #5 (id:80001) manually as
9baaa466b48180a74da839727c7f050ba8a05544 (presubmit successful).