LGTM with nits

http://codereview.chromium.org/11275186/diff/10/src/isolate.cc
File src/isolate.cc (right):

http://codereview.chromium.org/11275186/diff/10/src/isolate.cc#newcode562
src/isolate.cc:562: bool ShowFrameInStackTrace(StackFrame* raw_frame,
Make this static or a method of StackFrame. Furthermore, the name should
make it clearer that this is a predicate, e.g. IsVisibleInStackTrace.

http://codereview.chromium.org/11275186/diff/10/src/isolate.cc#newcode566
src/isolate.cc:566: if (!raw_frame->is_java_script()) {
Just for consistency: One-liner if...

http://codereview.chromium.org/11275186/diff/10/src/isolate.cc#newcode572
src/isolate.cc:572: if (!raw_fun->IsJSFunction()) {
Again just for consistency: One-liner if...

http://codereview.chromium.org/11275186/diff/10/src/isolate.cc#newcode611
src/isolate.cc:611: while (!iter.done() && frames_seen < limit) {
A for-loop with a conditional break in it might be more readable.

http://codereview.chromium.org/11275186/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to