Support for stack traces in catch blocks.

Support for stack traces in catch blocks.
Committed: https://code.google.com/p/dart/source/detail?r=5018

[ahe, 2012-03-06 08:44:22]

LGTM

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/lib/js_helper....
File frog/leg/lib/js_helper.dart (right):

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/lib/js_helper....
frog/leg/lib/js_helper.dart:1307: class StackTrace {
This class is hidden from users (because it is defined in this file). Is it
supposed to be part of coreimpl?

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/lib/js_helper....
frog/leg/lib/js_helper.dart:1313: getTraceFromException(exception) {
Please add a documentation comment that explains where this is called from.

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/lib/js_helper....
frog/leg/lib/js_helper.dart:1314: return new StackTrace(JS("var", @"$0.stack",
exception));
Nice. I didn't realize you could use "var" here. But I guess it is obvious.

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/tree/nodes.dart
File frog/leg/tree/nodes.dart (right):

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/tree/nodes.dar...
frog/leg/tree/nodes.dart:1595: VariableDefinitions declarations =
formals.nodes.head;
This may be null. The parser accepts:

try {} catch () {}

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/tree/nodes.dar...
frog/leg/tree/nodes.dart:1600: Link<VariableDefinitions> declarations =
formals.nodes.tail;
See comment above.

[floitsch, 2012-03-06 10:22:32]

LGTM.

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/lib/js_helper....
File frog/leg/lib/js_helper.dart (right):

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/lib/js_helper....
frog/leg/lib/js_helper.dart:1310: toString() => stack != null ? stack : '';
Please type toString as returning a String.

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/ssa/builder.dart
File frog/leg/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/ssa/builder.da...
frog/leg/ssa/builder.dart:2334: HInstruction traceInstruction = null;
Did you consider to fetch the trace lazily (in the visitThen) instead of
eagerly?

[ngeoffray, 2012-03-06 12:12:50]

Thanks for the comments!

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/lib/js_helper....
File frog/leg/lib/js_helper.dart (right):

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/lib/js_helper....
frog/leg/lib/js_helper.dart:1307: class StackTrace {
On 2012/03/06 08:44:22, ahe wrote:
> This class is hidden from users (because it is defined in this file). Is it
> supposed to be part of coreimpl?

The StackTrace class/interface is not in core, nor in coreimpl. I belive it
should be in the core library.

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/lib/js_helper....
frog/leg/lib/js_helper.dart:1310: toString() => stack != null ? stack : '';
On 2012/03/06 10:22:32, floitsch wrote:
> Please type toString as returning a String.

Done.

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/lib/js_helper....
frog/leg/lib/js_helper.dart:1313: getTraceFromException(exception) {
On 2012/03/06 08:44:22, ahe wrote:
> Please add a documentation comment that explains where this is called from.

Done.

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/ssa/builder.dart
File frog/leg/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/ssa/builder.da...
frog/leg/ssa/builder.dart:2334: HInstruction traceInstruction = null;
On 2012/03/06 10:22:32, floitsch wrote:
> Did you consider to fetch the trace lazily (in the visitThen) instead of
> eagerly?

Yes. But the last implementation I ended up with did not do this. I have upaded
the code to do it lazily.

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/tree/nodes.dart
File frog/leg/tree/nodes.dart (right):

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/tree/nodes.dar...
frog/leg/tree/nodes.dart:1595: VariableDefinitions declarations =
formals.nodes.head;
On 2012/03/06 08:44:22, ahe wrote:
> This may be null. The parser accepts:
> 
> try {} catch () {}

Done.

https://chromiumcodereview.appspot.com/9601014/diff/1/frog/leg/tree/nodes.dar...
frog/leg/tree/nodes.dart:1600: Link<VariableDefinitions> declarations =
formals.nodes.tail;
On 2012/03/06 08:44:22, ahe wrote:
> See comment above.

Done.

[ahe, 2012-03-06 12:24:07]

LGTM

https://chromiumcodereview.appspot.com/9601014/diff/4002/frog/leg/ssa/builder...
File frog/leg/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9601014/diff/4002/frog/leg/ssa/builder...
frog/leg/ssa/builder.dart:2388: HInstruction traceInstruction = null;
This is unused.

https://chromiumcodereview.appspot.com/9601014/diff/4002/frog/leg/ssa/builder...
frog/leg/ssa/builder.dart:2389: if (requiresTrace) {
Remove this.

https://chromiumcodereview.appspot.com/9601014/diff/4002/frog/leg/tree/nodes....
File frog/leg/tree/nodes.dart (right):

https://chromiumcodereview.appspot.com/9601014/diff/4002/frog/leg/tree/nodes....
frog/leg/tree/nodes.dart:1664: if (formals.nodes.isEmpty()) return null;
Perhaps you should check if this returns null in the resolver.

[ngeoffray, 2012-03-06 12:29:40]

Thanks Peter for catching this!

https://chromiumcodereview.appspot.com/9601014/diff/4002/frog/leg/ssa/builder...
File frog/leg/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9601014/diff/4002/frog/leg/ssa/builder...
frog/leg/ssa/builder.dart:2388: HInstruction traceInstruction = null;
On 2012/03/06 12:24:07, ahe wrote:
> This is unused.

Done.

https://chromiumcodereview.appspot.com/9601014/diff/4002/frog/leg/ssa/builder...
frog/leg/ssa/builder.dart:2389: if (requiresTrace) {
On 2012/03/06 12:24:07, ahe wrote:
> Remove this.

Done.

https://chromiumcodereview.appspot.com/9601014/diff/4002/frog/leg/tree/nodes....
File frog/leg/tree/nodes.dart (right):

https://chromiumcodereview.appspot.com/9601014/diff/4002/frog/leg/tree/nodes....
frog/leg/tree/nodes.dart:1664: if (formals.nodes.isEmpty()) return null;
On 2012/03/06 12:24:07, ahe wrote:
> Perhaps you should check if this returns null in the resolver.

It's already doing it.