Don't add the catch frame at a rethrow to the stacktrace.

Don't add the catch frame at a rethrow to the stacktrace.

R=asiva@google.com

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

[rmacnak, 2014-06-18 20:38:00]

ParserCombinators
+11% x64
+13% ia32
+22% ARM

[rmacnak, 2014-06-20 22:27:34]

Took out the optimization.

[rmacnak, 2014-06-25 21:49:24]

As discussed offline, moved logic to skip the first frame to Stacktrace::Append.

[siva, 2014-06-25 22:46:12]

LGTM with some comments.

https://codereview.chromium.org/342473006/diff/60001/runtime/vm/exceptions.cc
File runtime/vm/exceptions.cc (right):

https://codereview.chromium.org/342473006/diff/60001/runtime/vm/exceptions.cc...
runtime/vm/exceptions.cc:441: } else {
How about moving the assert on top to here as
ASSERT(is_rethrow);

https://codereview.chromium.org/342473006/diff/60001/runtime/vm/exceptions.cc...
runtime/vm/exceptions.cc:447: const intptr_t offset = is_rethrow ? 1 : 0;
Can this just be:
const intptr_t offset = 1;
since we have asserted that it is always a rethrow in this case.

https://codereview.chromium.org/342473006/diff/60001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/342473006/diff/60001/runtime/vm/object.cc#new...
runtime/vm/object.cc:18604: ASSERT(offset <= code_list.Length());
ASSERT(offset....) should be above the use of 'offset' in the subtract above.

https://codereview.chromium.org/342473006/diff/60001/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/342473006/diff/60001/runtime/vm/object.h#newc...
runtime/vm/object.h:6786: const intptr_t offset) const;
I would rename 'offset' to 'start_index'

[rmacnak, 2014-06-25 23:00:43]

https://codereview.chromium.org/342473006/diff/60001/runtime/vm/exceptions.cc
File runtime/vm/exceptions.cc (right):

https://codereview.chromium.org/342473006/diff/60001/runtime/vm/exceptions.cc...
runtime/vm/exceptions.cc:447: const intptr_t offset = is_rethrow ? 1 : 0;
On 2014/06/25 22:46:12, siva wrote:
> Can this just be:
> const intptr_t offset = 1;
> since we have asserted that it is always a rethrow in this case.

Ah, yes.

https://codereview.chromium.org/342473006/diff/60001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/342473006/diff/60001/runtime/vm/object.cc#new...
runtime/vm/object.cc:18604: ASSERT(offset <= code_list.Length());
On 2014/06/25 22:46:12, siva wrote:
> ASSERT(offset....) should be above the use of 'offset' in the subtract above.

Done

https://codereview.chromium.org/342473006/diff/60001/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/342473006/diff/60001/runtime/vm/object.h#newc...
runtime/vm/object.h:6786: const intptr_t offset) const;
On 2014/06/25 22:46:12, siva wrote:
> I would rename 'offset' to 'start_index'

That's better.

[rmacnak, 2014-06-25 23:02:03]

Committed patchset #5 manually as r37712 (presubmit successful).