Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(20)

Issue 78343002: Ensure stack trace are identical. (Closed)

Created:
7 years, 1 month ago by ahe
Modified:
5 years, 10 months ago
Reviewers:
nweiz, siva, sigurdm
CC:
reviews_dartlang.org, nweiz, bakster
Visibility:
Public.

Description

Ensure stack trace are identical. BUG=http://dartbug.com/15171

Patch Set 1 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+30 lines, -1 line) Patch
M dart/sdk/lib/_internal/lib/js_helper.dart View 1 chunk +7 lines, -1 line 0 comments Download
A dart/tests/language/identical_trace_test.dart View 1 chunk +23 lines, -0 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
ahe
@iposva: The VM already does this, let me know if you have any concerns about ...
7 years, 1 month ago (2013-11-20 13:03:39 UTC) #1
sigurdm
Basically this LGTM. I noticed something, this might be unrelated to the issue, but if ...
7 years, 1 month ago (2013-11-20 15:43:24 UTC) #2
ahe
On 2013/11/20 15:43:24, sigurdm wrote: > Basically this LGTM. > > I noticed something, this ...
7 years, 1 month ago (2013-11-20 23:57:06 UTC) #3
nweiz
I agree with Peter than adding the rethrow line to the stack trace is confusing, ...
7 years, 1 month ago (2013-11-22 01:00:49 UTC) #4
Ivan Posva
On 2013/11/20 13:03:39, ahe wrote: > @iposva: The VM already does this, let me know ...
7 years, 1 month ago (2013-11-22 08:10:46 UTC) #5
ahe
7 years, 1 month ago (2013-11-22 09:06:23 UTC) #6
+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.

Powered by Google App Engine
This is Rietveld 408576698