Ensure stack trace are identical.

Ensure stack trace are identical.

BUG=http://dartbug.com/15171

[ahe, 2013-11-20 13:03:39]

@iposva: The VM already does this, let me know if you have any concerns about
guaranteeing it.

[sigurdm, 2013-11-20 15:43:24]

Basically this LGTM.

I noticed something, this might be unrelated to the issue, but if I run this:

main () {
  var st1;
  try {
    try {
      throw 'bad';
    } catch (e, st) {
      st1 = st;
      print(st);
      rethrow;
    }
  } catch (e, st2) {
    print(st2);
    print(st1 == st2);
    print(identical(st1, st2));
  }
}

in dart vm i get:

#0      main (file:///usr/local/google/home/sigurdm/test/stacktrace.dart:5:9)
#1      _startIsolate.isolateStartHandler
(dart:isolate-patch/isolate_patch.dart:190)
#2      _RawReceivePortImpl._handleMessage
(dart:isolate-patch/isolate_patch.dart:93)

#0      main (file:///usr/local/google/home/sigurdm/test/stacktrace.dart:5:9)
#1      main (file:///usr/local/google/home/sigurdm/test/stacktrace.dart:9:9)
#2      _startIsolate.isolateStartHandler
(dart:isolate-patch/isolate_patch.dart:190)
#3      _RawReceivePortImpl._handleMessage
(dart:isolate-patch/isolate_patch.dart:93)

true
true

But in dart2js:

Error
    at Object.wrapException (out.js:389:15)
    at Object.main (out.js:957:17)
    at out.js:1151:7
    at init.currentScript (out.js:1127:5)
    at out.js:1145:3
    at out.js:1366:3
Error
    at Object.wrapException (out.js:389:15)
    at Object.main (out.js:957:17)
    at out.js:1151:7
    at init.currentScript (out.js:1127:5)
    at out.js:1145:3
    at out.js:1366:3
false
false

It seems that when the exception is re-thrown we need to add a line to the
stacktrace, as happens in dart/runtime/vm/exceptions.cc line 425

[ahe, 2013-11-20 23:57:06]

On 2013/11/20 15:43:24, sigurdm wrote:
> Basically this LGTM.
> 
> I noticed something, this might be unrelated to the issue, but if I run this:
> 
> main () {
>   var st1;
>   try {
>     try {
>       throw 'bad';
>     } catch (e, st) {
>       st1 = st;
>       print(st);
>       rethrow;
>     }
>   } catch (e, st2) {
>     print(st2);
>     print(st1 == st2);
>     print(identical(st1, st2));
>   }
> }
> 
> in dart vm i get:
> 
> #0      main (file:///usr/local/google/home/sigurdm/test/stacktrace.dart:5:9)
> #1      _startIsolate.isolateStartHandler
> (dart:isolate-patch/isolate_patch.dart:190)
> #2      _RawReceivePortImpl._handleMessage
> (dart:isolate-patch/isolate_patch.dart:93)
> 
> #0      main (file:///usr/local/google/home/sigurdm/test/stacktrace.dart:5:9)
> #1      main (file:///usr/local/google/home/sigurdm/test/stacktrace.dart:9:9)
> #2      _startIsolate.isolateStartHandler
> (dart:isolate-patch/isolate_patch.dart:190)
> #3      _RawReceivePortImpl._handleMessage
> (dart:isolate-patch/isolate_patch.dart:93)
> 
> true
> true
> 
> But in dart2js:
> 
> Error
>     at Object.wrapException (out.js:389:15)
>     at Object.main (out.js:957:17)
>     at out.js:1151:7
>     at init.currentScript (out.js:1127:5)
>     at out.js:1145:3
>     at out.js:1366:3
> Error
>     at Object.wrapException (out.js:389:15)
>     at Object.main (out.js:957:17)
>     at out.js:1151:7
>     at init.currentScript (out.js:1127:5)
>     at out.js:1145:3
>     at out.js:1366:3
> false
> false
> 
> It seems that when the exception is re-thrown we need to add a line to the
> stacktrace, as happens in dart/runtime/vm/exceptions.cc line 425

dart2js is hard pressed to change what the browser provides. But even if we
could, I'm not sure the VM's stack trace is correct (I can see how it might be
useful, but it can also be confusing). Perhaps the VM should say that line 9 is
a rethrow.

[nweiz, 2013-11-22 01:00:49]

I agree with Peter than adding the rethrow line to the stack trace is confusing,
especially since asynchronous "rethrows" don't do so.

[Ivan Posva, 2013-11-22 08:10:46]

On 2013/11/20 13:03:39, ahe wrote:
> @iposva: The VM already does this, let me know if you have any concerns about
> guaranteeing it.

+ asiva

In my opinion it is an accident that the VM already does guarantee that the
different traces are identical. I don't think we should and can rely on this.
You'll notice that string representation of the stack trace changes over time,
which is definitely not something we want long-term either. See the example
below and its output:
$ cat ~/dart/Bug15171.dart
main() {
  var st1, st2;
  var st1_str, st2_str;
  try {
    try {
      throw 'bad';
    } catch (e, st) {
      st1 = st;
      st1_str = "$st1";
      print(st1);
      rethrow;
    }
  } catch (e, st) {
    st2 = st;
    st2_str = "$st2";
    print(st1);
    print(st2);
  }
  print(st1);
  print(st2);
  print(st1 == st2);
  print(identical(st1, st2));
  print(st1_str == st2_str);
}
$ dart ~/dart/Bug15171.dart
#0      main (file:///Users/iposva/dart/Bug15171.dart:6:7)
#1      _startIsolate.isolateStartHandler
(dart:isolate-patch/isolate_patch.dart:191)
#2      _RawReceivePortImpl._handleMessage
(dart:isolate-patch/isolate_patch.dart:95)

#0      main (file:///Users/iposva/dart/Bug15171.dart:6:7)
#1      main (file:///Users/iposva/dart/Bug15171.dart:11:7)
#2      _startIsolate.isolateStartHandler
(dart:isolate-patch/isolate_patch.dart:191)
#3      _RawReceivePortImpl._handleMessage
(dart:isolate-patch/isolate_patch.dart:95)

#0      main (file:///Users/iposva/dart/Bug15171.dart:6:7)
#1      main (file:///Users/iposva/dart/Bug15171.dart:11:7)
#2      _startIsolate.isolateStartHandler
(dart:isolate-patch/isolate_patch.dart:191)
#3      _RawReceivePortImpl._handleMessage
(dart:isolate-patch/isolate_patch.dart:95)

#0      main (file:///Users/iposva/dart/Bug15171.dart:6:7)
#1      main (file:///Users/iposva/dart/Bug15171.dart:11:7)
#2      _startIsolate.isolateStartHandler
(dart:isolate-patch/isolate_patch.dart:191)
#3      _RawReceivePortImpl._handleMessage
(dart:isolate-patch/isolate_patch.dart:95)

#0      main (file:///Users/iposva/dart/Bug15171.dart:6:7)
#1      main (file:///Users/iposva/dart/Bug15171.dart:11:7)
#2      _startIsolate.isolateStartHandler
(dart:isolate-patch/isolate_patch.dart:191)
#3      _RawReceivePortImpl._handleMessage
(dart:isolate-patch/isolate_patch.dart:95)

true
true
false
$ 

The value printed for st1 changes over time, which is mildly surprising.

[ahe, 2013-11-22 09:06:23]

+bak

On 2013/11/22 08:10:46, Ivan Posva wrote:
> On 2013/11/20 13:03:39, ahe wrote:
> > @iposva: The VM already does this, let me know if you have any concerns
about
> > guaranteeing it.
> 
> + asiva
> 
> In my opinion it is an accident that the VM already does guarantee that the
> different traces are identical. I don't think we should and can rely on this.

I talked to Lars about this before sending out the CL, and my understanding was
that he had a different opinion.

Sounds like this is a topic for the language team to discuss. I suggest Nathan
tries to attend this discussion or make sure he has prepared someone else to
argue on his behalf.

From the perspective of dart2js implementation, I think we can go either way
without affecting performance.

Cheers,
Peter

> You'll notice that string representation of the stack trace changes over time,
> which is definitely not something we want long-term either. See the example
> below and its output:
> $ cat ~/dart/Bug15171.dart
> main() {
>   var st1, st2;
>   var st1_str, st2_str;
>   try {
>     try {
>       throw 'bad';
>     } catch (e, st) {
>       st1 = st;
>       st1_str = "$st1";
>       print(st1);
>       rethrow;
>     }
>   } catch (e, st) {
>     st2 = st;
>     st2_str = "$st2";
>     print(st1);
>     print(st2);
>   }
>   print(st1);
>   print(st2);
>   print(st1 == st2);
>   print(identical(st1, st2));
>   print(st1_str == st2_str);
> }
> $ dart ~/dart/Bug15171.dart
> #0      main (file:///Users/iposva/dart/Bug15171.dart:6:7)
> #1      _startIsolate.isolateStartHandler
> (dart:isolate-patch/isolate_patch.dart:191)
> #2      _RawReceivePortImpl._handleMessage
> (dart:isolate-patch/isolate_patch.dart:95)
> 
> #0      main (file:///Users/iposva/dart/Bug15171.dart:6:7)
> #1      main (file:///Users/iposva/dart/Bug15171.dart:11:7)
> #2      _startIsolate.isolateStartHandler
> (dart:isolate-patch/isolate_patch.dart:191)
> #3      _RawReceivePortImpl._handleMessage
> (dart:isolate-patch/isolate_patch.dart:95)
> 
> #0      main (file:///Users/iposva/dart/Bug15171.dart:6:7)
> #1      main (file:///Users/iposva/dart/Bug15171.dart:11:7)
> #2      _startIsolate.isolateStartHandler
> (dart:isolate-patch/isolate_patch.dart:191)
> #3      _RawReceivePortImpl._handleMessage
> (dart:isolate-patch/isolate_patch.dart:95)
> 
> #0      main (file:///Users/iposva/dart/Bug15171.dart:6:7)
> #1      main (file:///Users/iposva/dart/Bug15171.dart:11:7)
> #2      _startIsolate.isolateStartHandler
> (dart:isolate-patch/isolate_patch.dart:191)
> #3      _RawReceivePortImpl._handleMessage
> (dart:isolate-patch/isolate_patch.dart:95)
> 
> #0      main (file:///Users/iposva/dart/Bug15171.dart:6:7)
> #1      main (file:///Users/iposva/dart/Bug15171.dart:11:7)
> #2      _startIsolate.isolateStartHandler
> (dart:isolate-patch/isolate_patch.dart:191)
> #3      _RawReceivePortImpl._handleMessage
> (dart:isolate-patch/isolate_patch.dart:95)
> 
> true
> true
> false
> $ 
> 
> The value printed for st1 changes over time, which is mildly surprising.