|
|
Created:
7 years, 1 month ago by ahe Modified:
5 years, 10 months ago CC:
reviews_dartlang.org, nweiz, bakster Visibility:
Public. |
DescriptionEnsure stack trace are identical.
BUG=http://dartbug.com/15171
Patch Set 1 : #
Messages
Total messages: 6 (0 generated)
@iposva: The VM already does this, let me know if you have any concerns about guaranteeing it.
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
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.
I agree with Peter than adding the rethrow line to the stack trace is confusing, especially since asynchronous "rethrows" don't do so.
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.
+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. |