Add some ArgumentError and RangeError constructors that capture more information.

Add some ArgumentError and RangeError constructors that capture more information.

Switch some uses of RangeError.range to RangeError.index.
Fix bug in Queue where elementAt allowed `length` as input.

R=sgjesse@google.com

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

[Lasse Reichstein Nielsen, 2014-11-10 11:37:43]

lrn@google.com changed reviewers:
+ sgjesse@google.com

[Søren Gjesse, 2014-11-10 12:27:29]

lgtm

[Lasse Reichstein Nielsen, 2014-11-11 07:56:04]

Committed patchset #1 manually as r41653 (presubmit successful).

[sra1, 2014-11-11 20:04:16]

sra@google.com changed reviewers:
+ sra@google.com

[sra1, 2014-11-11 20:04:17]

This change adds 4.3k to swarm.

Is there a way to do this without so much new code?

One problem is the default values for the additional arguments occur at many
call sites.
Another problem is that the toString methods seem to have poor code quality

[Lasse Reichstein Nielsen, 2014-11-11 20:18:04]

On 2014/11/11 20:04:17, sra1 wrote:
> This change adds 4.3k to swarm.
> 
> Is there a way to do this without so much new code?
> 
> One problem is the default values for the additional arguments occur at many
> call sites.

So Dart2JS prefers to not have default arguments values? 
It should be easy to change to a default of null, and use the default if the
value is null.

> Another problem is that the toString methods seem to have poor code quality

True, they are not trying to be smart, since being in an error is being on the
slow path.
If that causes code bloat too, it's a different problem - slow path shouldn't be
big path.

[sra1, 2014-11-11 21:45:22]

On 2014/11/11 20:18:04, Lasse Reichstein Nielsen wrote:
> On 2014/11/11 20:04:17, sra1 wrote:
> > This change adds 4.3k to swarm.
> > 
> > Is there a way to do this without so much new code?
> > 
> > One problem is the default values for the additional arguments occur at many
> > call sites.
> 
> So Dart2JS prefers to not have default arguments values? 
> It should be easy to change to a default of null, and use the default if the
> value is null.

https://codereview.chromium.org/717823002/ saves ~400 bytes 
If you are OK with this, feel free to commit it.

> 
> > Another problem is that the toString methods seem to have poor code quality

To be clear, I mean it generates verbose generic code, not a comment on the the
Dart level code.

> 
> True, they are not trying to be smart, since being in an error is being on the
> slow path.
> If that causes code bloat too, it's a different problem - slow path shouldn't
be
> big path.

1355 bytes comes from the fact that Error.toSafeString no longer has the 'object
is String' path optimized away.
I'm not sure if this is because of falling off an edge with type inference, or
whether strings now flow to the function.

The rest is in the new toString methods.

[sra1, 2014-11-11 22:04:11]

https://codereview.chromium.org/711003002/diff/1/sdk/lib/core/errors.dart
File sdk/lib/core/errors.dart (right):

https://codereview.chromium.org/711003002/diff/1/sdk/lib/core/errors.dart#new...
sdk/lib/core/errors.dart:273: int length]) = IndexError;
Doesn't this forwarding incorrectly store the index in IndexError.indexable?

https://codereview.chromium.org/711003002/diff/1/sdk/lib/core/errors.dart#new...
sdk/lib/core/errors.dart:307: final int invalidValue;
This field shadows the field of the same name in ArgumentError, which becomes an
unused field.

ArgumentError._hasValue field is also unused in subclasses.

It might be cleaner to extend Error and implement message/name/start/end.

RangeError has the same problem.

[Lasse Reichstein Nielsen, 2014-11-12 07:21:38]

On 2014/11/11 21:45:22, sra1 wrote:
 
> https://codereview.chromium.org/717823002/ saves ~400 bytes 
> If you are OK with this, feel free to commit it.

Looks fine! I even like it better than the original :)

> > 
> > > Another problem is that the toString methods seem to have poor code
quality
> 
> To be clear, I mean it generates verbose generic code, not a comment on the
the
> Dart level code.

Well, it could mean that too. It really is "dumb" code, and I think it could 
save some size by doing some common sub-expression unification instead of having
four independent string interpolations.
It'll cost a little in execution time and space, but this is an Error's toString
- if it is called, your program is likely already hosed.

> > True, they are not trying to be smart, since being in an error is being on
the
> > slow path.
> > If that causes code bloat too, it's a different problem - slow path
shouldn't
> be
> > big path.
> 
> 1355 bytes comes from the fact that Error.toSafeString no longer has the
'object
> is String' path optimized away.
> I'm not sure if this is because of falling off an edge with type inference, or
> whether strings now flow to the function.

I have made no attempt to avoid strings ending up in there, so it's quite
possible
that it looks like it can happen.
The string branch is rather big. In dart2js, you could just do JS("string",
"JSON.stringify(#)", string)
to get mostly the same result (uses \u instead of \x, and also escapes
form-feed). 
We could extract the string part into an external function?

> The rest is in the new toString methods.

I'll look at those and see if I can optimize them for compiled size.

[Lasse Reichstein Nielsen, 2014-11-12 07:38:06]

https://codereview.chromium.org/711003002/diff/1/sdk/lib/core/errors.dart
File sdk/lib/core/errors.dart (right):

https://codereview.chromium.org/711003002/diff/1/sdk/lib/core/errors.dart#new...
sdk/lib/core/errors.dart:273: int length]) = IndexError;
Yes, fixed in follow-up CL.
Only Dart2js actually caught it.

https://codereview.chromium.org/711003002/diff/1/sdk/lib/core/errors.dart#new...
sdk/lib/core/errors.dart:307: final int invalidValue;
Yes, this is silly. Using implements ArgumentError is probably simpler, or
reusing the invalidValue field if possible.