I'm okay with this change even though I'm not completely convinced that the template trick is superior to simply specializing an abstract iterator, but it still LGTM.
http://codereview.chromium.org/39009/diff/1001/1003 File src/frames.cc (right): http://codereview.chromium.org/39009/diff/1001/1003#newcode171 Line 171: if (!iteration_done_) { How about inverting this and returning with an early bailout? http://codereview.chromium.org/39009/diff/1001/1003#newcode173 Line 173: if (!iterator_.done()) { Ditto. http://codereview.chromium.org/39009/diff/1001/1004 File src/frames.h (right): http://codereview.chromium.org/39009/diff/1001/1004#newcode561 Line 561: explicit JavaScriptFrameIteratorTemp(Address low_bound, Address high_bound) : Nit: Doesn't have to be explicit. http://codereview.chromium.org/39009/diff/1001/1004#newcode611 Line 611: static bool IsInBounds( Maybe IsWithinBounds? http://codereview.chromium.org/39009/diff/1001/1004#newcode615 Line 615: bool IsGoodStackAddress(Address addr) const { Maybe IsValidStackAddress? http://codereview.chromium.org/39009/diff/1001/1006 File src/log.h (right): http://codereview.chromium.org/39009/diff/1001/1006#newcode282 Line 282: // Maximum number of stack frames to capture Terminate comment with . http://codereview.chromium.org/39009/diff/1001/1007 File src/platform.h (right): http://codereview.chromium.org/39009/diff/1001/1007#newcode484 Line 484: if (depth) { depth != 0 (or maybe >) http://codereview.chromium.org/39009 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
