Nice messages on uncaught exceptions.

Nice messages on uncaught exceptions.

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

[ngeoffray, 2013-01-12 19:08:31]

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/implementation/lib/foreign_helper.dart
(right):

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/foreign_helper.dart:121: *
Warning: this is dangerous, consider using [DART_CLOSURE_TO_JS] instead.
dangerous because it does not save the isolate context nor the call arity?
Anything else? Please add a comment.

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart (right):

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:964: // Use V8
API for recording a "fast" stack trace.
Add a comment that this has the side effect of setting the JS 'stack' property.

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:968: //
Otherwise, produce a stack trace and record it in the wrapper.
new Error().stack produces a stack trace? On all engines? Please add a comment.

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:984:
JS('void', 'this.toString = #', DART_CLOSURE_TO_JS(toStringWrapper));
#.toString

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:987: get stack
=> JS('', 'this.stack');
Why is that not just a Dart field? Because V8 and maybe other engines/browsers
look at the 'stack' property of an Error? Please add a comment.

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:987: get stack
=> JS('', 'this.stack');
#.stack

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:992: //
actually a getter which calls toString.
The JS stack property is a JS getter which calls JS toString of what object?

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:992: //
actually a getter which calls toString.
So where is the stack printer if Error.captureStackTrace is defined?

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:1006:
toStringWrapper() => JS('', r'this').toString();
Can that be a static in DartError?

[ahe, 2013-01-15 08:58:39]

Thank you, Nicolas. PTAL

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/implementation/lib/foreign_helper.dart
(right):

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/foreign_helper.dart:121: *
Warning: this is dangerous, consider using [DART_CLOSURE_TO_JS] instead.
On 2013/01/12 19:08:31, ngeoffray wrote:
> dangerous because it does not save the isolate context nor the call arity?
> Anything else? Please add a comment.

Done.

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart (right):

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:964: // Use V8
API for recording a "fast" stack trace.
On 2013/01/12 19:08:31, ngeoffray wrote:
> Add a comment that this has the side effect of setting the JS 'stack'
property.

Done.

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:968: //
Otherwise, produce a stack trace and record it in the wrapper.
On 2013/01/12 19:08:31, ngeoffray wrote:
> new Error().stack produces a stack trace? On all engines? Please add a
comment.

Done.

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:984:
JS('void', 'this.toString = #', DART_CLOSURE_TO_JS(toStringWrapper));
On 2013/01/12 19:08:31, ngeoffray wrote:
> #.toString

Done.

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:987: get stack
=> JS('', 'this.stack');
On 2013/01/12 19:08:31, ngeoffray wrote:
> #.stack

Done.

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:987: get stack
=> JS('', 'this.stack');
On 2013/01/12 19:08:31, ngeoffray wrote:
> Why is that not just a Dart field? Because V8 and maybe other engines/browsers
> look at the 'stack' property of an Error? Please add a comment.

Done.

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:992: //
actually a getter which calls toString.
On 2013/01/12 19:08:31, ngeoffray wrote:
> The JS stack property is a JS getter which calls JS toString of what object?

Done.

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:992: //
actually a getter which calls toString.
On 2013/01/12 19:08:31, ngeoffray wrote:
> So where is the stack printer if Error.captureStackTrace is defined?

Chrome dev-tools and d8 prints the stack trace.

https://codereview.chromium.org/11858017/diff/2001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:1006:
toStringWrapper() => JS('', r'this').toString();
On 2013/01/12 19:08:31, ngeoffray wrote:
> Can that be a static in DartError?

Done.

[ngeoffray, 2013-01-15 09:15:42]

LGTM! Thanks Peter.

https://codereview.chromium.org/11858017/diff/7001/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart (right):

https://codereview.chromium.org/11858017/diff/7001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:972: //
Otherwise, produce a stack trace and record it in the wrapper.
I think you can merge this comment with the new one above.

[ahe, 2013-01-15 10:34:30]

https://codereview.chromium.org/11858017/diff/7001/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart (right):

https://codereview.chromium.org/11858017/diff/7001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:972: //
Otherwise, produce a stack trace and record it in the wrapper.
On 2013/01/15 09:15:42, ngeoffray wrote:
> I think you can merge this comment with the new one above.

Done.

https://codereview.chromium.org/11858017/diff/10001/dart/sdk/lib/_internal/co...
File dart/sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/11858017/diff/10001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/ssa/builder.dart:3082:
FunctionSignature handleForeignRawFunctionRef(Send node, String name) {
Found a type error during testing.

[sra1, 2013-01-15 11:48:43]

https://codereview.chromium.org/11858017/diff/7001/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart (right):

https://codereview.chromium.org/11858017/diff/7001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:963: if
(JS('bool', 'Error.captureStackTrace')) {
!!Error.captureStackTrace

Otherwise it depends too much on how the dart2js inserts bool checks in checked
mode.  It is possible that in checked mode that the JS function or 'undefined'
value will be flagged as not bool.

https://codereview.chromium.org/11858017/diff/7001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:1015: if
(JS('bool', 'Error.captureStackTrace') || (stack == null)) {
!!

https://codereview.chromium.org/11858017/diff/7001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:1027: static
toStringWrapper() => JS('', r'this').toString();
Make this a private name?

https://codereview.chromium.org/11858017/diff/7001/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/11858017/diff/7001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/ssa/builder.dart:3084:
compiler.cancel('$name requires exactly one argument',
It is a good idea to put names and program elements in single quotes in
diagnostics to avoid confusion when the name makes sense as a word in the
sentence

"'$name' requires exactly one argument".

https://codereview.chromium.org/11858017/diff/7001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/ssa/builder.dart:3091: '$name
requires a static or top-level method',
ditto quotes

[ahe, 2013-01-15 12:56:01]

Addressed in https://codereview.chromium.org/11877029/.

https://codereview.chromium.org/11858017/diff/7001/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart (right):

https://codereview.chromium.org/11858017/diff/7001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:963: if
(JS('bool', 'Error.captureStackTrace')) {
On 2013/01/15 11:48:43, sra1 wrote:
> !!Error.captureStackTrace
> 
> Otherwise it depends too much on how the dart2js inserts bool checks in
checked
> mode.  It is possible that in checked mode that the JS function or 'undefined'
> value will be flagged as not bool.

Done.

https://codereview.chromium.org/11858017/diff/7001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:1015: if
(JS('bool', 'Error.captureStackTrace') || (stack == null)) {
On 2013/01/15 11:48:43, sra1 wrote:
> !!

Done.

https://codereview.chromium.org/11858017/diff/7001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:1027: static
toStringWrapper() => JS('', r'this').toString();
On 2013/01/15 11:48:43, sra1 wrote:
> Make this a private name?

Why?

[ahe, 2013-01-15 13:02:34]

Also added to https://codereview.chromium.org/11877029

https://codereview.chromium.org/11858017/diff/7001/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/11858017/diff/7001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/ssa/builder.dart:3084:
compiler.cancel('$name requires exactly one argument',
On 2013/01/15 11:48:43, sra1 wrote:
> It is a good idea to put names and program elements in single quotes in
> diagnostics to avoid confusion when the name makes sense as a word in the
> sentence
> 
> "'$name' requires exactly one argument".

Done (except I prefer double quotation marks in error messages).

https://codereview.chromium.org/11858017/diff/7001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/ssa/builder.dart:3091: '$name
requires a static or top-level method',
On 2013/01/15 11:48:43, sra1 wrote:
> ditto quotes

Done.