Rename BadNumberFormatException -> FormatException.

Rename BadNumberFormatException -> FormatException.

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

[Bob Nystrom, 2012-08-03 00:51:19]

I got a positive response from my suggestions about doing a little exception
class clean-up, so this is the first small step.

This touches a lot of places, so I added lots of reviewers. Here's the stuff you
only need to look at (but, of course, you're welcome to review the whole patch):

Peter: anything you like, especially the status file
Vijay: lib/html/
Kasper: lib/compiler/
Ivan: runtime/, lib/math, tests/lib/math
Gram: lib/unittest, tests/lib/unittest
Lasse, Mads: corelib/, tests/corelib
Nathan: lib/args, tests/lib/args, utils/

I've never done a patch that touches this many things, so let me know if I did
anything dumb. I ran:

$ test.py -m release
$ test.py -m release -c dart2js -r d8

Assuming I eventually get an LGTM, I'll file a bug on the co19 bug tracker to
get the co19 tests updated to the new name.

[kasperl, 2012-08-03 05:25:19]

The (rather trivial) changes to lib/compiler/* look good to me and I definitely
support cleaning up and simplifying the exception class hierarchy.

https://chromiumcodereview.appspot.com/10850034/diff/1/corelib/src/exceptions...
File corelib/src/exceptions.dart (right):

https://chromiumcodereview.appspot.com/10850034/diff/1/corelib/src/exceptions...
corelib/src/exceptions.dart:118: const FormatException(String this._s);
There's no need to explicitly type the constructor parameter in this case. Maybe
it's time to give it a better name than _s too?

[ahe, 2012-08-03 06:31:21]

I look through everything and it all LGTM.

I think you should make sure that someone from the VM team reviews this change.
If Ivan is busy, you could try pinging one of regis, hausner, asiva, or srdjan.

[ahe, 2012-08-03 06:32:59]

On 2012/08/03 06:31:21, ahe wrote:
> I look through everything and it all LGTM.

Sigh. "I looked". Past tense.

[Mads Ager (google), 2012-08-03 07:28:53]

LGTM

https://chromiumcodereview.appspot.com/10850034/diff/1/tests/co19/co19-runtim...
File tests/co19/co19-runtime.status (right):

https://chromiumcodereview.appspot.com/10850034/diff/1/tests/co19/co19-runtim...
tests/co19/co19-runtime.status:443: [ $mode == debug || $mode == release ]
Maybe just move these to the top without a section marker instead of having the
always true condition?

[gram, 2012-08-03 16:54:21]

lgtm

The changes to unittest library and its tests look fine.

[vsm, 2012-08-03 16:59:48]

lgtm on lib/html

[Bob Nystrom, 2012-08-03 17:46:54]

Thanks!

There's one other change I added: I'm marking pub_test as fail. Pub invokes the
pre-built Dart binaries under tools/testing/bin, which I haven't rebuilt.

I can build them, but I want to make sure they're built with the right
configuration. So my plan is to get this patch in and then talk to Søren about
rebuilding those and then re-enabling that test.

https://chromiumcodereview.appspot.com/10850034/diff/1/corelib/src/exceptions...
File corelib/src/exceptions.dart (right):

https://chromiumcodereview.appspot.com/10850034/diff/1/corelib/src/exceptions...
corelib/src/exceptions.dart:118: const FormatException(String this._s);
On 2012/08/03 05:25:19, kasperl wrote:
> There's no need to explicitly type the constructor parameter in this case.
Maybe
> it's time to give it a better name than _s too?

Done and good call. After I sent this out, I realized making it a nicely named
public field would let you get at the raw message, which toString() doesn't.
Renamed to "message".

https://chromiumcodereview.appspot.com/10850034/diff/1/tests/co19/co19-runtim...
File tests/co19/co19-runtime.status (right):

https://chromiumcodereview.appspot.com/10850034/diff/1/tests/co19/co19-runtim...
tests/co19/co19-runtime.status:443: [ $mode == debug || $mode == release ]
On 2012/08/03 07:28:53, Mads Ager wrote:
> Maybe just move these to the top without a section marker instead of having
the
> always true condition?

Done.

[nweiz, 2012-08-03 18:22:23]

lgtm

https://chromiumcodereview.appspot.com/10850034/diff/8001/utils/tests/pub/pub...
File utils/tests/pub/pub.status (right):

https://chromiumcodereview.appspot.com/10850034/diff/8001/utils/tests/pub/pub...
utils/tests/pub/pub.status:8: pub_test: Fail
Add a TODO here, so we remember to re-enable this.

[Bob Nystrom, 2012-08-03 18:25:02]

https://chromiumcodereview.appspot.com/10850034/diff/8001/utils/tests/pub/pub...
File utils/tests/pub/pub.status (right):

https://chromiumcodereview.appspot.com/10850034/diff/8001/utils/tests/pub/pub...
utils/tests/pub/pub.status:8: pub_test: Fail
On 2012/08/03 18:22:23, nweiz wrote:
> Add a TODO here, so we remember to re-enable this.

Done. Also, this is marked fail instead of skip, so when new binaries are
uploaded, this test will start passing and the build will yell at us.

[Ivan Posva, 2012-08-03 20:36:27]

The VM changes LGTM.

Please fix the issue about pub using the wrong Dart binaries. I would be fine if
that was in a different CL though.

-Ivan

https://chromiumcodereview.appspot.com/10850034/diff/9007/corelib/src/excepti...
File corelib/src/exceptions.dart (right):

https://chromiumcodereview.appspot.com/10850034/diff/9007/corelib/src/excepti...
corelib/src/exceptions.dart:121: final String message;
Is it intentional that you are making the message public here?

https://chromiumcodereview.appspot.com/10850034/diff/9007/utils/tests/pub/pub...
File utils/tests/pub/pub.status (right):

https://chromiumcodereview.appspot.com/10850034/diff/9007/utils/tests/pub/pub...
utils/tests/pub/pub.status:7: # TODO(rnystrom): Remove this once new binaries
are checked in.
The fact that you have to put this here points out why it is broken to use the
prebuilt binaries to run actual tests.

[Bob Nystrom, 2012-08-06 23:55:49]

On 2012/08/03 20:36:27, Ivan Posva wrote:
> The VM changes LGTM.
> 
> Please fix the issue about pub using the wrong Dart binaries. I would be fine
if
> that was in a different CL though.

Thanks! I think I will do that in this CL so I don't have to disable the pub
tests.

> 
> -Ivan
> 
>
https://chromiumcodereview.appspot.com/10850034/diff/9007/corelib/src/excepti...
> File corelib/src/exceptions.dart (right):
> 
>
https://chromiumcodereview.appspot.com/10850034/diff/9007/corelib/src/excepti...
> corelib/src/exceptions.dart:121: final String message;
> Is it intentional that you are making the message public here?
> 
>
https://chromiumcodereview.appspot.com/10850034/diff/9007/utils/tests/pub/pub...
> File utils/tests/pub/pub.status (right):
> 
>
https://chromiumcodereview.appspot.com/10850034/diff/9007/utils/tests/pub/pub...
> utils/tests/pub/pub.status:7: # TODO(rnystrom): Remove this once new binaries
> are checked in.
> The fact that you have to put this here points out why it is broken to use the
> prebuilt binaries to run actual tests.