Rename RuntimeError to CyclicIntializationError, as per spec.

Rename RuntimeError to CyclicIntializationError, as per spec.

The RuntimeError has been used to report cyclic initialization errors in
dart2js.
It has also been used for other, unrelated, uses, where it makes
little to no sense.

The RuntimeError type has been removed, but added in js_helper.dart,
where dart2js uses it.

Uses of new RuntimeError("some message") have been converted to
other errors (Argument/State/Unsupported). In some cases, just
throwing a String might have been just as good.

BUG=http://dartbug.com/11040
R=johnniwinther@google.com

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

[Lasse Reichstein Nielsen, 2013-06-04 09:42:13]

@regis: This adds the exception to throw on circular initialization (per TODO in
parser.cc).

[Johnni Winther, 2013-06-04 09:58:52]

https://codereview.chromium.org/16154017/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/lib/js_helper.dart (right):

https://codereview.chromium.org/16154017/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:1468: */
What is the reason for all the changes above?

https://codereview.chromium.org/16154017/diff/1/tests/language/lazy_static3_t...
File tests/language/lazy_static3_test.dart (right):

https://codereview.chromium.org/16154017/diff/1/tests/language/lazy_static3_t...
tests/language/lazy_static3_test.dart:48: Expect.throws(() => fib(x3), (e) => e
is CyclicInitializationError);
Doesn't this change make the test fail?

https://codereview.chromium.org/16154017/diff/1/tests/language/lazy_static3_t...
tests/language/lazy_static3_test.dart:57: Expect.throws(() => fib(x7), (e) => e
is CyclicInitializationError);
Ditto.

[Lasse Reichstein Nielsen, 2013-06-04 10:21:07]

ptal

https://codereview.chromium.org/16154017/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/lib/js_helper.dart (right):

https://codereview.chromium.org/16154017/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:1468: */
Looks like a really bad merge.

https://codereview.chromium.org/16154017/diff/1/tests/language/lazy_static3_t...
File tests/language/lazy_static3_test.dart (right):

https://codereview.chromium.org/16154017/diff/1/tests/language/lazy_static3_t...
tests/language/lazy_static3_test.dart:48: Expect.throws(() => fib(x3), (e) => e
is CyclicInitializationError);
The VM already fails this test. I think it just throws a String.
Dart2js should succeede.

https://codereview.chromium.org/16154017/diff/1/tests/language/lazy_static3_t...
tests/language/lazy_static3_test.dart:57: Expect.throws(() => fib(x7), (e) => e
is CyclicInitializationError);
Ditto too.

[Johnni Winther, 2013-06-04 10:28:50]

lgtm

[Lasse Reichstein Nielsen, 2013-06-04 10:33:29]

Committed patchset #2 manually as r23584 (presubmit successful).

[ahe, 2013-06-04 13:14:11]

https://codereview.chromium.org/16154017/diff/22/sdk/lib/_internal/compiler/i...
File sdk/lib/_internal/compiler/implementation/lib/js_mirrors.dart (right):

https://codereview.chromium.org/16154017/diff/22/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/lib/js_mirrors.dart:431: throw new
ArgumentError('Bad field descriptor: $descriptor');
What makes you think this is an argument error?

https://codereview.chromium.org/16154017/diff/22/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/lib/js_mirrors.dart:484: throw new
ArgumentError('Cannot find callName on "$reflectee"');
What makes you think this is an argument error?  What argument in particular do
you feel this error is related to?

[Lasse Reichstein Nielsen, 2013-06-13 08:08:57]

https://codereview.chromium.org/16154017/diff/22/sdk/lib/_internal/compiler/i...
File sdk/lib/_internal/compiler/implementation/lib/js_mirrors.dart (right):

https://codereview.chromium.org/16154017/diff/22/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/lib/js_mirrors.dart:431: throw new
ArgumentError('Bad field descriptor: $descriptor');
Descriptor is an argument. It's being tested in some way, and if it doesn't
satisfy the test, an error is thrown that says that the descriptor is bad.
That sounds like an argument error.
Could be wrong ofcourse, and you are free to find another error to use.

https://codereview.chromium.org/16154017/diff/22/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/lib/js_mirrors.dart:484: throw new
ArgumentError('Cannot find callName on "$reflectee"');
True, I probably misread the code due to the inlined code-as-text above.
Looks more like a StateError.

[ahe, 2013-06-13 09:59:14]

Lasse,

Sorry, you found one of my many pet peeves ;-)

Cheers,
Peter

https://codereview.chromium.org/16154017/diff/22/sdk/lib/_internal/compiler/i...
File sdk/lib/_internal/compiler/implementation/lib/js_mirrors.dart (right):

https://codereview.chromium.org/16154017/diff/22/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/lib/js_mirrors.dart:431: throw new
ArgumentError('Bad field descriptor: $descriptor');
On 2013/06/13 08:08:57, Lasse Reichstein Nielsen wrote:
> Descriptor is an argument. It's being tested in some way, and if it doesn't
> satisfy the test, an error is thrown that says that the descriptor is bad.
> That sounds like an argument error.
> Could be wrong ofcourse, and you are free to find another error to use.

I'm sorry, I'm on a crusade against incorrect uses of "generic" exceptions: any
exception thrown gets exposed to the user because it bubbles up through the call
stack. So I think it is important that the exception conveys the right message.

The message an ArgumentError coveys is that the user provided a wrong argument
to a method (or factory). However, this factory is not accessible to users and
the user has no control over the descriptor argument, and this exception might
occur when a user writes code like this:

someClassMirror.variables

There are no arguments here, but an internal error might happen at runtime, and
I need to know the descriptor to debug the problem. The users can't fix it
themselves: there's nothing wrong with any arguments (a getter doesn't take
arguments).

https://codereview.chromium.org/16154017/diff/22/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/lib/js_mirrors.dart:484: throw new
ArgumentError('Cannot find callName on "$reflectee"');
On 2013/06/13 08:08:57, Lasse Reichstein Nielsen wrote:
> True, I probably misread the code due to the inlined code-as-text above.
> Looks more like a StateError.

This is what StateError says it represents: "The operation was not allowed by
the current state of the object."

See above rant: the user has done nothing to put the JsClosureMirror in a bad
state, and cannot do anything to bring it to a different state.  The object has
no states besides being there or not (all fields are final, and all fields refer
to immutable objects).

You can only obtain an instance of this object by calling "reflect(foo)" where
foo is a closure.  If it is not a closure, reflect will return a
JsInstanceMirror.

This represents an internal error in the runtime implementation of the Dart
platform.  I think RuntimeError (or a string) conveys that much better than
StateError.